Minimal example:
pub fn broken(x: Option<()>) -> i32 {
match x {
Some(()) => (1),
None => (2),
}
}
pub fn works(x: Option<()>) -> i32 {
if x.is_none() {
(2)
} else {
(1)
}
}
I would expect that the unused_parens lint fire for the broken function, but it does not. Compare this to the works function, where it does. This can potentially show up when refactoring a function, e.g. if converting to the above from:
pub fn broken(x: Option<()>) -> i32 {
match x {
Some(()) => do_something_with(1),
None => do_something_with(2),
}
}
This could arguably be a formatting issue, as some people might like to split things on multiple lines like:
However, we don't do this for other cases where unused_parens is added. Take, for example:
if (
some_long_expr
) {
// ...
}
This still triggers the unused_parens lint.
This is a particularly weird case because users will often separate out the match arm into a block if it gets long, e.g.:
But not with parentheses. I think that for the case of parentheses, the precedent should go with removing them.
Minimal example:
I would expect that the
unused_parenslint fire for thebrokenfunction, but it does not. Compare this to theworksfunction, where it does. This can potentially show up when refactoring a function, e.g. if converting to the above from:This could arguably be a formatting issue, as some people might like to split things on multiple lines like:
However, we don't do this for other cases where
unused_parensis added. Take, for example:This still triggers the
unused_parenslint.This is a particularly weird case because users will often separate out the match arm into a block if it gets long, e.g.:
But not with parentheses. I think that for the case of parentheses, the precedent should go with removing them.