Add wildcard_match_arm lint #3652
Conversation
phansch
added
the
S-waiting-on-review
There was a problem hiding this comment.
LGTM!
It indeed would be great if you could add tests. You can look in the tests/ui/ folder. Just add a new file there, where you put patterns in where the lint should trigger and some where it shouldn't. One test case where it shouldn't trigger is for example an _ if some_cond => (), arm IMO. Make sure to add #![warn(clippy::wildcard_match_arm)] to the top of the file.
After you added this file run cargo test (you can restrict this to just your test, see CONTRIBUTING.md for this). Now your test should fail, because you need to generate a *.stderr file. You can do this by running the tests/ui/update-references.sh script. The exact command will be printed with the output of cargo test, so you can just c&p this.
flip1995
added
S-waiting-on-author
|
Update: I've made some changes (including the above), but it'll be a few days until I can finish implementing the tests. I'll push everything once that's done. |
|
Wrong button! I think this should be good to go now. |
|
Thanks! Only a |
|
Ran |
|
Thanks! That should be fixed by now. @bors r |
|
📌 Commit fefe55e has been approved by |
Add wildcard_match_arm lint This lint prevents using a wildcard in a match arm. Implemented as a restriction currently, because this is pretty much an edge case. See #3649 for more information. Didn't add any tests because I wasn't sure how, but if someone wants to point me in the right direction, I'd be happy to!
|
💔 Test failed - checks-travis |
|
It seems that a formatting of the tests is missing. This should be the last chore. https://travis-ci.com/rust-lang/rust-clippy/jobs/173709864#L1383-L1447 A |
|
Neither |
|
@bors retry |
|
I think it would be better just to emit a lint in cases of an enum. So match 3 {
4 => println!(),
_ => println!(),
};would not emit a warning. Note also that it would need to check for matching in an enum on any nesting level. I.e. match Some(Color::Red) {
Some(Color::Blue) => println!(),
Some(_) => println!(),
None => println!(),
};would emit a lint. |
|
@bors r- pending decision on linting non-enum matches |
|
I agree re: linting only on enums. I've changed the behavior and updated the lint name accordingly. I'm not familiar enough with compiler internals to implement the nested check yet, so I added a note of this limitation to the known issues. I may try to resolve this in the future if I have time, or someone else can, but I won't be able to do it in the near future. Thanks for the insightful feedback, @orium! |
|
@bors r LGTM now, thanks! |
|
📌 Commit 97c7d28 has been approved by |
Add wildcard_match_arm lint This lint prevents using a wildcard in a match arm. Implemented as a restriction currently, because this is pretty much an edge case. See #3649 for more information. Didn't add any tests because I wasn't sure how, but if someone wants to point me in the right direction, I'd be happy to!
This lint prevents using a wildcard in a match.
Also fix message capitalization.
We're not only working with Results.
We also don't need `ex` as an argument.
|
Updated lint count; should actually work next time. |
|
@bors retry |
|
@bors ping |
|
😪 I'm awake I'm awake |
|
@bors r=phansch |
|
📌 Commit 7fa50fb has been approved by |
Add wildcard_match_arm lint This lint prevents using a wildcard in a match arm. Implemented as a restriction currently, because this is pretty much an edge case. See #3649 for more information. Didn't add any tests because I wasn't sure how, but if someone wants to point me in the right direction, I'd be happy to!
|
☀️ Test successful - checks-travis, status-appveyor |
This lint prevents using a wildcard in a match arm. Implemented as a restriction currently, because this is pretty much an edge case. See #3649 for more information.
Didn't add any tests because I wasn't sure how, but if someone wants to point me in the right direction, I'd be happy to!