about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2017-09-21 20:12:22 +0000
committerbors <bors@rust-lang.org>2017-09-21 20:12:22 +0000
commit17f56c549c35bb2cb316e5abff116e65277c7bb1 (patch)
tree4cbb280a21fbe176a91666ff86a1f9c4c4b2cd02
parent1b55d19479eabcb5d6cbda651187c7004aa28f7f (diff)
parentf0e7a5b8e51ea7e2e6b66293d96696d68491b6ad (diff)
Auto merge of #44215 - oli-obk:import_sugg, r=nrc
don't suggest placing `use` statements into expanded code

r? @nrc

fixes #44210

```rust
#[derive(Debug)]
struct Foo;

type X = Path;
```

will try to place `use std::path::Path;` between `#[derive(Debug)]` and `struct Foo;`

I am not sure how to obtain a span before the first attribute, because derive attributes are removed during expansion.

It would be trivial to detect this case and place the `use` after the item, but that would be somewhat weird I think.
-rw-r--r--src/librustc_resolve/lib.rs54
-rw-r--r--src/test/ui/resolve/privacy-struct-ctor.stderr6
-rw-r--r--src/test/ui/resolve/use_suggestion_placement.rs9
-rw-r--r--src/test/ui/resolve/use_suggestion_placement.stderr29
-rw-r--r--src/test/ui/span/issue-35987.stderr2
5 files changed, 70 insertions, 30 deletions
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 691329c608c..36cd69f91b9 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -605,7 +605,7 @@ impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
                 ItemKind::Use(..) => {
                     // don't suggest placing a use before the prelude
                     // import or other generated ones
-                    if item.span == DUMMY_SP {
+                    if item.span.ctxt().outer().expn_info().is_none() {
                         self.span = Some(item.span.with_hi(item.span.lo()));
                         self.found_use = true;
                         return;
@@ -615,11 +615,22 @@ impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
                 ItemKind::ExternCrate(_) => {}
                 // but place them before the first other item
                 _ => if self.span.map_or(true, |span| item.span < span ) {
-                    self.span = Some(item.span.with_hi(item.span.lo()));
+                    if item.span.ctxt().outer().expn_info().is_none() {
+                        // don't insert between attributes and an item
+                        if item.attrs.is_empty() {
+                            self.span = Some(item.span.with_hi(item.span.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.with_hi(attr.span.lo()));
+                                }
+                            }
+                        }
+                    }
                 },
             }
         }
-        assert!(self.span.is_some(), "a file can't have no items and emit suggestions");
     }
 }
 
@@ -3553,8 +3564,7 @@ impl<'a> Resolver<'a> {
             };
             visit::walk_crate(&mut finder, krate);
             if !candidates.is_empty() {
-                let span = finder.span.expect("did not find module");
-                show_candidates(&mut err, span, &candidates, better, finder.found_use);
+                show_candidates(&mut err, finder.span, &candidates, better, finder.found_use);
             }
             err.emit();
         }
