Rustfmt: A comment is removed after `pub` keyword in a struct.

[xfix]

After reading http://journal.stuffwithstuff.com/2015/09/08/the-hardest-program-ive-ever-written/, I decided to put one of code examples provided here in Rust.

pub // Oh, crap. A line comment.
struct Foo {}

The original code used abstract, but there is no such keyword in Rust, I used pub instead. Rustfmt seeing that removed a comment. This is an edge case, I don't care what it formats down to, but I think comment should be preserved.

[anthonyclays]

This happens in a very large number of 'uncommon places for comments': http://play.rust-lang.org/?gist=f0e8521555680aed289378e1d8a7d849&version=stable&mode=debug

It'd be nice to at least print a warning when comments get thrown away. Ideally (IMO), rustfmt should move the comment to the first 'logical' place for comments strictly before its original location (but that's debatable: should these comments be moved before or after doc comments / atributes in the case of type / fn definitions?)

[zacheryph]

Another example. Comment in _the middle_ of struct/impl declaration.

Input File

struct X<T>
// where
//     T: Clone
{
    inner: String,
}

After rustfmt

struct X<T> {
    inner: String,
}

[hsandt]

Exactly. This was noted in https://github.com/rust-lang/rustfmt/pull/1911 and fixed for impl and trait only in https://github.com/rust-lang/rustfmt/pull/1925

We can assume the fix for struct would be similar as the cause should be similar, but not sure. I've noticed in particular that a comment just before a non-commented is not removed, as if protected. But I don't have an old rustfmt right now to check if this was also the case for impl and trait.

struct X<T>
// this will remain
where
    T: Clone
{
    inner: T
}

Should we split this issue in 2? (one for the pub part, one for the part before the brace)

I think a developer solving one issue will solve the other in the surrounding code at the same time, but I can't confirm until we find the exact cause.

Inspired by the fix for impl and trait, I searched for find_uncommented("{") and find_uncommented("where") in rustfmt/src/items.rs and found some occurrences, but not inside format_struct_struct itself.

Anyway, when solving this issue we should add comments at uncommon places in the unit tests to make sure they are preserved.

[scampi]

the problem in https://github.com/rust-lang/rustfmt/issues/2781#issuecomment-405924561 is fixed in #3495

[hellow554]

The rest isn't fixed.

Another example:

pub fn a(_: &/*'static*/()) {}

becomes

pub fn a(_: &()) {}

@topecongiro any reason why this (kind of critical) bug hasn't been fixed yet? It's almost open for a year now.

[scampi]

@hellow554 Thanks for giving an additional example where the comment is lost. That seems a strange place to put a comment though.
You are welcome to propose a PR to help resolve this issue faster too ;o)

[scampi]

Maybe we could try to create some kind of code generator that inserts comments at random places and see where comments are lost. The reason is that the span between some token is not taken into consideration and therefore you get lost comments. But I feel like that would make the code base harder to maintain. Or a re-design would be needed to facilitate how these missing spans are retained, at the moment it creeps in every rewrite method and the missing span needs to be carefully handled...

[paulstansifer]

I think I found another example (https://github.com/paulstansifer/unseemly/pull/17/files#diff-7328e869d09a114ad85e6ac59329ae4fL45)

    type Err : Debug /*Display*/ + Reifiable + Clone;

...goes to:

    type Err: Debug + Reifiable + Clone;

...I agree that I probably should not have put a comment there, though

[0xd34d10cc]

Yet another example:

use std::collections::{
  HashMap,
  // hash_map::Entry  
};

[pnkfelix]

Yet another example:

pub type Export /*: Bound */ = S1;

This is one where I think a comment is justified; today we do not enforce such bounds (i.e. the compiler does not check that the right-hand side of a type definition actually conforms to such a bound), and the compiler actually errors if you try to include it (because we don't want people to mistakenly think that such bounds are enforced).

But the comment here serves a useful purpose (about the intent, that we intend for this type definition to conform to the bound).

[hellow554]

@ayazhafiz I see, that you opened a seperate issue for the other problems, but I can't find that you covered my problem and it's still not fixed. Am I missing something?

[ayazhafiz]

@hellow554 sorry about that, added back in #4245