rust-clippy
rust-clippy copied to clipboard
rust-lang
Single branch ifs in the body of a match arm should be made explicit
What it does
It raises a warning when the body of a match arm is an if condition with only the positive branch and suggests making the condition a part of the matching, and thus making the false path explicit.
Advantage
- Improves exhaustive pattern matching by forcing possibly missed cases to be explicitly handled.
- Prevents bugs due to accidentally putting the wide arrow before the if condition.
Drawbacks
- Increases verbosity of a program by making matches more complex.
Example
let outside_value1 = todo!();
let outside_value2 = todo!();
match (int_value1, int_value2) {
(0, 0) => todo!(),
(low, high) if low == high => {
todo!()
},
(_, 1) => if outside_value < outside_value2 {
// some code here
}
_ => {
todo!()
}
}
Could be written as:
let outside_value1 = todo!();
let outside_value2 = todo!();
match (int_value1, int_value2) {
(0, 0) => todo!(),
(low, high) if low == high => {
todo!()
},
(_, 1) if outside_value < outside_value2 => {
// some code here
},
// we are forced to handle this case explicitly,
// and don't change the semantics of the program by
// implicitly making the false branch fall into the wild-card pattern.
(_, 1) => {},
_ => {
todo!()
}
}
I accidentally created such a bug and the only reason I caught it was because my condition was _ => if ... and not (_, 1) => if ..., and thus rustc complained about the final wildcard being unreachable.
This may catch bugs, but it would have far too many false positives. For correct code the suggestion conflicts with match_same_arms/wildcard_in_or_patterns also
Notably rustfmt does not allow you to have PAT => if ..., it will always wrap it in a block to reduce the confusion here
Right the suggestion will introduce more errors. Could we limit it to just a warning and suspicious?
IMO the number of false positives would make it unable to be above restriction
A perhaps equivalent restriction lint would be to lint on all uses of guards in match arms
But that's not the same, is it? the intention is basically to disallow pat => if and pat => { if ... {rest} } and instead recommend pat if ... => {rest}
The other way, requiring every guard (pat if ... =>) to become pat if ... => { ... }; pat => { ... } seems equivalent but less direct than forbidding guards, so instead pat => { if ... { ... } else { ... } }
That makes sense. It feels too restrictive though, doesn't it? It increases nesting by an additional level.
I think we can close this as not to be implemented, but I'd love a way to ensure this accident doesn't happen.