@@ -3748,7 +3758,8 @@ fn import_candidate_to_paths(suggestion: &ImportSuggestion) -> (Span, String, St
 /// entities with that name in all crates. This method allows outputting the
 /// results of this search in a programmer-friendly way
 fn show_candidates(err: &mut DiagnosticBuilder,
-                   span: Span,
+                   // This is `None` if all placement locations are inside expansions
+                   span: Option<Span>,
                    candidates: &[ImportSuggestion],
                    better: bool,
                    found_use: bool) {
@@ -3766,18 +3777,27 @@ fn show_candidates(err: &mut DiagnosticBuilder,
     };
     let msg = format!("possible {}candidate{} into scope", better, msg_diff);
 
-    for candidate in &mut path_strings {
-        // produce an additional newline to separate the new use statement
-        // from the directly following item.
-        let additional_newline = if found_use {
-            ""
-        } else {
-            "\n"
-        };
-        *candidate = format!("use {};\n{}", candidate, additional_newline);
-    }
+    if let Some(span) = span {
+        for candidate in &mut path_strings {
+            // produce an additional newline to separate the new use statement
+            // from the directly following item.
+            let additional_newline = if found_use {
+                ""
+            } else {
+                "\n"
+            };
+            *candidate = format!("use {};\n{}", candidate, additional_newline);
+        }
 
-    err.span_suggestions(span, &msg, path_strings);
+        err.span_suggestions(span, &msg, path_strings);
+    } else {
+        let mut msg = msg;
+        msg.push(':');
+        for candidate in path_strings {
+            msg.push('\n');
+            msg.push_str(&candidate);
+        }
+    }
 }
 
 /// A somewhat inefficient routine to obtain the name of a module.
diff --git a/src/test/ui/resolve/privacy-struct-ctor.stderr b/src/test/ui/resolve/privacy-struct-ctor.stderr
index ee1481ec6f2..f7e5c602644 100644
--- a/src/test/ui/resolve/privacy-struct-ctor.stderr
+++ b/src/test/ui/resolve/privacy-struct-ctor.stderr
@@ -10,7 +10,7 @@ error[E0423]: expected value, found struct `Z`
    |
 help: possible better candidate is found in another module, you can import it into scope
    |
-16 |     use m::n::Z;
+22 |     use m::n::Z;
    |
 
 error[E0423]: expected value, found struct `S`
@@ -24,7 +24,7 @@ error[E0423]: expected value, found struct `S`
    |
 help: possible better candidate is found in another module, you can import it into scope
    |
-15 | use m::S;
+32 | use m::S;
    |
 
 error[E0423]: expected value, found struct `xcrate::S`
@@ -38,7 +38,7 @@ error[E0423]: expected value, found struct `xcrate::S`
    |
 help: possible better candidate is found in another module, you can import it into scope
    |
-15 | use m::S;
+32 | use m::S;
    |
 
 error[E0603]: tuple struct `Z` is private
diff --git a/src/test/ui/resolve/use_suggestion_placement.rs b/src/test/ui/resolve/use_suggestion_placement.rs
index e0027fed4d6..a43b8fc99df 100644
--- a/src/test/ui/resolve/use_suggestion_placement.rs
+++ b/src/test/ui/resolve/use_suggestion_placement.rs
@@ -16,6 +16,15 @@ mod m {
     pub const A: i32 = 0;
 }
 
+mod foo {
+    #[derive(Debug)]
+    pub struct Foo;
+
+    // test whether the use suggestion isn't
+    // placed into the expansion of `#[derive(Debug)]
+    type Bar = Path;
+}
+
 fn main() {
     y!();
     let _ = A;
diff --git a/src/test/ui/resolve/use_suggestion_placement.stderr b/src/test/ui/resolve/use_suggestion_placement.stderr
index 5c74d8bed66..8a4dfdc8027 100644
--- a/src/test/ui/resolve/use_suggestion_placement.stderr
+++ b/src/test/ui/resolve/use_suggestion_placement.stderr
@@ -1,7 +1,18 @@
+error[E0412]: cannot find type `Path` in this scope
+  --> $DIR/use_suggestion_placement.rs:25:16
+   |
+25 |     type Bar = Path;
+   |                ^^^^ not found in this scope
+   |
+help: possible candidate is found in another module, you can import it into scope
+   |
+21 |     use std::path::Path;
+   |
+
 error[E0425]: cannot find value `A` in this scope
-  --> $DIR/use_suggestion_placement.rs:21:13
+  --> $DIR/use_suggestion_placement.rs:30:13
    |
-21 |     let _ = A;
+30 |     let _ = A;
    |             ^ not found in this scope
    |
 help: possible candidate is found in another module, you can import it into scope
@@ -10,9 +21,9 @@ help: possible candidate is found in another module, you can import it into scop
    |
 
 error[E0412]: cannot find type `HashMap` in this scope
-  --> $DIR/use_suggestion_placement.rs:26:23
+  --> $DIR/use_suggestion_placement.rs:35:23
    |
-26 |     type Dict<K, V> = HashMap<K, V>;
+35 |     type Dict<K, V> = HashMap<K, V>;
    |                       ^^^^^^^ not found in this scope
    |
 help: possible candidates are found in other modules, you can import them into scope
@@ -23,16 +34,16 @@ help: possible candidates are found in other modules, you can import them into s
    |
 
 error[E0091]: type parameter `K` is unused
-  --> $DIR/use_suggestion_placement.rs:26:15
+  --> $DIR/use_suggestion_placement.rs:35:15
    |
-26 |     type Dict<K, V> = HashMap<K, V>;
+35 |     type Dict<K, V> = HashMap<K, V>;
    |               ^ unused type parameter
 
 error[E0091]: type parameter `V` is unused
-  --> $DIR/use_suggestion_placement.rs:26:18
+  --> $DIR/use_suggestion_placement.rs:35:18
    |
-26 |     type Dict<K, V> = HashMap<K, V>;
+35 |     type Dict<K, V> = HashMap<K, V>;
    |                  ^ unused type parameter
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors
 
diff --git a/src/test/ui/span/issue-35987.stderr b/src/test/ui/span/issue-35987.stderr
index 0cd7e1046f6..b57b58e3d2a 100644
--- a/src/test/ui/span/issue-35987.stderr
+++ b/src/test/ui/span/issue-35987.stderr
@@ -6,7 +6,7 @@ error[E0404]: expected trait, found type parameter `Add`
    |
 help: possible better candidate is found in another module, you can import it into scope
    |
-11 | use std::ops::Add;
+13 | use std::ops::Add;
    |
 
 error[E0601]: main function not found