Struct fields: remove unnecessary parens - #7161
matthewhughes934 wants to merge 2 commits into
Conversation
| .rewrite_result(context, Shape::legacy(budget, shape.indent + 1)) | ||
| .map(|ty_str| format!("({})", ty_str)); | ||
| } | ||
| let (ty, _, _, _, _) = unwrap_parens( |
There was a problem hiding this comment.
Discuss: should this have a style edition gate? I feel like it is a bug fix (inconsistency in expression vs. type formatting), but it also could change existing code.
There was a problem hiding this comment.
@matthewhughes934 for now let's add the style edition gate
There was a problem hiding this comment.
@matthewhughes934 for now let's add the style edition gate
👍 36d00f6
This comment has been minimized.
This comment has been minimized.
Like is already done for expressions, e.g.
let _ = ((var)) -> let _ = (var)
Do this by extending the function used for expressions. This is part of
my working looking in to issue rust-lang#6642 (but doesn't
directly address it)
5549214 to
deb911f
Compare
| // rustfmt-style_edition: 2027 | ||
|
|
||
| struct F { | ||
| f: ((u32)), |
There was a problem hiding this comment.
Are parens removed if there are comments?
There was a problem hiding this comment.
Should probably also add a test case with a lot more parens. Something like f: (((((u32))))), to make sure it's idempotent and doesn't just remove one set of parens every time you run rustfmt.
There was a problem hiding this comment.
Also, if this doesn't already it should probably also be configured using remove_nested_parens. We'll want to add a test case to show that parens aren't removed when remove_nested_parens=false. It's true by default so it's not necessary in this case, but might be good to be explicit about it.
I think we should also add a style_edition=2024 test case to show that parens aren't removed even with remove_nested_parens=true to highlight the issue we're solving.
Like is already done for expressions, e.g.
Do this by extending the function used for expressions. This is part of
my working looking in to issue #6642 (but doesn't
directly address it)