match_same_arms, ifs_same_cond: lint once per same arm/condition
#14637
Conversation
rustbot
added
the
S-waiting-on-review
clippy_utils/src/lib.rs
Outdated
| /// `exprs` | ||
| /// Returns a list of groups where elements in each group are equal according to `eq` | ||
| /// | ||
| /// Groups contain at least two elements and appear in the order they occur in `exprs` |
There was a problem hiding this comment.
| /// Groups contain at least two elements and appear in the order they occur in `exprs` | |
| /// Groups contain at least two elements which appear in the order they occur in `exprs` |
The groups themselves are not ordered, only elements within a group are.
There was a problem hiding this comment.
The groups are also ordered, updated the comment to say that
| group.iter().map(|(_, arm)| arm.span).collect_vec(), | ||
| "these match arms have identical bodies", | ||
| |diag| { | ||
| diag.help("make the arms return different values"); |
There was a problem hiding this comment.
I find this suggestion a bit strange. While it sounds logical to group the arms if possible, changing the body value means changing the semantics.
There was a problem hiding this comment.
It's from the basis that the same arms may not be intentional and so inherently require changing the semantics, reworded it to state that explicitly
|
@Alexendoo This looks really nice (see the two nitpicking comments). I like how the patterns stay in order compared to the previous implementation. I prefer to wait until 1.88 is branched out of master (in two days) before merging this in case an unplanned Clippy sync happens in the meantime. Given the scope of the change, a full nightly + beta cycle will be useful to catch any oversight. |
|
@rustbot author |
rustbot
added
S-waiting-on-author
Alexendoo
force-pushed
the
group-search-same
branch
from
d6ccb71 to
39ab00a
Compare
Alexendoo
added
S-waiting-on-review
|
Thanks! |
A large fraction of the lints emitted in CI lintcheck come from this
match, currently fornsame arms((n - 1) * n)/2lints are emitted, with this change it will be emitted as a single lintAlso fixes #13835
changelog: none