Closed
Description
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:
=> (
some_long_expr
)
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.:
=> {
some_long_expr
}
But not with parentheses. I think that for the case of parentheses, the precedent should go with removing them.