new lint that detects useless match expression #8471
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
flip1995
added
S-waiting-on-author
modify `manual_map_option` uitest because one test case has confliction.
re-design test cases as some of them are not worth the effort to check.
|
Finally got some time to get back to this PR! I could use some suggestions for my code, such as any missing cases, improvement for readability etc, thanks~ @rustbot ready |
rustbot
added
S-waiting-on-review
llogiq
left a comment
llogiq
left a comment
There was a problem hiding this comment.
This looks good! 👍 The naming should be more consistent, and we should reconsider the category, and I have a suggestion for further test cases, but otherwise this is merge-worthy.
clippy_lints/src/matches/mod.rs
Outdated
| /// } | ||
| /// ``` | ||
| #[clippy::version = "1.61.0"] | ||
| pub NOP_MATCH, |
There was a problem hiding this comment.
NOP is uncommon vocabulary. I'd suggest naming it needless_match or match_identity (as we already have the quite similar map_identity).
clippy_lints/src/matches/mod.rs
Outdated
| /// ``` | ||
| #[clippy::version = "1.61.0"] | ||
| pub NOP_MATCH, | ||
| correctness, |
There was a problem hiding this comment.
I don't see this as a correctness lint. At best, it's suspicious, or complexity.
tests/ui/nop_match.rs
Outdated
| D, | ||
| } | ||
|
|
||
| fn useless_match(x: i32) { |
There was a problem hiding this comment.
How about adding a test case with a x => x arm?
There was a problem hiding this comment.
That's a good point, I didn't check for that in my code. :D
Also, that reminds me, should I be checking ref x => *x as well? Or is that too much?
and change its lint group to "complexity"
|
Thank you! @bors r+ |
|
📌 Commit ec91164 has been approved by |
new lint that detects useless match expression fixes #7040 changelog: - Register new `correctness` lint called `nop_match` - add basic test cases - add logic to check for unnecessary match - add logic to check for unnecessary if-let expressions, remove some test cases because checking them will take a lot extra effort TODO: - ~~detects unnecessary match~~ - ~~detects unnecessary if-let expression that similar to match~~ - modify `question_mark` and `manual_map` to solve possible confliction
|
💔 Test failed - checks-action_test |
Thank you so much for the approval! But before I change the PR description which possiblly causing this being merged, may I request one last review for the next commit I'm about to push, I have taken your suggestion of adding a |
|
@bors r+ |
|
📌 Commit 086b045 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
6 participants
fixes #7040
changelog: Add new lint [
needless_match] under complexity lint group