Add structured suggestions for trait imports

5 files changed, 166 insertions, 38 deletions

diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 8207057fd0a..5da795ddd1e 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -588,6 +588,18 @@ struct UsePlacementFinder {
     found_use: bool,
 }
 
+impl UsePlacementFinder {
+    fn check(krate: &Crate, target_module: NodeId) -> (Option<Span>, bool) {
+        let mut finder = UsePlacementFinder {
+            target_module,
+            span: None,
+            found_use: false,
+        };
+        visit::walk_crate(&mut finder, krate);
+        (finder.span, finder.found_use)
+    }
+}
+
 impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
     fn visit_mod(
         &mut self,
@@ -3588,14 +3600,9 @@ impl<'a> Resolver<'a> {
 
     fn report_with_use_injections(&mut self, krate: &Crate) {
         for UseError { mut err, candidates, node_id, better } in self.use_injections.drain(..) {
-            let mut finder = UsePlacementFinder {
-                target_module: node_id,
-                span: None,
-                found_use: false,
-            };
-            visit::walk_crate(&mut finder, krate);
+            let (span, found_use) = UsePlacementFinder::check(krate, node_id);
             if !candidates.is_empty() {
-                show_candidates(&mut err, finder.span, &candidates, better, finder.found_use);
+                show_candidates(&mut err, span, &candidates, better, found_use);
             }
             err.emit();
         }
diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs
index 8613ec86b4a..50e996dd954 100644
--- a/src/librustc_typeck/check/method/suggest.rs
+++ b/src/librustc_typeck/check/method/suggest.rs
@@ -340,16 +340,35 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                               err: &mut DiagnosticBuilder,
                               mut msg: String,
                               candidates: Vec<DefId>) {
-        let limit = if candidates.len() == 5 { 5 } else { 4 };
-        for (i, trait_did) in candidates.iter().take(limit).enumerate() {
-            msg.push_str(&format!("\ncandidate #{}: `use {};`",
-                                    i + 1,
-                                    self.tcx.item_path_str(*trait_did)));
-        }
-        if candidates.len() > limit {
-            msg.push_str(&format!("\nand {} others", candidates.len() - limit));
+        let module_did = self.tcx.hir.get_module_parent(self.body_id);
+        let module_id = self.tcx.hir.as_local_node_id(module_did).unwrap();
+        let krate = self.tcx.hir.krate();
+        let (span, found_use) = UsePlacementFinder::check(self.tcx, krate, module_id);
+        if let Some(span) = span {
+            let path_strings = candidates.iter().map(|did| {
+                // produce an additional newline to separate the new use statement
+                // from the directly following item.
+                let additional_newline = if found_use {
+                    ""
+                } else {
+                    "\n"
+                };
+                format!("use {};\n{}", self.tcx.item_path_str(*did), additional_newline)
+            }).collect();
+
+            err.span_suggestions(span, &msg, path_strings);
+        } else {
+            let limit = if candidates.len() == 5 { 5 } else { 4 };
+            for (i, trait_did) in candidates.iter().take(limit).enumerate() {
+                msg.push_str(&format!("\ncandidate #{}: `use {};`",
+                                        i + 1,
+                                        self.tcx.item_path_str(*trait_did)));
+            }
+            if candidates.len() > limit {
+                msg.push_str(&format!("\nand {} others", candidates.len() - limit));
+            }
+            err.note(&msg[..]);
         }
-        err.note(&msg[..]);
     }
 
     fn suggest_valid_traits(&self,
@@ -604,3 +623,83 @@ impl<'a> Iterator for AllTraits<'a> {
         })
     }
 }
+
+
+struct UsePlacementFinder<'a, 'tcx: 'a, 'gcx: 'tcx> {
+    target_module: ast::NodeId,
+    span: Option<Span>,
+    found_use: bool,
+    tcx: TyCtxt<'a, 'gcx, 'tcx>
+}
+
+impl<'a, 'tcx, 'gcx> UsePlacementFinder<'a, 'tcx, 'gcx> {
+    fn check(
+        tcx: TyCtxt<'a, 'gcx, 'tcx>,
+        krate: &'tcx hir::Crate,
+        target_module: ast::NodeId,
+    ) -> (Option<Span>, bool) {
+        let mut finder = UsePlacementFinder {
+            target_module,
+            span: None,
+            found_use: false,
+            tcx,
+        };
+        hir::intravisit::walk_crate(&mut finder, krate);
+        (finder.span, finder.found_use)
+    }
+}
+
+impl<'a, 'tcx, 'gcx> hir::intravisit::Visitor<'tcx> for UsePlacementFinder<'a, 'tcx, 'gcx> {
+    fn visit_mod(
+        &mut self,
+        module: &'tcx hir::Mod,
+        _: Span,
+        node_id: ast::NodeId,
+    ) {
+        if self.span.is_some() {
+            return;
+        }
+        if node_id != self.target_module {
+            hir::intravisit::walk_mod(self, module, node_id);
+            return;
+        }
+        // find a use statement
+        for item_id in &module.item_ids {
+            let item = self.tcx.hir.expect_item(item_id.id);
+            match item.node {
+                hir::ItemUse(..) => {
+                    // don't suggest placing a use before the prelude
+                    // import or other generated ones
+                    if item.span.ctxt().outer().expn_info().is_none() {
+                        self.span = Some(item.span.with_hi(item.span.lo()));
+                        self.found_use = true;
+                        return;
+                    }
+                },
+                // don't place use before extern crate
+                hir::ItemExternCrate(_) => {}
+                // but place them before the first other item
+                _ => if self.span.map_or(true, |span| item.span < span ) {
+                    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()));
+                                }
+                            }
+                        }
+                    }
+                },
+            }
+        }
+    }
+    fn nested_visit_map<'this>(
+        &'this mut self
+    ) -> hir::intravisit::NestedVisitorMap<'this, 'tcx> {
+        hir::intravisit::NestedVisitorMap::None
+    }
+}
diff --git a/src/test/ui/impl-trait/no-method-suggested-traits.stderr b/src/test/ui/impl-trait/no-method-suggested-traits.stderr
index 23f115858cd..793e1c66683 100644
--- a/src/test/ui/impl-trait/no-method-suggested-traits.stderr
+++ b/src/test/ui/impl-trait/no-method-suggested-traits.stderr
@@ -5,11 +5,16 @@ error[E0599]: no method named `method` found for type `u32` in the current scope
    |          ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
