Auto merge of rust-lang#85427 - ehuss:fix-use-placement, r=jackh726

Fix use placement for suggestions near main.

This fixes an edge case for the suggestion to add a `use`. When running with `--test`, the `main` function will be annotated with an `#[allow(dead_code)]` attribute. The `UsePlacementFinder` would end up using the dummy span of that synthetic attribute. If there are top-level inner attributes, this would place the `use` in the wrong position. The solution here is to ignore attributes with dummy spans.

In the process of working on this, I discovered that the `use_suggestion_placement` test was broken. `UsePlacementFinder` is unaware of active attributes. Attributes like `#[derive]` don't exist in the AST since they are removed. Fixing that is difficult, since the AST does not retain enough information. I considered trying to place the `use` towards the top of the module after any `extern crate` items, but I couldn't find a way to get a span for the start of a module block (the `mod` span starts at the `mod` keyword, and it seems tricky to find the spot just after the opening bracket and past inner attributes). For now, I just put some comments about the issue. This appears to have been a known issue in rust-lang#44215 where the test for it was introduced, and the fix seemed to be deferred to later.

Author: bors

compiler/rustc_resolve/src/lib.rs

@@ -335,15 +335,15 @@ impl UsePlacementFinder {
                     if self.span.map_or(true, |span| item.span < span)
                         && !item.span.from_expansion()
                     {
+                        self.span = Some(item.span.shrink_to_lo());
                         // don't insert between attributes and an item
-                        if item.attrs.is_empty() {
-                            self.span = Some(item.span.shrink_to_lo());
-                        } else {
-                            // find the first attribute on the item
-                            for attr in &item.attrs {
-                                if self.span.map_or(true, |span| attr.span < span) {
-                                    self.span = Some(attr.span.shrink_to_lo());
-                                }
+                        // find the first attribute on the item
+                        // FIXME: This is broken for active attributes.
+                        for attr in &item.attrs {
+                            if !attr.span.is_dummy()
+                                && self.span.map_or(true, |span| attr.span < span)
+                            {
+                                self.span = Some(attr.span.shrink_to_lo());
                             }
                         }
                     }

compiler/rustc_typeck/src/check/method/suggest.rs

@@ -1650,16 +1650,16 @@ impl intravisit::Visitor<'tcx> for UsePlacementFinder<'tcx> {
                 _ => {
                     if self.span.map_or(true, |span| item.span < span) {
                         if !item.span.from_expansion() {
+                            self.span = Some(item.span.shrink_to_lo());
                             // Don't insert between attributes and an item.
                             let attrs = self.tcx.hir().attrs(item.hir_id());
-                            if attrs.is_empty() {
-                                self.span = Some(item.span.shrink_to_lo());
-                            } else {
-                                // Find the first attribute on the item.
-                                for attr in attrs {
-                                    if self.span.map_or(true, |span| attr.span < span) {
-                                        self.span = Some(attr.span.shrink_to_lo());
-                                    }
+                            // Find the first attribute on the item.
+                            // FIXME: This is broken for active attributes.
+                            for attr in attrs {
+                                if !attr.span.is_dummy()
+                                    && self.span.map_or(true, |span| attr.span < span)
+                                {
+                                    self.span = Some(attr.span.shrink_to_lo());
                                 }
                             }
                         }

src/test/ui/resolve/use_suggestion_placement.fixed

@@ -0,0 +1,39 @@
+// run-rustfix
+#![allow(dead_code)]
+
+use m::A;
+
+use std::collections::HashMap;
+
+macro_rules! y {
+    () => {}
+}
+
+mod m {
+    pub const A: i32 = 0;
+}
+
+mod foo {
+    // FIXME: UsePlacementFinder is broken because active attributes are
+    // removed, and thus the `derive` attribute here is not in the AST.
+    // An inert attribute should work, though.
+    // #[derive(Debug)]
+    use std::path::Path;
+
+#[allow(warnings)]
+    pub struct Foo;
+
+    // test whether the use suggestion isn't
+    // placed into the expansion of `#[derive(Debug)]
+    type Bar = Path; //~ ERROR cannot find
+}
+
+fn main() {
+    y!();
+    let _ = A; //~ ERROR cannot find
+    foo();
+}
+
+fn foo() {
+    type Dict<K, V> = HashMap<K, V>; //~ ERROR cannot find
+}

src/test/ui/resolve/use_suggestion_placement.rs

@@ -1,3 +1,6 @@
+// run-rustfix
+#![allow(dead_code)]
+
 macro_rules! y {
     () => {}
 }
@@ -7,7 +10,11 @@ mod m {
 }
 
 mod foo {
-    #[derive(Debug)]
+    // FIXME: UsePlacementFinder is broken because active attributes are
+    // removed, and thus the `derive` attribute here is not in the AST.
+    // An inert attribute should work, though.
+    // #[derive(Debug)]
+    #[allow(warnings)]
     pub struct Foo;
 
     // test whether the use suggestion isn't

src/test/ui/resolve/use_suggestion_placement.stderr

@@ -1,5 +1,5 @@
 error[E0412]: cannot find type `Path` in this scope
-  --> $DIR/use_suggestion_placement.rs:15:16
+  --> $DIR/use_suggestion_placement.rs:22:16
    |
 LL |     type Bar = Path;
    |                ^^^^ not found in this scope
@@ -10,7 +10,7 @@ LL |     use std::path::Path;
    |
 
 error[E0425]: cannot find value `A` in this scope
-  --> $DIR/use_suggestion_placement.rs:20:13
+  --> $DIR/use_suggestion_placement.rs:27:13
    |
 LL |     let _ = A;
    |             ^ not found in this scope
@@ -21,7 +21,7 @@ LL | use m::A;
    |
 
 error[E0412]: cannot find type `HashMap` in this scope
-  --> $DIR/use_suggestion_placement.rs:25:23
+  --> $DIR/use_suggestion_placement.rs:32:23
    |
 LL |     type Dict<K, V> = HashMap<K, V>;
    |                       ^^^^^^^ not found in this scope

src/test/ui/suggestions/use-placement-resolve.fixed

@@ -0,0 +1,13 @@
+// compile-flags: --test
+// run-rustfix
+// Checks that the `use` suggestion appears *below* this inner attribute.
+// There was an issue where the test synthetic #[allow(dead)] attribute on
+// main which has a dummy span caused the suggestion to be placed at the top
+// of the file.
+#![allow(unused)]
+
+use std::fmt::Debug;
+
+fn main() {}
+
+fn foobar<T: Debug>(x: T) {} //~ ERROR expected trait, found derive macro

src/test/ui/suggestions/use-placement-resolve.rs

@@ -0,0 +1,11 @@
+// compile-flags: --test
+// run-rustfix
+// Checks that the `use` suggestion appears *below* this inner attribute.
+// There was an issue where the test synthetic #[allow(dead)] attribute on
+// main which has a dummy span caused the suggestion to be placed at the top
+// of the file.
+#![allow(unused)]
+
+fn main() {}
+
+fn foobar<T: Debug>(x: T) {} //~ ERROR expected trait, found derive macro

src/test/ui/suggestions/use-placement-resolve.stderr

@@ -0,0 +1,14 @@
+error[E0404]: expected trait, found derive macro `Debug`
+  --> $DIR/use-placement-resolve.rs:11:14
+   |
+LL | fn foobar<T: Debug>(x: T) {}
+   |              ^^^^^ not a trait
+   |
+help: consider importing this trait instead
+   |
+LL | use std::fmt::Debug;
+   |
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0404`.

src/test/ui/suggestions/use-placement-typeck.fixed

@@ -0,0 +1,22 @@
+// compile-flags: --test
+// run-rustfix
+// Checks that the `use` suggestion appears *below* this inner attribute.
+// There was an issue where the test synthetic #[allow(dead)] attribute on
+// main which has a dummy span caused the suggestion to be placed at the top
+// of the file.
+#![allow(unused)]
+
+use m::Foo;
+
+fn main() {
+    let s = m::S;
+    s.abc(); //~ ERROR no method named `abc`
+}
+
+mod m {
+    pub trait Foo {
+        fn abc(&self) {}
+    }
+    pub struct S;
+    impl Foo for S{}
+}

src/test/ui/suggestions/use-placement-typeck.rs

@@ -0,0 +1,20 @@
+// compile-flags: --test
+// run-rustfix
+// Checks that the `use` suggestion appears *below* this inner attribute.
+// There was an issue where the test synthetic #[allow(dead)] attribute on
+// main which has a dummy span caused the suggestion to be placed at the top
+// of the file.
+#![allow(unused)]
+
+fn main() {
+    let s = m::S;
+    s.abc(); //~ ERROR no method named `abc`
+}
+
+mod m {
+    pub trait Foo {
+        fn abc(&self) {}
+    }
+    pub struct S;
+    impl Foo for S{}
+}