Let the narrower behavior win where both landed - #48
Merged
Conversation
The catalog is not committed and git does not track an empty directory, so `infact-packs/rust-std/api/` is absent on a fresh checkout — and `--output` does not create the path it is given. The documented command therefore failed on the one machine state it exists for, with a bare `No such file or directory` that names neither the path nor the reason. Found by running it on a fresh checkout of merged main rather than by reading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
`Option::and_then` is `match self { Some(x) => f(x), None => None }`, and the
hole swallows whatever a narrower way of consuming an `Option` puts there. So it
matches `map`, and `filter`, and `ok_or`, and says less about each than they do
about themselves. On clippy's `manual_map` test it landed on fifteen of the same
lines `Option::map` did; every one of those was a second, weaker finding on code
that already had a better one.
`is_reportable` was the existing answer to a behavior that describes too much,
and it does not reach this: `and_then` names two variants, so it passes the
anchor floor while still subsuming everything narrower. What separates them is
not how much either names on its own but how they stand to EACH OTHER, and a
form used as a pattern already answers that — a form that matches another
behavior's form accepts everywhere that one does and elsewhere besides.
Asked per placement rather than per pack, which is the part that matters. Being
broader is not being wrong: code that really does reimplement `and_then` should
hear about it, and a pack-wide rule would lose that. It is only grounds for
standing aside where something narrower has already landed.
Measured. On the 13-file clippy corpus, unchanged: 26/201, 36 reported, 36/36
on-target — this removes nothing that was right. On a wider sample of 18
`manual_*` lints fetched for this, reported findings fall from 59 to 44 and
off-target from 43 to 28, with all ten on-target findings kept. The on-target
share goes from 16/59 to 16/44.
What is left off-target is not this problem. Eleven are `manual_unwrap_or_default`
reporting `unwrap_or`, which is correct and merely less sharp than the API clippy
names. Eleven are `manual_filter` reporting `and_then` because `Option::filter`
derives no behavior at all — nothing narrower exists to win, and that is a recall
gap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
`Iterator::fold` reported that no implementation was found. It has one: a loop that accumulates, right there in the trait. What went wrong is that `max_by` and `min_by` each declare their own `fn fold` helper inside their bodies, the container was carried down into those bodies, and so three callables answered to `Iterator::fold`. The resolver refuses a name two callables answer to — correctly — and a method with a perfectly good body fell out of the pack. The container stops at a function boundary now. A function declared inside another function's body is a local helper, not a method of whatever type surrounds them both. Measured on core: 261 behaviors to 263, and nothing lost. `Iterator::fold` and `Iterator::count` derive where they did not. WHAT I PREDICTED AND DID NOT GET: that the whole family built on `fold` would follow it — `sum`, `max`, `min`, `last`, `reduce`, `product`, `nth`. None did. They fail for their own separate reasons, which are not this one and are not yet known. That is the sixth prediction in this file's history to be wrong about what a correct fix would reach, and it is recorded here because the tally it came from is only useful if the reach is measured rather than argued. The clippy corpus is unchanged at 26/201, 36/36 on-target. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017ghYbUJFcVfkKYacbTVJsR
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
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.
Follow-up to the merged idiom work, from measuring precision off the 13-file
corpus for the first time.
The problem
Option::and_thenismatch self { Some(x) => f(x), None => None }. The holeswallows whatever a narrower way of consuming an
Optionputs there, so itmatches
map,filterandok_orand says less about each than they say aboutthemselves. On clippy's
manual_maptest it landed on fifteen of the samelines
Option::mapdid — fifteen second, weaker findings on code that alreadyhad a better one.
This is the failure mode
notes/todo.txtrecords twice, most recently asOption::is_none_orfiring 1,390 times across five hundred crates: "both werecorrect, both subsumed every narrower behavior".
Why
is_reportabledoes not catch itIt is the existing answer to a behavior that describes too much, and it cannot
reach this one.
and_thennames two variants, so it clears the anchor floorwhile still subsuming everything narrower. What separates the two is not how
much either names on its own but how they stand to each other — and a form
used as a pattern already answers that. A form that matches another behavior's
form accepts everywhere that one does and elsewhere besides.
Per placement, not per pack
The part worth reviewing. Being broader is not being wrong: code that really
does reimplement
and_thenshould hear about it, and a pack-wide rule wouldlose that. It is only grounds for standing aside where something narrower has
already landed.
tests/subsumption.rspins both directions.Measured
manual_*sampleNothing that was right was removed. On-target share goes from 16/59 to 16/44.
The 28 that remain are not this problem:
manual_unwrap_or_defaultreportingunwrap_or— correct, justless sharp than the API clippy names
manual_filterreportingand_thenbecauseOption::filterderives no behavior at all. Nothing narrower exists to win. That is a recall
gap, and a separate piece of work.
Also worth knowing
The
measure/packs/stdpack is stale. Re-derived with current code it is589 behaviors rather than 483, and
Iterator::filter_mapcomes out as(sift f0 v1 (call f1 v1))rather than the unliftedSequence[Traverse, None]it is stored as. Re-deriving does not move the score — that fresh form is
size 5 against
MINIMUM_REPORTABLE_SIZE6 with 0 anchors, so it is correctlyrefused — but anyone reading that pack should know it predates the one-step lift.
358 tests, clippy clean.