-           candidate #1: `use foo::Bar;`
-           candidate #2: `use no_method_suggested_traits::foo::PubPub;`
-           candidate #3: `use no_method_suggested_traits::qux::PrivPub;`
-           candidate #4: `use no_method_suggested_traits::Reexported;`
+help: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
+   |
+14 | use foo::Bar;
+   |
+14 | use no_method_suggested_traits::foo::PubPub;
+   |
+14 | use no_method_suggested_traits::qux::PrivPub;
+   |
+14 | use no_method_suggested_traits::Reexported;
+   |
 
 error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&u32>>` in the current scope
   --> $DIR/no-method-suggested-traits.rs:38:44
@@ -18,11 +23,16 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
    |                                            ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
-           candidate #1: `use foo::Bar;`
-           candidate #2: `use no_method_suggested_traits::foo::PubPub;`
-           candidate #3: `use no_method_suggested_traits::qux::PrivPub;`
-           candidate #4: `use no_method_suggested_traits::Reexported;`
+help: the following traits are implemented but not in scope, perhaps add a `use` for one of them:
+   |
+14 | use foo::Bar;
+   |
+14 | use no_method_suggested_traits::foo::PubPub;
+   |
+14 | use no_method_suggested_traits::qux::PrivPub;
+   |
+14 | use no_method_suggested_traits::Reexported;
+   |
 
 error[E0599]: no method named `method` found for type `char` in the current scope
   --> $DIR/no-method-suggested-traits.rs:44:9
@@ -31,8 +41,10 @@ error[E0599]: no method named `method` found for type `char` in the current scop
    |         ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
-           candidate #1: `use foo::Bar;`
+help: the following trait is implemented but not in scope, perhaps add a `use` for it:
+   |
+14 | use foo::Bar;
+   |
 
 error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&char>>` in the current scope
   --> $DIR/no-method-suggested-traits.rs:48:43
@@ -41,8 +53,10 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
    |                                           ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
-           candidate #1: `use foo::Bar;`
+help: the following trait is implemented but not in scope, perhaps add a `use` for it:
+   |
+14 | use foo::Bar;
+   |
 
 error[E0599]: no method named `method` found for type `i32` in the current scope
   --> $DIR/no-method-suggested-traits.rs:53:10
@@ -51,8 +65,10 @@ error[E0599]: no method named `method` found for type `i32` in the current scope
    |          ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
-           candidate #1: `use no_method_suggested_traits::foo::PubPub;`
+help: the following trait is implemented but not in scope, perhaps add a `use` for it:
+   |
+14 | use no_method_suggested_traits::foo::PubPub;
+   |
 
 error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::boxed::Box<&i32>>` in the current scope
   --> $DIR/no-method-suggested-traits.rs:57:44
@@ -61,8 +77,10 @@ error[E0599]: no method named `method` found for type `std::rc::Rc<&mut std::box
    |                                            ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
-           candidate #1: `use no_method_suggested_traits::foo::PubPub;`
+help: the following trait is implemented but not in scope, perhaps add a `use` for it:
+   |
+14 | use no_method_suggested_traits::foo::PubPub;
+   |
 
 error[E0599]: no method named `method` found for type `Foo` in the current scope
   --> $DIR/no-method-suggested-traits.rs:62:9
diff --git a/src/test/ui/issue-35976.stderr b/src/test/ui/issue-35976.stderr
index 9fb67449734..84bdbe3c697 100644
--- a/src/test/ui/issue-35976.stderr
+++ b/src/test/ui/issue-35976.stderr
@@ -4,8 +4,10 @@ error: the `wait` method cannot be invoked on a trait object
 24 |     arg.wait();
    |         ^^^^
    |
-   = note: another candidate was found in the following trait, perhaps add a `use` for it:
-           candidate #1: `use private::Future;`
+help: another candidate was found in the following trait, perhaps add a `use` for it:
+   |
+11 | use private::Future;
+   |
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/trait-method-private.stderr b/src/test/ui/trait-method-private.stderr
index c7a7b689edc..549c84b9b03 100644
--- a/src/test/ui/trait-method-private.stderr
+++ b/src/test/ui/trait-method-private.stderr
@@ -5,8 +5,10 @@ error[E0624]: method `method` is private
    |         ^^^^^^
    |
    = help: items from traits can only be used if the trait is in scope
-   = note: the following trait is implemented but not in scope, perhaps add a `use` for it:
-           candidate #1: `use inner::Bar;`
+help: the following trait is implemented but not in scope, perhaps add a `use` for it:
+   |
+11 | use inner::Bar;
+   |
 
 error: aborting due to previous error