Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

trait bounds with comments are not formatted #2055

Open
gnzlbg opened this issue Oct 11, 2017 · 6 comments · May be fixed by #5059 or #6048
Open

trait bounds with comments are not formatted #2055

gnzlbg opened this issue Oct 11, 2017 · 6 comments · May be fixed by #5059 or #6048
Labels
a-comments poor-formatting

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 11, 2017 •
edited
Loading

This compiles file:

pub trait A {}
pub trait B {}
pub trait C {}

pub trait Foo:
// A and C
A   C
// and B
      B
{}

but rustfmt formats it into this:

pub trait Foo: A   C   B // A and C
A
      C
// and B
      B {
}

try this live (just click on Format in the playground)

which fails to compile with:

   Compiling playground v0.0.1 (file:///playground)
error: expected one of `(`, ` `, `::`, `<`, `where`, or `{`, found `A`
 --> src/main.rs:6:1
  |
5 | trait Foo: A   C   B // A and C:
  |                     - expected one of `(`, ` `, `::`, `<`, `where`, or `{` here
6 | A   C
  | ^ unexpected token
@topecongiro topecongiro added bug Panic, non-idempotency, invalid code, etc. a-comments and removed bug Panic, non-idempotency, invalid code, etc. labels Oct 11, 2017
@nrc nrc added the p-high label Nov 2, 2017
@topecongiro
Copy link
Contributor

To fix this, we need to extract comments between

  1. Foo and :
  2. : and the first trait bound
  3. each trait bound and

...which is, a lot of work. I hope that in the future we generalize write_list() and use it to rewrite trait bounds. For now, I think we should just return the original code if there are comments that we cannot handle.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 9, 2017 •
edited
Loading

Would this be easier if rustfmt wouldn't reorder the trait bounds at all?

(that is, keep the trait bounds in the declared order, align them properly, put one per line, etc, but don't reorder them)

@nrc
Copy link
Member

nrc commented Nov 9, 2017

Would this be easier if rustfmt wouldn't reorder the trait bounds at all?

No, not really, the hard bit is processing the source, rather than formatting the output

topecongiro added a commit to topecongiro/rustfmt that referenced this issue Nov 10, 2017
@topecongiro
Add a test for rust-lang#2055
38a5350
@topecongiro topecongiro mentioned this issue Nov 10, 2017
Merged
@nrc
Copy link
Member

nrc commented Nov 10, 2017 •
edited
Loading

This is now somewhat addressed. We no longer corrupt the trait bounds, but we don't do any formatting at all if there are comments.

@nrc nrc added poor-formatting and removed bug Panic, non-idempotency, invalid code, etc. p-high labels Nov 10, 2017
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Nov 10, 2017 via email

@topecongiro topecongiro mentioned this issue Nov 19, 2017
Closed
@nrc nrc modified the milestones: 1.0, 1.0 (preview 2) Jul 10, 2018
@nrc nrc changed the title trait bounds are reordered and dupplicated around comments trait bounds with comments are not formatted Jul 10, 2018
@scampi scampi mentioned this issue Jul 12, 2019
Closed
@davidBar-On davidBar-On mentioned this issue Jan 22, 2021
Open
@davidBar-On
Copy link
Contributor

PR #4666 enhance the resolution of this issue by handling comments between the Trait Generics and Bounds, e.g. the // A and C comment in the issue description. The PR does not handle comments withing the Bounds, e.g. the // and B in the description, which probably will be solved by handling BinOp comments.

ulyssa added a commit to ulyssa/rustfmt that referenced this issue Sep 25, 2021
@ulyssa
Add "type_separator" option to control placement of " " in types
9a124e9 
Fixes rust-lang#4979
Fixes rust-lang#2055
ulyssa added a commit to ulyssa/rustfmt that referenced this issue Sep 25, 2021
@ulyssa
Add "type_separator" option to control placement of " " in types
cf66692 
Fixes rust-lang#4979
Fixes rust-lang#2055
@ulyssa ulyssa mentioned this issue Sep 25, 2021
Open
ulyssa added a commit to ulyssa/rustfmt that referenced this issue Oct 18, 2021
@ulyssa
Support comments in trait bounds
f937c04 
Fixes rust-lang#2055
ulyssa added a commit to ulyssa/rustfmt that referenced this issue Nov 1, 2021
@ulyssa
Support comments in trait bounds
cbd2edf 
Fixes rust-lang#2055
ulyssa added a commit to ulyssa/rustfmt that referenced this issue Nov 2, 2021
@ulyssa
Support comments in trait bounds
1715777 
Fixes rust-lang#2055
@ulyssa ulyssa linked a pull request Nov 2, 2021 that will close this issue
Open
@calebcartwright calebcartwright mentioned this issue Jun 18, 2023
Open
This was referenced Jan 24, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-comments poor-formatting
Projects
None yet
Milestone
No milestone
4 participants
@nrc @gnzlbg @topecongiro @davidBar-On