Single match else #579
Single match else #579
Conversation
| @@ -112,34 +112,40 @@ impl LateLintPass for MatchPass { | |||
| fn check_single_match(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) { | |||
| if arms.len() == 2 && | |||
| arms[0].pats.len() == 1 && arms[0].guard.is_none() && | |||
| arms[1].pats.len() == 1 && arms[1].guard.is_none() && | |||
| is_unit_expr(&arms[1].body) { | |||
| arms[1].pats.len() == 1 && arms[1].guard.is_none() { | |||
There was a problem hiding this comment.
Perhaps we should have an Allow lint for matches with an else block? I don't think one style is clearly preferred over the other, cc @llogiq .
There was a problem hiding this comment.
Yeah, I'm also not sure about this – on one hand the match statement has one more level of nesting, which may lead to more line breaks which hurt readability, on the other hand if let will often lead to one more line of very sparse code, which takes more vertical space then necessary.
There was a problem hiding this comment.
The conclusion is: add a lint that suggests turning regular if/else into a match on a boolean xD
One thing I have noticed is that I find statements much more readable with if let and expressions with match
There was a problem hiding this comment.
if the match arm is a (multiline) block, the if let version takes up one line less:
match x {
Y => println!("Jupiter"),
_ => {
println!("Mars");
println!("Venus");
},
}vs
if let Y = x {
println!("Jupiter");
} else {
println!("Mars");
println!("Venus");
}There was a problem hiding this comment.
True. But I'm more worried about
if let Some(x) = whaa {
x
} else {
0
}
which would obviously better be written as whaa.unwrap_or(0)...
There was a problem hiding this comment.
I can change the check to only show up if there's no else arm or if any of the arms is a block with at least a statement and an expression
|
On the bikeshed issue someone pointed out that this is a matter of personal style. I think we should just unconditionally allow (as in, a pedantic lint) matches with an else block. |
|
I have added a new lint and made it allow by default. The new lint is also much better than before. There is now no (imo) controversial fallout in clippy. |
fixes #172