Rustfix discards comments

[jyn514]

$ cat src/lib.rs
extern "C" {
    fn xsave64(p: *mut u8, hi: u32, lo: u32)
        -> /* comment */ ();
}
$ cargo +nightly clippy --fix  -Z unstable-options
$ git diff
diff --git a/src/lib.rs b/src/lib.rs
index e07dbbc..0110132 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1,6 +1,6 @@
 extern "C" {
     fn xsave64(p: *mut u8, hi: u32, lo: u32)
-        -> /*comment here*/ ();
+        ;
 }

@flip1995 suggested that I file a bug against rustfix, not clippy, since the information about comments has been lost by the time clippy sees it.

[jyn514]

and a more realistic comment that's lost:

index 791d2686cb5..d1e51763a7a 100644
--- a/compiler/rustc_expand/src/mbe/macro_rules.rs
+++ b/compiler/rustc_expand/src/mbe/macro_rules.rs
@@ -1036,17 +1036,13 @@ fn token_can_be_followed_by_any(tok: &mbe::TokenTree) -> bool {
 /// a fragment specifier (indeed, these fragments can be followed by
 /// ANYTHING without fear of future compatibility hazards).
 fn frag_can_be_followed_by_any(kind: NonterminalKind) -> bool {
-    match kind {
-        NonterminalKind::Item           // always terminated by `}` or `;`
+    matches!(kind, NonterminalKind::Item           // always terminated by `}` or `;`
         | NonterminalKind::Block        // exactly one token tree
         | NonterminalKind::Ident        // exactly one token tree
         | NonterminalKind::Literal      // exactly one token tree
         | NonterminalKind::Meta         // exactly one token tree
         | NonterminalKind::Lifetime     // exactly one token tree
-        | NonterminalKind::TT => true,  // exactly one token tree
-
-        _ => false,
-    }
+        | NonterminalKind::TT)
 }

Notice // exactly one token tree has disappeared.

[ehuss]

I'm not sure there's anything rustfix can do here. It only works with the suggested spans from rustc, it has no knowledge of Rust syntax (it works on simple byte replacements as instructed by the compiler).

I'm not sure what a reasonable output for the first example would be. I think it would be wrong for suggestions to somehow retain arbitrary comments within a span.

The second example also looks like it would be quite difficult to translate properly. One option is that Clippy could emit a multi-part suggestion, with each part containing a minimal span to translate. For example:

• match kind { → matches!(kind,
• => true, → empty
• _ => false, → empty
• } → )

However, I think that would be a significant decrease in the readable quality of the diagnostic, and would probably be more difficult to implement and error-prone.

[flip1995]

I don't think we should do this in Clippy/rustc. We would have to go through every lint with a suggestion, determine if it could be triggered in multiline spans and then work out some complicated diagnostic to prevent Clippy from removing comments. Which we can't even completely guarantee in the end, since you can put /* ... */ comments everywhere.

That being said, if rustfix cannot handle this either we might just have to live with it.

However, I think that would be a significant decrease in the readable quality of the diagnostic, and would probably be more difficult to implement and error-prone.

Plus the issue, that rustfix can't handle multipart suggestions #141

[ehuss]

I'm going to close this per the comments above.