-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Closed
Labels
C-bugCategory: This is a bug.Category: This is a bug.
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.
Metadata
Metadata
Assignees
Labels
C-bugCategory: This is a bug.Category: This is a bug.