docs(plan): in_place_sort, in_place_dedup, itertools_sort_dedup_collect#313
Draft
KSXGitHub wants to merge 4 commits into
Draft
docs(plan): in_place_sort, in_place_dedup, itertools_sort_dedup_collect#313KSXGitHub wants to merge 4 commits into
in_place_sort, in_place_dedup, itertools_sort_dedup_collect#313KSXGitHub wants to merge 4 commits into
Conversation
…ect`, `itertools_sort_dedup_collect` Plan the three lint rules requested in #308, which proposes preferring the `into-sorted` and `into-deduped` crates. Problem 1 (`mut` elided by an owning transform) becomes two rules, as the issue asks — one for sorting, one for deduping: - `in_place_sort_after_collect`: an iterator collected into a `Vec` whose binding is `mut` only so a following `Vec::sort*` can run in place; prefer the owning `into_sorted*` method. - `in_place_dedup_after_collect`: the same shape for `Vec::dedup*`; prefer `into_deduped*`. Excludes non-consecutive dedup (`itertools::unique`), which has no `into-deduped` equivalent. Problem 2 (itertools codegen) becomes one rule, because the adaptors compose in a single chain that must be rewritten as a unit: - `itertools_sort_dedup_collect`: an `itertools` `sorted*` / `dedup*` adaptor chain terminated by a `Vec` `collect()`; prefer collecting first and applying the owning methods. Method-name mappings were taken from the crates' own source (`IntoSorted` / `IntoSortedUnstable` / `IntoDeduped`). README index gains a "Sorting and deduplication" section. The issue is still marked "under construction", so these files capture the design as currently framed; rule count, names, and scope can be revised in review.
Addresses two design flaws in the first draft of `in_place_sort_after_collect` / `in_place_dedup_after_collect`, raised in review of #308: 1. Detecting whether the `mut` is necessary required a borrow walk over every later use of the binding — costly and the wrong anchor. An in-place `vec.sort()` only type-checks when `vec` is already `mut`, so `mut` is implied by the trigger, and folding the call into the initializer is value-identical regardless of later uses. The rules now read only the two adjacent statements (collect-rooted `let` + the immediately-following in-place call), keep the `mut` as written, and leave removal of the now-redundant `mut` to rustc's `unused_mut`. 2. `collect; sort; dedup` was silently skipped, because each rule's "is the mut needed for anything else" check was failed by the *other* operation. The rules now cascade: each accepts a collect-rooted owning chain as its initializer and folds the in-place operation that immediately follows, so the sequence collapses to one `collect().into_sorted().into_deduped()` over successive `--fix` iterations, source order preserved. Updates the itertools rule's cross-reference and the README index entries to match. The `itertools_sort_dedup_collect` rule is unchanged (it never had the mut problem — it rewrites a single expression chain).
Sync this feature branch with master (now 0.0.0-rc.19). Notable changes pulled in: - Git hooks: commit-msg is split into per-check scripts under .githooks/commit-msg-hooks/ with shared helpers in .githooks/lib/. Installation is now just `git config core.hooksPath .githooks` (the redundant chmod was dropped, as the hook files carry the executable bit in git). - A new commit-msg check rejects commit subject lines longer than 72 characters, with a PERFECTIONIST_GIT_HOOK_ALLOW_LONG_SUBJECT tri-state opt-out. - Doc conventions: a declare_tool_lint! rustdoc block must describe behaviour, not pass machinery; per-site self-suppression examples use #[expect(perfectionist::...)] over #[allow(...)]. - excessive_inline_tests no longer misclassifies cfg(any(test, ...)) as test-only. No conflicts: this branch only adds the in-place-sort-after-collect, in-place-dedup-after-collect, and itertools-sort-dedup-collect planning files (plus their README index entries), none of which master touches. Co-authored-by: Claude <noreply@anthropic.com>
The `_after_collect` suffix named a mechanic that did no correctness work. Requiring the sorted/deduped `Vec` to come from an `Iterator::collect` was pure issue-scoping: the owning fold (`x.into_sorted()` / `x.into_deduped()`) is value-identical for *any* owned `Vec` the binding holds — a `vec![…]` literal, a `Vec`-returning call, a moved binding — as the old "Out of scope" bullet already conceded. Collect is also the *hardest* source (its element type is often fixed only by the `let` annotation the fix removes, forcing the `::<Vec<_>>()` turbofish), while the excluded sources carry concrete types and fold trivially. So drop the restriction and the suffix: - Rename `in_place_sort_after_collect` -> `in_place_sort` and `in_place_dedup_after_collect` -> `in_place_dedup` (files renamed to match). - Broaden the trigger to any by-value owned `Vec<T>` binding immediately followed by an in-place `sort*` / `dedup*`; reject `&Vec` / `&mut Vec` receivers, which the `into_*` methods cannot consume. - The cascade now falls out with no special "intervening owning calls" clause — a chain ending in `into_sorted()` is just another owned `Vec`. - Make the turbofish conditional: only threaded for a type-inferred initializer (a bare `collect()`), not for already-typed sources. - Update the README index entries and the `itertools_sort_dedup_collect` cross-references (the siblings are no longer "collect-rooted"). Source: #308. Co-authored-by: Claude <noreply@anthropic.com>
KSXGitHub
changed the title
in_place_sort_after_collect, in_place_dedup_after_collect, itertools_sort_dedup_collectin_place_sort, in_place_dedup, itertools_sort_dedup_collect
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Planning docs (
docs(plan), no Rust) for the lints proposed in the issue: prefer theinto-sorted/into-dedupedcrates over in-placeVecmutation and overitertools'sorted*/dedup*adaptors.Three new files under
planned-rules/, plus a "Sorting and deduplication" section in the README index:in_place_sort— an ownedVecbinding whose very next statement sorts it in place (Vec::sort*) folds into the owninginto_sorted*.in_place_dedup— same shape forVec::dedup*→into_deduped*. Excludesitertools::unique(non-consecutive; nointo-dedupedequivalent).itertools_sort_dedup_collect— anitertoolssorted*/dedup*chain ending in aVeccollect()→ collect-first owning form (the issue'sopt-level = 3codegen point).Design notes (documented in the files, open to review):
Vecbinding immediately followed by an in-placesort/dedup— acollect(), avec!literal, aVec-returning call, a moved binding. The owning fold (x.into_sorted()/x.into_deduped()) is value-identical for every ownedVec, so the earliercollect-only scope was dropped — that is thein_place_*_after_collect→in_place_*rename. A&Vec/&mut Vecbinding stays out of scope (theinto_*methods consume by value).mutanalysis.in_place_sort/in_place_dedupdeliberately do not analyse whether themutis necessary — an in-placesort/dedupalready requiresmut, the fold is value-identical, and rustc'sunused_mutremoves the now-redundantmut. They read only the two adjacent statements.collect; sort; dedupcollapses to onecollect().into_sorted().into_deduped()over successive--fixiterations, source order preserved — so the combined case is handled rather than skipped. With the source-agnostic trigger it needs no special-casing: a chain already ending ininto_sorted()is just another ownedVec.The issue is still marked "under construction", so these files capture the design as currently framed; rule count, names, and scope can be revised here.
Resolves #308.