Issue-5892: Type alias generic ident should follow the same linebreak rule as trait #6293
Conversation
johnhuichen
force-pushed
the
issue-5892-type-alias-generic-ident-should-be-4-spaces
branch
from
4ba8d22
to
fa07d97
Compare
johnhuichen
changed the title
johnhuichen
force-pushed
the
issue-5892-type-alias-generic-ident-should-be-4-spaces
branch
from
fa07d97
to
c801834
Compare
johnhuichen
force-pushed
the
issue-5892-type-alias-generic-ident-should-be-4-spaces
branch
2 times, most recently
from
519ffc4
to
e79db51
Compare
There was a problem hiding this comment.
Left a few notes mostly on code style, but not really on the substance of the change. It looks like there are two issues being worked here and it'll be easier to review the code if each issue had its own PR. When you get a chance, can you please remove the code changes for rust-lang/style-team#189.
@calebcartwright I'd still want @rust-lang/style to weigh in here and direct us on what the correct line break behavior should be for type aliases.
// Is this right?
type AAAAAAAAAAAAA: BBBBBBBBBBBBBBB<
CCCCCCCCCCCCCCCCC,
DDDDDDDDDDDDDDDDD,
EEEEEEEEEEEEEEEEE,
FFFFFFFFFFFFFFFFF,
GGGGGGGGGGGGGGGGG,
HHHHHHHHHHHHHHHHH,
IIIIIIIIIIIIIIIII,
>;
// or is this right?
type AAAAAAAAAAAAA:
BBBBBBBBBBBBBBB<
CCCCCCCCCCCCCCCCC,
DDDDDDDDDDDDDDDDD,
EEEEEEEEEEEEEEEEE,
FFFFFFFFFFFFFFFFF,
GGGGGGGGGGGGGGGGG,
HHHHHHHHHHHHHHHHH,
IIIIIIIIIIIIIIIII,
>;
src/items.rs
Outdated
| match context.config.style_edition() { | ||
| style_edition if style_edition < StyleEdition::Edition2024 => { | ||
| // 2 = `: ` | ||
| let shape = | ||
| Shape::indented(indent, context.config).offset_left(result.len() + 2)?; | ||
| let type_bounds = bounds.rewrite(context, shape).map(|s| format!(": {}", s))?; | ||
| result.push_str(&type_bounds); | ||
| } | ||
| _ => { | ||
| let shape = | ||
| Shape::indented(indent, context.config).offset_left(result.len())?; | ||
| result = rewrite_assign_rhs_with( | ||
| context, | ||
| result.clone() + ":", | ||
| bounds, | ||
| shape, | ||
| &RhsAssignKind::Bounds, | ||
| RhsTactics::ForceNextLineWithoutIndent, | ||
| )?; | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we need to use a match here. I would probably write this using an if else since you can avoid an extra level on indentation. Also, let's use <= StyleEdition::Edition2021 for older formatting instead of < StyleEdition::Edition2024. It's easier to grep for Edition2021 to find all the places where we're using the older formatting rules.
src/types.rs
Outdated
| style_edition @ _ if style_edition <= StyleEdition::Edition2021 => { | ||
| style_edition if style_edition <= StyleEdition::Edition2021 => { |
There was a problem hiding this comment.
not necessary. It was suggested by clippy
.gitignore
Outdated
|
|
||
| # nvim local dap settings | ||
| .nvim-dap.lua |
There was a problem hiding this comment.
Let's revert this. It's not really part of the fix for this issue
tests/target/issue-189.rs
Outdated
| // rustfmt-style_edition: 2024 | ||
|
|
||
| impl SomeType { | ||
| fn method(&mut self) { | ||
| self.array[array_index as usize] | ||
| .as_mut() | ||
| .expect("thing must exist") | ||
| .extra_info = Some(ExtraInfo { | ||
| parent, | ||
| count: count as u16, | ||
| children: children.into_boxed_slice(), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
@johnhuichen can you open up a separate PR for this. These two issue are unrelated, right? Also, can you rename the file to reflect that this isn't rustfmt issue number 189, but rather rust-lang/style-team#189
There was a problem hiding this comment.
Hi @ytmimi thanks for reviewing. I can separate the two issues
johnhuichen
force-pushed
the
issue-5892-type-alias-generic-ident-should-be-4-spaces
branch
from
e79db51
to
2152473
Compare
johnhuichen
force-pushed
the
issue-5892-type-alias-generic-ident-should-be-4-spaces
branch
from
2152473
to
b6f5b62
Compare
calebcartwright
added
blocked
Pull Request Template
Checklist
cargo testpassesRelated Issues/PRs
#5892
Changes
The type alias formatting uses overlapping code path as trait formatting.
The difference is that type alias doesn't use the logic of
rewrite_assign_rhs_with_comments, which checks if the generic bounds should be placed on the next lineTesting
added a test case
tests/source/issue-5892.rs