New lint: match_wildcard_for_single_variants
#5582
Conversation
clippy_lints/src/misc.rs
Outdated
| @@ -509,7 +509,7 @@ fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> boo | |||
| Constant::F64(f) => *f == 0.0 || (*f).is_infinite(), | |||
| _ => false, | |||
| }), | |||
| _ => false, | |||
| Some(_) | None => false, | |||
There was a problem hiding this comment.
This shouldn't lint here, since Some(_) and None is left.
There was a problem hiding this comment.
Current implementation only checks if a variant exists and doesn't check if it's exsaustive. I guess it was ok for wildcard_enum_match_arm, but it's not ok in case of the current lint. I'll try to implement it.
I don't know if there can be cases when it's not possible to check for exuastiveness, but if there is, what should I do then? Should I just skip that match?
I'm sorry, I guess I should have opened a WIP PR
There was a problem hiding this comment.
We could also implement the same logic as with skipping arm guards.
If a variant is appearing only in patterns with arm guards, then it is treated as missing.
https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/matches.rs#L722
if arm.guard.is_some() {
// Guards mean that this case probably isn't exhaustively covered. Technically
// this is incorrect, as we should really check whether each variant is exhaustively
// covered by the set of guards that cover it, but that's really hard to do.
continue;
}
We could check only for simple case Variant(wild), and if it doesn't match then don't count Variant as present.
Which way do you want to go?
There was a problem hiding this comment.
I'm sorry, I guess I should have opened a WIP PR
Not a problem, doesn't really matter review wise.
Which way do you want to go?
The way you think is the easiest way to implement this. I see you put more thought into this, than I have, so I trust your judgement here.
In the end it should only suggest to replace it with the remaining variant, if we can be absolutely sure that there is only one left.
There was a problem hiding this comment.
I've fixed it. Currently variants with TupleStruct patterns are treated as missing unless they pass checks for exhaustiveness. The checks are simple. They check for wild and binding patterns.
vtmargaryan
force-pushed
the
match_wildcard_for_single_variants
branch
from
aae97ec to
1c59cd5
Compare
flip1995
added
the
S-waiting-on-author
…is spanned on Option" This reverts commit 4948307.
TupleStruct matches are checked for exhaustiveness
|
There is a label S-waiting-on-author on this PR. I just wanted to clarify that I've applied all suggestions. |
|
@vtmargaryan it will be re-reviewed eventually :) AFAIK you can't change the tags, but there are other ways the maintainers can realize that changes were done to the PR |
|
@ebroto, thanks, I just didn't know if I should do something else or is it enough to push the commits :) Maybe it's worth adding to contribution guide |
|
Nice solution for the exhaustive check! currently waiting for #5621 |
flip1995
added
S-waiting-on-bors
|
@bors r+ |
|
📌 Commit d906253 has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: Added a new lint match_wildcard_for_single_variants to warn on enum matches where a wildcard is used to match a single variant
Closes #5556