Repository navigation
Conversation
Four closures passed to `any` or `map` chained two steps onto the item, so one adapter did both. Each now has one step per adapter, lifting the earlier one into a leading `map` or `filter_map`. `render_tree`'s length-minus-one also moves out of its `map`: the shortest path is a value worth a name, and the comment about keeping the last segment now sits on the line it explains. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Flag a closure passed to an iterator adapter where the chain of steps rooted at the closure's item is two or more long, so one adapter does all of them. The adapters in scope are the ones whose item enters by value and never comes back out, which is what lets a leading `map` mean the same thing. The chain is found by walking up from the item's occurrence, because what makes a parent a step is that the chain so far is what it is applied to. Three gates stand between the walk and a diagnostic: the item has to occur exactly once, every step moving into a `map` has to be liftable, and the chain has to be reached on every run of the closure. Only the planning file's chain trigger lands, over `Iterator` alone; its Status section records what is deferred and why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The help described the unary split alone: lift all but the last step, which stays with the adapter. A binary adapter lifts every step, which is what the liftability test was asked about, and its closure keeps only the accumulation. Following the old text there left a step behind that the rule had already found liftable. The wording now says what to leave rather than what to lift, which is right for `map`, `any` and `fold` alike. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
A mutation sweep over every guard the rule applies found nine it could remove without a fixture noticing. Each now has a case: - `inspecting` moved its chain out of a `println!`, where the anchor rejected it whatever the adapter table said, so admitting `inspect` to the table went unnoticed. - `option_map` and `another_trait_map` hold the two halves of the trait gate: the method has to resolve to a trait, and to the right one. - `named_in_a_nested_closure` and `accumulator_in_a_nested_closure` hold the walk's descent into a step's own closure. - `named_twice_in_a_fold` holds the occurs-once rule on the binary path, where the anchor does not already imply it. - `last_step_borrows` holds the unary adapter keeping its last step. - `destructured_without_a_borrow` holds the destructuring guard with no borrowing step to reject the chain first. - `callee_is_the_state` holds a step whose callee is another parameter. - `built_from_a_macro` holds the expansion guard. - `fold_with_a_let`, `scrutinee_fold`, `condition_fold`, `branching_fold`, `arm_fold` and `looping_fold` hold the position gate, which only the binary path consults. `consumer` drops `drop` for a sink of its own, so the baseline carries no `dropping_copy_types`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Measured by removing each and running the fixtures: neither mutant is distinguishable, and reading them says why. The closure-arity check cannot fire. Every adapter in the table fixes its closure's arity through its `FnMut` bound, so a closure that type-checks has exactly the parameters the shape expects. Its comment blamed an async closure's wrapped body, which is the anchor's business rather than the arity's. `evaluates`' block arm cannot answer no. The walk reaches a block expression only through its own block, so the comparison it made was always true; the arm joins the shapes that evaluate every operand they hold. The `if` arm's comment claimed the arm was the `if let` form, which `condition_fold` disproves by firing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Both were in the adapter table and neither could ever fire: they are `DoubleEndedIterator`'s methods, so the gate asking for `Iterator` turned them away. The planning file put them in scope and measured both forms agreeing, and they lift into `Iterator::map` like the rest because `Map` is double-ended wherever its iterator is. The gate now accepts either trait, and a case for every entry in the table holds the whole of it, `?` through a `try_fold` included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The walk took every `let` statement to run whenever its block did, so a chain inside a `let ... else` block counted as always reached. Lifting it then runs it for every item rather than only where the pattern fails to match. That compiles and answers differently, which is the one way this gate can be wrong without the compiler saying so. The arm now stops where the child is the `else` block rather than the initialiser, and `let_else_fold` fires without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
An adapter taking `&mut self` leaves its receiver positioned and usable, and a leading `map` moves it, so code going on to use the receiver stops compiling. The planning file measured that `E0382` and asked the rule to decline where it cannot tell; this one does not decline, so the Status section says so rather than leaving the note reading as implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
`master` gained the `refactor` commit this branch also carries, which PR 504 squashed as `169115f`, so the two sides made the same change and it reconciles without a conflict. It also gained a `Cow` return that `owned_as_conversion` now reports, and a planning file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Each was verified by running the split the diagnostic asks for.
`is_liftable` asked whether the *item* is a reference, and applied that
answer to every step. Only the first step is applied to the item; each
later one is applied to the step before it, so
`.map(|line| line.to_lowercase().trim().to_string())` fired and the
middle `map` of its split is `E0515`. It now asks about each step's own
receiver, which recovers what the planning file prescribes without the
region comparison that erased regions make impossible.
`?` lowers to a `match` on `Try::branch(operand)`, whose sole argument
is the chain, so it counted as a step the reader never wrote. The count
was one too high and a one-step closure fired. The walk now stops at a
desugared parent.
The position gate asked what stands above the chain and never what runs
before it, so a closure diverting first still counted as always
reaching the chain. `fold(0, |t, line| { if flag { return t; } t +
line.trim().len() })` fired while the same program as an `if` was
declined, and the lifted step then runs for every item where the folded
form ran it for none.
A lifted step runs in a closure beside the one the adapter keeps, and
two closures cannot both hold a mutable borrow of one capture, so a
step reaching one is `E0499` once lifted. The rule now declines where a
lifted step names a capture held any way but shared.
Dropped a claim that an `if let` reaches `evaluates`' `If` arm. It does
not: the walk answers no at the `Let` before it gets there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The planning file's second trigger. A predicate that is a conjunction does one test and then another, and each test can have its own adapter, which is the chain rule's statement reached by a different route. It needs a trigger of its own because the item occurs once per conjunct where the chain rule requires it once in all. A conjunct lifts into an adapter filtering with the same discipline rather than into a `map`, so `crate::adapter_discipline` carries that distinction for this rule and the `Option`-returning one to come. A set-shaped adapter lifts into `filter` and a prefix-shaped one into `take_while`; `all`, `position`, `rposition`, `partition` and `skip_while` match neither and are left alone, since filtering first flips what the first two answer and loses what the others count. Evaluation survives the split untouched, so this trigger needs none of the position care the chain's does. Two scope decisions beyond the planning file, both from running the rule over this crate: A conjunct that is a comparison is a bound rather than a question, and a conjunction of them is how Rust spells one test: `pos >= start && pos < end` asks whether a position is in a range. Splitting those reads worse than the conjunction, so a conjunction holding a comparison is left alone. Three of the five findings in this crate's own source were that shape; the other two are split here, since the rule is right about them. A conjunct not naming the item holds for every item or none, so it wants hoisting out of the pipeline rather than an adapter of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The planning file's third trigger, over the adapters the other two must leave alone. `filter_map` and its kin return an `Option`, so there is no conjunction to cut, and a leading `map` would hand the next adapter an `Option` rather than a value. What splits is the `Option` work inside: each combinator has an iterator adapter doing the same thing, and the split hands the work over. `and_then` goes to a `filter_map`, `Option::map` to a trailing `map`, `Option::filter` to a trailing `filter`, and a `bool::then` guard to a leading adapter with the value in a `map`, which is where the rule's name comes from. Which adapter it hands to depends on the outer one's discipline. A leading `filter_map` in front of a `map_while` would drop the very item that would have stopped the run, so a prefix-shaped adapter takes only the guard, whose target is a `take_while`, and a trailing `map`, which drops nothing. Where the argument may name the item differs by form, which cost a false positive before the fixtures caught it. A `then`'s value becomes a `map` over the item, so it may name it; every other form's argument becomes the closure of an adapter handed the stage before it, where the item is out of scope. The walk asking that question now lives in `crate::binding_uses`, since all three triggers need it and all three need it to enter nested closure bodies: a visitor left at its default filter never sees a closure argument naming the item, which is exactly the name a split would leave behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The chain trigger ran over iterators alone. It now also speaks about `Option`, `Result`, `Poll` and `ControlFlow`, each with the lift target that maps its own channel: `map` on a value, `map_err` on an error, `map_ok`, `map_break`, `map_continue`. The adapter table grew a family and a lift target to carry that, since the method names are shared across families and `Iterator::map` and `Option::map` want different things asked of them. A receiver's family comes from the trait a trait method resolves to, or from the type a inherent one is on. `Poll` carries no diagnostic item, which `get_diagnostic_name` says plainly, so it is identified by path. One scope decision beyond the planning file, again from running the rule over this crate. Each of these families carries one value rather than a stream, and only its mapping adapters are in scope: a fallible, defaulted or predicate-shaped adapter leaves the lifted step's result wrapped in what the adapter keeps, which costs a wrapper and sometimes a `mut` rebinding the folded form did not need. All five findings in this crate's own source were that shape, and none read better split, so `and_then`, `map_or`, `map_or_else`, `or_else`, `unwrap_or_else` and the `is_*_and` family stay folded. The planning file measured those splits as equivalent, which they are; what it did not weigh is what they read like. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
`master` gained `some_bool_comparison`, whose derive sits beside the two this branch added in the same auxiliary crate, and turned on `clippy::flat_map_option`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The Status section still said one trigger was implemented over iterators alone, which three rules and five families ago stopped being true. It now names which rule implements which trigger, which families they reach, and each way they are narrower than the file, and it keeps the file as the spec for what is left. The index entry loses its roster of adapter names. It had grown an internal contradiction, listing `all` and `position` as covered and then as excluded, which are the chain trigger's inclusions against the predicate trigger's exclusions; a reader wanting the roster has the Status section and the source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
`Itertools`, rayon's `ParallelIterator` and `pipe-trait`'s `Pipe`. None carries a diagnostic item, so each is identified by the path of the trait a call resolves to, which also means a crate the linted one does not depend on costs a lookup and answers nothing. Every claim the planning file makes about the three was re-measured against the real crates in a throwaway package outside this repo, since neither rayon nor the other two may become a dependency here. All held: each tabled split gives the value the file records, `map_ok` through `pipe` included, and each exclusion differs as claimed. `unique_by`, `tree_reduce` and `sorted_by_key` answer differently split; so does rayon's `filter`. rayon has no `scan`, no `map_while` and no `rposition`, and the `find_map_any` / `find_map_first` / `find_map_last` trio is there in place of `find_map`. The `Send` exemption is implemented, because rayon makes it necessary: `.map(|s| Rc::new(*s).len())` compiles and `.map(|s| Rc::new(*s)).map(|r| r.len())` is `E0277: Rc<&str> cannot be sent between threads safely`, measured. A lifted step under a parallel adapter has to produce something `Send`. `map_ok` is why a lift target is worth naming per channel: its closure is handed the `Ok` inside a `Result` item, a channel nested one level inside the iterator's own, so a lifted step goes into `map_ok` rather than `map`. `fold_ok` is binary over that same nested item and `fold_while` over the iterator's own, and they lift accordingly. rayon and `pipe-trait` get stub auxiliaries. `itertools` gets none: the driver's sysroot ships the real crate, because rustc depends on it, and a stub of that name collides with it as `E0464`, so the fixture reaches the real one through `rustc_private`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The predicate trigger ran over iterators alone. The planning file's discipline table also puts `Option::filter`, `Option::is_some_and` and rayon's `filter`, `any` and `find_first` under the set discipline, so those lift into a leading `filter` too. What stays out is what the table leaves without a lift target. Filtering before `Option::is_none_or` makes a value that failed the first test vacuously fine, the way it does before `all`. `Result` has no filtering adapter at all, so `is_ok_and` and `is_err_and` stay folded however they are written, and a fixture holds each. rayon's trait carries no diagnostic item, so it is identified by path, which also means a crate the linted one does not depend on costs a lookup and answers nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
A 68-mutation sweep over the three rules caught 56 and left eleven guards with no fixture to notice their removal. Each now has one. Two were regressions in the coverage rather than gaps from the start. `nothing_diverts_first`, added to stop a closure that leaves before the chain, now declines `looping_fold` and `let_else_fold` before `evaluates` or the `let`-`else` arm is reached, so both of those guards quietly lost their only witness. `loop_after_the_chain` and `let_else_diverting_after` put the divergence after the chain, which leaves each guard the one doing the declining again. A fix that silences a fixture can leave an unrelated guard untested with nothing going red, which is what the sweep is for. The rest: a shared capture reached by a lifted step, which two closures may both hold; a step whose result cannot cross a thread, which only a parallel adapter asks of it; rayon's `filter` and a parallel conjunction; `and_then` and `then_some` on receivers that are no `Option` and no `bool`; a `filter_map` of a trait's own; and `Iterator::map` holding `Option` work, which the chain rule splits and the other one leaves. The rayon stub gains the `filter` its exclusion needs to be testable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Each was demonstrated by running the split the diagnostic asks for, and
the first one changes an answer without the compiler saying so.
`find_map` and `filter_map` were both tabled as set-shaped, and a
set-shaped adapter was told it could take a trailing `filter`. But
`find_map` yields one value, so the only trailing `filter` it has is the
`Option`'s: `find_map(|line| parse(line).filter(positive))` gives
`Some(7)` and the advised `find_map(parse).filter(positive)` gives `None`,
because the search had already settled on the item the trailing filter
then rejects. Whether an adapter yields a stream is a second question the
discipline does not answer, so the table answers it, and a one-value
adapter now declines the two forms that lift work to its right. The
planning file is where this came from: it measured the `and_then` row for
`find_map` and generalised to the rest.
`nothing_diverts_first` listed the node kinds that leave a closure and so
missed every other way out. A `panic!`, a `std::process::exit`, a call to
a `-> !` function and a `loop {}` all leave, and a chain behind one fired:
the folded form exited 7 where the lifted one panicked on an item it never
used to reach. What leaves is now read from the type, `!` being the
answer, which also restores the invariant the module claims, since the
same program written as an `if` whose branch holds the chain was already
declined.
`is_liftable` took a borrowing result to be the receiver's to answer for.
`s.max(&String::from("m")[..])` returns the shorter of two lifetimes and
the shorter one is a temporary the closure made, so the split is `E0515`
although the receiver is a reference. A step is liftable only where
nothing else it was handed carries a region either.
The predicate rule had no capture guard where its sibling has one,
although its split makes two closures the same way: two tests reaching a
capture held mutably is `E0499`. The guard moves to
`crate::exclusive_captures` and both rules read it, the predicate rule
asking whether more than one test reaches one.
`then_some` builds its value whether the guard holds or not, so the split
does less work rather than the same, which the shipped claim now says.
The direction only ever drops a side effect.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
The chain trigger described its split and left the reader to make it. It applies one now, one adapter per step. Reusing the item's name was written first and thrown away: only the first step is handed the item, so a `|line|` above a `trim` is bound to a length, and `cargo dylint --fix` would have planted that across a codebase. Each step is named as the function it already is instead, which is the form the `// Good:` fixtures already carried and needs no binding at all. A method step takes the path form only where the method's self type has the borrow depth and the base type the receiver has. That is what keeps an item of `String` away from the `str::len` it reaches through a deref, where the path would ask for a `&str` the adapter is not handed. Shapes rather than types, because a signature's lifetimes are bound where a receiver's are not, and erasing regions does not reach a bound one. Where a step cannot be named, the first falls back to a closure over the item, whose name is the one it had, and a later one to a `/* name */` placeholder. That carries `Applicability::HasPlaceholders`, so the text is shown and never applied. Measured over the ui suite: 86 suggestions, 15 of them holding a placeholder. The autofix fixture gains five chain shapes, including the `String` item that must decline the path form and the `fold` that is left to the help text, and `cargo dylint --fix` leaves the crate compiling, every rewrite landing on the text the Good fixtures spell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
A header saying "nor does an arm of a `match`" or "an `if` condition runs
too" reads only in the order the file happens to sit in, and says nothing
where a diff shows it alone. Each one now carries its own subject and its own
reason, so none depends on a neighbour.
Swept the whole suite rather than the lines marked: 20 headers leaned on one,
across every file but the proc-macro fixture. The leaning words were `nor`,
`too`, `likewise`, `for the same reason`, `the same way`, and an explicit
`above` and `below`. Five of them were mine from the round that split the
grouped headers, which replaced one kind of coupling with another.
Left as they stand are the ones where the same words point inside the case
rather than out: steps `below` the one being described, `both` closures a
split writes, a value read `either way`.
One header also described items that are not its own, naming an `exit`, a
`-> !` call and a `loop {}` while sitting on the `panic!` case, two of which
are separate fixtures further down. It describes the `panic!` alone now, the
mechanism they share being that what leaves is read from the type.
No baseline moved, a `.stderr` recording `LL:CC` rather than a line number.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
`orx-parallel` is a rayon alternative with the same combinator shape, and tabling its method names would have been a copy of its API. It renamed its trait from `ParIter` to `Par`, dropped `take_while` and `map_while` and gained `fold`, all across one major release, so a table would have stopped matching with no error to show for it. What did not change is the shape each closure is declared with, and that is what the rule actually needs. `signature.rs` reads the `Fn` bound on the closure an adapter takes: `Fn(Self::Item) -> Q` hands the item over, so the adapter is the chain trigger's; `Fn(&Self::Item) -> bool` lends it, so it is the conjunction trigger's and a leading `map` would change what it sees; and `Fn(&mut B, Self::Item)` says the item is the second parameter because the first is state. All five of the chain trigger's questions about an adapter fall out of that one read, where the other families answer them from a table apiece. Two things a signature cannot carry stay by name, and both ask the trait rather than assert: which adapter stops a run rather than sieving it, since a prefix-shaped one is declared exactly as a set-shaped one, and the lift target, so a trait declaring no `map` offers no split. `crate::command_extra` is where asking the trait rather than carrying a version table comes from. The trait is identified by crate and name, under either name it has carried, rather than by a module path: it moved module at the rename too. Verified against the real crate, not only the stubs. Building 4.1.1 showed that a lifted step's result need not be `Send`, which rayon requires and which I would have assumed by analogy, so the family is not in `sends_between_threads`; the probe was checked against a deliberate error to confirm it could fail. `hands_the_item_over` now asks the signature too, since `orx-parallel`'s `any` lends the item where `Iterator`'s hands it over and the name is the same either way. A stub per generation, with a fixture apiece: the 4.x trait reads `fold`'s item at parameter 1, and the 3.x trait reads a `map_while` and a `take_while` that no longer exist, the latter taking the prefix discipline because the trait declares one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Mutation-sweeping the 28 guards the `orx-parallel` family added left 11 unheld, and the first was a correctness defect the fixture's own comment was hiding. `reduce` is declared `Fn(Self::Item, Self::Item)`. Reading the first parameter that is the item made it look like `fold`, whose first parameter is an accumulator no lift touches, so the rule built an adapter for it and advised lifting every step into a leading `map`. That applies the step to the second parameter as well, which is a different program, and the result does not type-check. `max_by`'s `Fn(&Self::Item, &Self::Item)` is the same shape. So a closure carrying the item in more than one parameter is now declined, and the fixture that asserted silence for `reduce` says why it is silent: it had been mentioning the item twice, which the occurs-once gate declined before the shape was ever reached, and it fires once reshaped to mention it once. Two further guards turned out to do nothing, each found by a missed mutation rather than by reading. The discipline asked the trait whether it declares `take_while`, but the only branch that could use the answer already required the method to *be* `take_while`, so both branches returned `Set`; deleted, along with the claim its doc comment made for it. The family's arm in the adapter table sat behind an early return that had already answered, so the match is now the only dispatch. A third, the discipline's parameter-count clause, duplicates the `let [parameter] = body.params` its caller performs. The stub was also a narrower spec than the real trait. Diffing the closure shapes it declares against `Par`'s found one missing, `Fn(&Self::Item, &Self::Item)`, carried by `max_by` and `min_by` in both generations — exactly the shape two of the missed mutations needed. The older stub had gone the other way, declaring eight methods no fixture calls, which are read by nothing; it is trimmed to the six its fixture exercises. Witnesses added for the gaps: a `Par` of the author's own in another crate, a generation of the trait declaring no `map`, a `map` whose closure is a conjunction, an `any` binding the lent item mutably, and `filter_map` and `flat_map`, which the stub declared and no fixture called. 9 of the 11 are now pinned, with the new guard and the restructured dispatch. Four stay untested on purpose, three because no faithful stub of the trait tells the mutant apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Coverage over the rule found five lines in `shape()` that no fixture reached: the arms rendering a path under `bool`, `char`, a signed integer, an unsigned integer and a float. The rewrite had only ever been measured on `str` and on structs. The new fixture is one case per primitive, and it took the discipline the fixtures' header states to write: the Good counterparts have to be the form the rule actually advises, not the point-free form they look like they should be. `usize::to_string` takes `&self`, so it is not `fn(usize) -> String` and `map` cannot be handed it. The rule is already right about this -- its borrow-depth reading declines the path and falls back to a placeholder closure -- but a hand-written `.map(usize::to_string)` does not compile, which is the kind of advice nobody can take that these fixtures exist to catch. So `abs` renders under `i32` and `f64`, which take their receiver by value, and `to_string` keeps a closure in every case. Three of the five get no rewrite at all, both of their steps borrowing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Coverage found `poll`'s `map_err` entry uncovered and the `_ => return None` of three family tables never taken. The first is why the method-name audit was not enough: it grepped the fixtures for `.map_err(` and found it, on a `Result` receiver rather than a `Poll` one, so the entry read as exercised while the `Poll` arm had never run. Grep the call on the family's own receiver type. `Poll`'s error channel now has its own case, and a method each family's table does not name is called so the fallback is taken: `is_ready`, `is_break`, and `itertools`' `unique_by`. The last is the better witness of the three, because the table's own comment names it among the adapters left out for being lent the item rather than handed it, and nothing had held that. `adapter::pipe()`'s fallback stays uncovered on purpose: the table lists all nine methods `pipe_trait`'s `Pipe` declares, so no call reaches it. That crate's `src/tests.rs` has test functions named `pipe_ref_lifetime_bound` and `pipe_mut_lifetime_bound` that read like two more methods in a grep over the whole crate; they are not, and `src/lib.rs` is what to check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
A fixture per position the chain can be written in, each with a call before it so the ordering is what would decline the lift. It closes no coverage hole, and saying why is the point of keeping it. `anchoring::preceding` has an arm per parent expression, and coverage reports its *fall-through* regions uncovered rather than the arms themselves: the `MethodCall` arm runs, but never with the chain outside the receiver; the `Binary` arm runs 24 times, never with the chain on the left. Reading those zeros as dead arms would have been wrong, and writing fixtures for the complements was the way to find out which reading holds. Neither a chain in an argument nor one in a tuple, an array, an indexed base or a left operand reaches `preceding` at all: the position gate declines a chain that is not what the closure answers with, before the ordering is ever consulted. So those regions are a property of the gate above them, not holes a fixture can fill, and whether the arms earn their keep is a question for the mutation sweep rather than for coverage. What the fixture does pin is the behaviour in twelve positions, so a change that made any of them fire would be caught. One case was mislabelled `Bad` on the way here and did not fire. A control case added to the same file fired on the first try, which is what showed the file was wired and the case was wrong rather than the harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Running the rule over `parallel-disk-usage` produced a rewrite that does not
compile, and `cargo fix` applied it because the suggestion claimed to be
machine-applicable:
.filter_map(|file| Sys::canonicalize(file).ok())
.map(Sys::canonicalize).filter_map(Result::ok) // E0631
The iterator yields `&PathBuf` and the function takes `&Path`. The call site
reached the parameter through `PathBuf`'s `Deref` impl; handing the function
to `map` performs no such conversion, so the rewrite asks it for a type it
does not take.
`point_free` had been careful about exactly this for a method, comparing the
self type against the receiver, and its doc comment claimed a call needed no
such care because it "hands its callee exactly what the chain evaluated to".
A call coerces its argument, so that was wrong, and the `Call` arm checked
nothing at all.
What distinguishes the two cases is in the adjustments on the argument, which
is what finally settled this after two wider attempts went too far. A
`&str` handed to a parameter of `&str` carries `Deref(Builtin)` then
`Borrow`, which is a reborrow and changes nothing; the `&PathBuf` carries an
`Overloaded` deref between them, which is the `Deref` impl being called. So
an overloaded deref is what declines the path form, and a builtin one is not.
Comparing the parameter and argument types directly was tried first and is
worse: a signature's late-bound lifetime does not compare equal to whatever
the argument was written with, and the shapes `point_free` already computes
cannot name a `()` or a type parameter, so both readings declined rewrites
that were correct.
Pointer coercions and `Pin` derefs decline for the same reason.
Verified on the repository that found it: the rewrite is now
`.map(|file| Sys::canonicalize(file))`, the crate compiles, and its own
`test.sh` gives the same 42 passing suites as the pristine checkout, with the
one `fs_errors` failure that refuses to run as root in both. Both of that
crate's fixes now apply, where before the unsound one made `cargo fix` revert
the sound one with it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
`fold` and its kin had a help note and no suggestion, because `fix` declined
every adapter whose chain leaves whole. Nothing blocked it beyond the work:
the shapes with a rewrite replace the closure outright, and this one has to
edit it.
So it is edited. The accumulator parameter and the rest of the body are the
reader's and stay as written; only the item's parameter and the chain inside
the body become the lifted value:
lines.fold(0, |total, line| total + line.trim().len())
lines.map(str::trim).map(str::len).fold(0, |total, /* name */| total + /* name */)
The chain is located in the body by span rather than by searching, which the
occurs-once gate is what makes sound: the item is mentioned exactly once, so
the chain rooted at it appears exactly once too. That is also why the splice
handles a block, a `match` and an `if` body without knowing anything about
them, and why a non-closure argument such as a struct literal spanning lines
survives untouched.
The name for the lifted value is the reader's, the item's own name having
described what the chain was handed rather than what it produced, so a
placeholder stands in and the rewrite is `HasPlaceholders`. The fixer
therefore never applies it, which is the property that keeps a `/* name */`
out of anyone's source, and `tests/splittable_adapter_closure_autofix.rs`
already held a `fold` case that pins it: dropping the applicability line makes
`cargo fix` apply the rewrite and the crate stops compiling.
A piping method still has no rewrite, its first step going into its own
method rather than into the lift target.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Running the rule over `pnpm/pnpm` found two defects in the rewriter. Neither
was reachable by this suite, and the first had been silently wrong in this
repo's own fixtures.
**The adapter's other arguments were discarded.** The assembler wrote the
method and the lifted step and nothing else:
written.push(format!("{}({step})", call.method));
`map_or` and `map_or_else` are tabled as `defaulted`, which says the closure is
their *second* argument and the first is a default. So the default was deleted:
options.throttle_progress.map_or(
if append_only { Duration::from_secs(1) } else { Duration::from_millis(200) },
|ms| Duration::from_millis(u64::from(ms)),
)
options.throttle_progress.map(u64::from).map_or(Duration::from_millis)
rustc answers `E0061: this method takes 2 arguments but 1 argument was
supplied`. The whole-chain rewrite added for `fold` already iterated
`call.arguments` correctly, so both paths now share that as `call_handed`.
What makes this worth saying twice: four suggestions in `ui/` were wrong, and
the `// Good:` counterpart written by hand beside each one was right all along
-- `header.map(str::trim).map_or(0, str::len)`. The baselines had been blessed
from the rule's own broken output, which is exactly the failure a `.stderr`
comparison cannot catch on its own.
**The path form named types the file does not import.** `point_free` writes
`Owner::method` from the unqualified name, which compiles only where that name
resolves:
.filter_map(|value| value.to_str().ok())
.map(HeaderValue::to_str).filter_map(Result::ok) // HeaderValue not in scope
Every type the earlier repositories named was `str`, `Result` or a primitive,
all always in scope, so this never showed. The receiver's owning ADT is now
asked whether its own name resolves where the path is written: the modules
around the call site are searched for an import of it or a definition of it,
innermost first, with the prelude's own types spared. A glob import is not
followed, so a name only a glob brings in reads as absent -- a downgrade and an
import offer nobody needed, where the opposite mistake would be a rewrite that
does not compile.
Where the name is absent the rewrite is still the advice, offered at
`MaybeIncorrect` alongside the `use` it wants, at the span rustc computes for
its own import suggestions. A fully qualified path was considered and rejected:
`.map(reqwest::header::HeaderValue::to_str)` is longer than the closure it
replaces, and this rule exists to make the call site read better.
Both are pinned. The `map_or` suggestions change in `ui/`, and the import case
needs the autofix test rather than a baseline, because applicability is the
property a `.stderr` cannot show: removing the scope check makes the fixer
apply the rewrite and the crate stops compiling with `E0433: cannot find type
`Wrapper` in this scope`.
Self-lint caught the new `resolves_here` chaining two steps in one closure,
which is the rule's own complaint; flattening the nested `any` over the module
items answers it and reads better.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
| // The argument's own type, before the call coerced it, against the | ||
| // parameter's. They differ exactly where the call performed a | ||
| // conversion that handing the function over will not. Regions are | ||
| // erased because a signature's are bound where an argument's are | ||
| // whatever it was written with, and the question is only whether | ||
| // the types agree. |
There was a problem hiding this comment.
Is this comment necessary?
There was a problem hiding this comment.
Partly. One of its three sentences was a second copy of the body, and it is gone in ecc6942; the other two I'd keep.
The one that went:
The modules around
atare asked for an import of it or a definition of it, innermost first.
That is hir_parent_iter and names_it, line for line — CLAUDE.md's rule about a passage that only restates the code beneath it. And "innermost first" was worse than redundant: the walk ends in .any(...), so the order it visits modules in cannot change the answer. A reader who believed the comment would go looking for a precedence the function does not have.
What stays, and why each earns it:
/// Whether `definition`'s own name resolves to it where `at` sits.
The function name says resolves_here; it does not say own name. That is the whole question — not whether some path reaches definition, but whether its last segment alone does, which is what decides between a point-free rewrite and an import offer.
/// A glob import is not followed, so a name only a glob brings in reads as
/// absent -- which costs a downgrade and an import offer the reader does not
/// need, where the opposite mistake would cost a rewrite that does not
/// compile.
The code can only show that globs are not followed. It cannot show that this was chosen, nor which way the asymmetry runs — a false "absent" costs a redundant use line in the suggestion, a false "present" costs a rewrite that does not compile. Without the paragraph the next reader cannot tell the omission from an oversight, and the ItemKind::Use arm in names_it is sitting right there to be "fixed".
Generated by Claude Code
A review asked whether `resolves_here`'s doc comment is necessary. Partly: one of its three sentences was a second copy of the body, and the body says it better. "The modules around `at` are asked for an import of it or a definition of it, innermost first" is what `hir_parent_iter` and `names_it` read as, line for line. "Innermost first" was worse than redundant: the walk ends in an `any()`, so the order it visits modules in cannot change the answer, and a reader who believed the comment would look for a precedence the function does not have. The summary line stays, for the word the function name leaves out: the question is whether `definition`'s *own name* resolves to it, not whether some path reaches it. So does the paragraph on glob imports, which records a decision with a cost on each side -- the code can only show that globs are not followed, not that leaving them out was the cheaper mistake. The Status section of the planning file goes with it, having gone stale when the whole-chain rewrite landed: it still denied an autofix for a chain under `fold`, and said nothing of the import offer that accompanies a step whose name does not resolve. Both are described now, and the piping methods remain the one shape with no rewrite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
A rewrite naming two unimported types offered two `use` lines through `span_suggestions`, whose list rustc renders as alternatives to choose between. Both are required: taking either one alone leaves the other path unresolved and the rewrite uncompilable. One `span_suggestion` carrying every line says what is meant, and the rendered note shows the difference -- two blocks separated by a rule before, one insertion of two lines after. The note's own plural wording had already said so, which is the part worth noticing: the text and the suggestion disagreed, and nothing was checking either. Only the autofix test reached the plural arm, and `MaybeIncorrect` keeps `cargo fix` away from it, so no `.stderr` anywhere held the rendering. A fixture with two steps over two unimported types now does. A second fixture puts a chain inside a module that imports the type its own steps name. `resolves_here` looks for the import in each module enclosing the chain and then at the crate root, and every fixture so far sat at the root, so the enclosing-module arm had never run -- the rewrite could have been reading only root imports and no test would have objected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Implements all three triggers of
planned-rules/splittable-adapter-closure.md, as one rule, over nine families. The planning file survives the PR because some of it is deferred.What fires
filterortake_while, by disciplineOptioncombinator or abool::thenThe triggers are asked in that order and the first finding wins, so a body that is
Optionwork gets the split handing each half to its counterpart, and any other body under the same adapter falls through to the chain's leadingmap.Families reached:
Iterator,DoubleEndedIterator,Option,Result,Poll,ControlFlow,Itertools,ParallelIteratorandPipe. The adapters in scope are the ones whose item enters the closure by value and never comes back out, which is what lets a leading lift mean the same thing. The tables are lists rather than type queries because the condition is about an adapter's contract, which no type exposes.A conjunction is rewritten for you; the other two shapes are described. A conjunction splits one way and one way only, one adapter per test in the order they were written, so there is no text to choose. A chain and a guard-and-value body each have more than one reasonable text, so they stay advice for now.
The rewrite keeps the closure each test was written in rather than reducing it to a path. A path asks more of a test than the split does, and
clippy::redundant_closure_for_method_calls, which this workspace enables, is the lint that reduces the ones that can be reduced. Measured acrossclippy::all,pedantic,nurseryandrestriction, nothing asks for the closure form back, so the two fixes compose in that order and stop.tests/splittable_adapter_closure_autofix.rsruns the real fixer over seven shapes and compares the whole file.What it flags
Beyond the three shapes above, these are the ones whose being in scope is not obvious. Every split below was compiled and compared against its folded form in a scratch crate.
filter_map,find_mapormap_whilelines.filter_map(|line| parse(line.trim()))OptionOkchannelitems.filter_map_ok(|text| text.trim().parse().ok())partition_maplines.partition_map(|text| classify(text.trim()))Eitherrather than the itemOptionandResult's non-mapping adaptersvalue.and_then(|text| parse(text.trim()))Pipe's eight borrow-handing methodsvalue.pipe_ref(|text| text.trim().len())pipehands a value where these hand a borrowlines.filter(|line| wanted(line) && line.len() > 3)spans.filter(|span| span.offset >= start && span.length < limit)lines.map(|line| line.trim_start_matches("# ").len())Two of those needed more than a table entry. The piping methods perform their conversion once, at the head of the chain, so the first lifted step keeps the method and every step above the head takes a value; that also settles the receiver-move question, since such a split borrows the receiver exactly where the folded form did. And liftability reads the method's declared signature rather than the types the call instantiated, because the regions there are erased where the signature still names them:
str::strip_prefix<P>(&self, prefix: P) -> Option<&str>shares none of them with its argument, whereOrd::max(self, other: Self) -> SelfsharesSelf.Applying the rule to the crate's own source is the honest test of whether the diagnostics are wanted. Every site it flagged takes the split rather than an
#[allow], and KSXGitHub/perfectionist#506 carries them one per row. The two endingand_then(Result::ok)take theOptionsibling of thefilter_map(Result::ok)shapesrc/cargo_manifest.rsalready carries, which is the rewrite this rule was distilled from.What it leaves alone
lines.map(|line: String| line.trim().len())mapends before the borrow is used:E0515Copyitem that is not a referencerows.map(|row: [u8; 4]| row.as_slice().len())lines.map(|line| pick(line).into_iter().count()).map(|s| if flag { s.trim().parse().unwrap() } else { 0 })pairs.map(|pair| pair.trim().to_owned() + pair)lines.map(|line| line.trim().len() + bump(&mut seen))positions.any(|pos| pos >= start && pos < end)lines.filter(|line| ready && line.len() > 3)lines.filter(|line| wanted(line) || line.is_empty())lines.filter(|line| line.trim().is_empty())mapchanges what it and every later adapter seeslines.all(|line| wanted(line) && line.len() > 3)&mut selflet empty = lines.any(..); lines.count()lines.filter_map(|line| staged!(line))Result::map_or_else's error closureoutcome.map_or_else(|text| text.trim().len(), |value| value)lines.filter_map(|line| line.strip_prefix("# ").map(str::len))Only the third and fourth rows can be wrong without the compiler saying so, which is where the care went: the rest are a compile error the reader would hit, so declining them costs a diagnostic rather than trust. A parallel adapter adds one more of the compiling kind, requiring what a lifted step produces to be
Sendwhere the folded form never moved it between threads.Narrower than the planning file
The narrowings, each measured against this crate's own source, are recorded in the planning file's
## Statussection rather than repeated here. The crate's own source obeys the rule with no#[allow]anywhere.Its
## Deferredsections keep their text as the active spec: the unanchored chain, and arrays behind a purity test.How it was checked
Beyond
just all, two procedures ran against this branch.Eight mutation passes. Every guard was removed in turn and the fixtures re-run, then again after each round of fixes added guards of its own. What that found, in order of how much it mattered: a correctness defect in
always_evaluated; a defect in the emitted help; two unreachable guards removed; two dead table entries made live; and fixtures for the guards no test held. The last pass, over the sixteen guards the recombination and the widening added or changed, pinned fifteen first time and found one gap: the guard-and-value trigger'sIteratorcheck, which every witness in the suite reached only through a family lookup that had already turned it away. rayon'sfilter_mapis the witness that reaches it. Left untested on purpose, because no compiling program tells the mutant apart:steps' "the receiver is the chain" check, which the argument check and the occurs-once gate make redundant, andpreceding's fallback for an operand its search cannot find, which every caller has already answered for.Six rounds of correctness review, rounds 35 to 40 of this repository's loop: 44 findings, all valid, none rejected. Each was reproduced against the built driver, and every claim that a suggested rewrite does not compile was confirmed by compiling it. The largest classes were the lift target's reference depth and receiver, a missing liftability check in the guard-and-value trigger that the chain's has had all along, and documentation claims the code does not make. The last round could not break the round before it.
Every Bad fixture case has the Good one the help asks for, 92 pairs of them, so the advice is compiled rather than asserted. Two defects in the suggested forms came out of writing them: a
map_or_elsesplit that dropped the step it was meant to lift, and atry_for_eachsplit that became afor_eachand lost theOption<()>its folded form returns. Both were invisible to a suite that only asks where the rule fires. A generated probe outside the tree now runs each pair's two forms over the same inputs and compares, and asserts that the pair returns the same type.The applied rewrite is run through the real fixer, over a fixture whose expected form is a file beside it, so the rewrite is pinned down to the brackets and nothing is reverted for failing to compile. That comparison is also the only place applicability is observable: a
.stderrrenders a suggestion whether or notcargo dylint --fixwould apply it, so the ui sweep cannot tellMachineApplicablefromUnspecified. Mutation-checked by making the comment guard always answer false, which the fixer run catches and the ui sweep cannot.What it does not check is that the split answers what the folded form did. An oracle inside the fixture does catch that, and was measured at two extra
cargo testinvocations, 0.55s to 1.29s for this rule alone. One such fixture per rule with an autofix is what does not scale, so it is dropped pending a mechanism that would: one project holding every rule's inputs, fixed in one pass, with the result cached.The chain trigger rewrites too, naming each step as the function it already is rather than inventing a binding:
lines.for_each(|line| record(line.trim().len()))becomeslines.map(str::trim).map(str::len).for_each(record). Reusing the item's name was built first and rejected, since only the first step is handed the item and a|line|above atrimis bound to a length. A method step takes the path form only where the method's self type has the borrow depth and base type the receiver has, which keeps an item ofStringaway from thestr::lenit reaches through a deref; such a step falls back to a closure over the item as the first step, and to a/* name */placeholder above that, carried asApplicability::HasPlaceholdersso it is shown and never applied.That brings the ui suite to 86 suggestions, 15 of them holding a placeholder. Still advice: the guard-and-value trigger, a chain under an adapter taking the whole chain, and a piping method's leading step.
Changes outside the rule
pub(crate):adapter_discipline,binding_uses,exclusive_captures,extra_referenceandreceiver_move, plusborrowsandbinds_mutablyincommon. Each is read by more than one trigger.ui/auxiliary/rayon_stub.rsandui/auxiliary/pipe_trait_stub.rsstand in for crates this repository does not depend on;rayon,itertoolsandpipe-traitthemselves were verified in scratch crates outside the tree.ui/auxiliary/proc_macro_synth_binding.rsgains derives that synthesise each trigger's shape.ui-toml/overly_long_method_chain/default/fixture.rsis the one exception that cannot go there, since its#[allow(perfectionist::splittable_adapter_closure)]names a lint that does not exist until this PR; itschains_in_closurescase exists to show an inner chain of three.Iterator, the extension traits, the conjunctions, which comparisons read as one test, the guard-and-value shapes, and one proc-macro regression file.// Bad:fires,// Good:is what the help asks for and stays silent,// Not flagged:is neither, and every Bad is followed by its Good.refactorcommit splitting four of the crate's own chained closures landed separately as KSXGitHub/perfectionist#504.Verification
PERFECTIONIST_CARGO_LOCKED=true just allpasses on the branch tip:fmt --check,build,doc,lint,test(269 unit tests plus 97 ui and every ui-toml fixture) andself-lintunder-D warnings. Every commit was gated on the ui suite before it was made.🤖 Generated with Claude Code
https://claude.ai/code/session_01A65ivJxFWzYSVxtGzcH2PX
Generated by Claude Code