Auto merge of #50969 - nikomatsakis:issue-50673-broken-migration-lint, r=alexcrichton

fix suggestions with nested paths

Fixes #50673

cc @Manishearth @petrochenkov
r? @alexcrichton

Author: bors

src/librustc_resolve/build_reduced_graph.rs

@@ -100,6 +100,8 @@ impl<'a> Resolver<'a> {
     }
 
     fn build_reduced_graph_for_use_tree(&mut self,
+                                        root_use_tree: &ast::UseTree,
+                                        root_id: NodeId,
                                         use_tree: &ast::UseTree,
                                         id: NodeId,
                                         vis: ty::Visibility,
@@ -182,7 +184,14 @@ impl<'a> Resolver<'a> {
                     type_ns_only,
                 };
                 self.add_import_directive(
-                    module_path, subclass, use_tree.span, id, vis, expansion,
+                    module_path,
+                    subclass,
+                    use_tree.span,
+                    id,
+                    root_use_tree.span,
+                    root_id,
+                    vis,
+                    expansion,
                 );
             }
             ast::UseTreeKind::Glob => {
@@ -191,7 +200,14 @@ impl<'a> Resolver<'a> {
                     max_vis: Cell::new(ty::Visibility::Invisible),
                 };
                 self.add_import_directive(
-                    module_path, subclass, use_tree.span, id, vis, expansion,
+                    module_path,
+                    subclass,
+                    use_tree.span,
+                    id,
+                    root_use_tree.span,
+                    root_id,
+                    vis,
+                    expansion,
                 );
             }
             ast::UseTreeKind::Nested(ref items) => {
@@ -226,7 +242,7 @@ impl<'a> Resolver<'a> {
 
                 for &(ref tree, id) in items {
                     self.build_reduced_graph_for_use_tree(
-                        tree, id, vis, &prefix, true, item, expansion
+                        root_use_tree, root_id, tree, id, vis, &prefix, true, item, expansion
                     );
                 }
             }
@@ -249,7 +265,7 @@ impl<'a> Resolver<'a> {
                 };
 
                 self.build_reduced_graph_for_use_tree(
-                    use_tree, item.id, vis, &prefix, false, item, expansion,
+                    use_tree, item.id, use_tree, item.id, vis, &prefix, false, item, expansion,
                 );
             }
 
@@ -266,10 +282,12 @@ impl<'a> Resolver<'a> {
                 let binding =
                     (module, ty::Visibility::Public, sp, expansion).to_name_binding(self.arenas);
                 let directive = self.arenas.alloc_import_directive(ImportDirective {
+                    root_id: item.id,
                     id: item.id,
                     parent,
                     imported_module: Cell::new(Some(module)),
                     subclass: ImportDirectiveSubclass::ExternCrate(orig_name),
+                    root_span: item.span,
                     span: item.span,
                     module_path: Vec::new(),
                     vis: Cell::new(vis),
@@ -640,10 +658,12 @@ impl<'a> Resolver<'a> {
 
         let (graph_root, arenas) = (self.graph_root, self.arenas);
         let macro_use_directive = |span| arenas.alloc_import_directive(ImportDirective {
+            root_id: item.id,
             id: item.id,
             parent: graph_root,
             imported_module: Cell::new(Some(module)),
             subclass: ImportDirectiveSubclass::MacroUse,
+            root_span: span,
             span,
             module_path: Vec::new(),
             vis: Cell::new(ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX))),

src/librustc_resolve/lib.rs

@@ -12,6 +12,7 @@
       html_favicon_url = "https://doc.rust-lang.org/favicon.ico",
       html_root_url = "https://doc.rust-lang.org/nightly/")]
 
+#![feature(crate_visibility_modifier)]
 #![feature(rustc_diagnostic_macros)]
 #![feature(slice_sort_by_cached_key)]
 
@@ -1634,11 +1635,17 @@ impl<'a> Resolver<'a> {
             .map(|seg| Ident::new(seg.name, span))
             .collect();
         // FIXME (Manishearth): Intra doc links won't get warned of epoch changes
-        match self.resolve_path(&path, Some(namespace), true, span, None) {
+        match self.resolve_path(&path, Some(namespace), true, span, CrateLint::No) {
             PathResult::Module(module) => *def = module.def().unwrap(),
             PathResult::NonModule(path_res) if path_res.unresolved_segments() == 0 =>
                 *def = path_res.base_def(),
-            PathResult::NonModule(..) => match self.resolve_path(&path, None, true, span, None) {
+            PathResult::NonModule(..) => match self.resolve_path(
+                &path,
+                None,
+                true,
+                span,
+                CrateLint::No,
+            ) {
                 PathResult::Failed(span, msg, _) => {
                     error_callback(self, span, ResolutionError::FailedToResolve(&msg));
                 }
@@ -2352,8 +2359,13 @@ impl<'a> Resolver<'a> {
             if def != Def::Err {
                 new_id = Some(def.def_id());
                 let span = trait_ref.path.span;
-                if let PathResult::Module(module) = self.resolve_path(&path, None, false, span,
-                                                                      Some(trait_ref.ref_id)) {
+                if let PathResult::Module(module) = self.resolve_path(
+                    &path,
+                    None,
+                    false,
+                    span,
+                    CrateLint::SimplePath(trait_ref.ref_id),
+                ) {
                     new_val = Some((module, trait_ref.clone()));
                 }
             }
@@ -2813,7 +2825,7 @@ impl<'a> Resolver<'a> {
                 } else {
                     let mod_path = &path[..path.len() - 1];
                     let mod_prefix = match this.resolve_path(mod_path, Some(TypeNS),
-                                                             false, span, None) {
+                                                             false, span, CrateLint::No) {
                         PathResult::Module(module) => module.def(),
                         _ => None,
                     }.map_or(format!(""), |def| format!("{} ", def.kind_name()));
@@ -3143,7 +3155,13 @@ impl<'a> Resolver<'a> {
             ));
         }
 
-        let result = match self.resolve_path(&path, Some(ns), true, span, Some(id)) {
+        let result = match self.resolve_path(
+            &path,
+            Some(ns),
+            true,
+            span,
+            CrateLint::SimplePath(id),
+        ) {
             PathResult::NonModule(path_res) => path_res,
             PathResult::Module(module) if !module.is_normal() => {
                 PathResolution::new(module.def().unwrap())
@@ -3180,7 +3198,13 @@ impl<'a> Resolver<'a> {
            path[0].name != keywords::CrateRoot.name() &&
            path[0].name != keywords::DollarCrate.name() {
             let unqualified_result = {
-                match self.resolve_path(&[*path.last().unwrap()], Some(ns), false, span, None) {
+                match self.resolve_path(
+                    &[*path.last().unwrap()],
+                    Some(ns),
+                    false,
+                    span,
+                    CrateLint::No,
+                ) {
                     PathResult::NonModule(path_res) => path_res.base_def(),
                     PathResult::Module(module) => module.def().unwrap(),
                     _ => return Some(result),
@@ -3195,14 +3219,14 @@ impl<'a> Resolver<'a> {
         Some(result)
     }
 
-    fn resolve_path(&mut self,
-                    path: &[Ident],
-                    opt_ns: Option<Namespace>, // `None` indicates a module path
-                    record_used: bool,
-                    path_span: Span,
-                    node_id: Option<NodeId>) // None indicates that we don't care about linting
-                                             // `::module` paths
-                    -> PathResult<'a> {
+    fn resolve_path(
+        &mut self,
+        path: &[Ident],
+        opt_ns: Option<Namespace>, // `None` indicates a module path
+        record_used: bool,
+        path_span: Span,
+        crate_lint: CrateLint,
+    ) -> PathResult<'a> {
         let mut module = None;
         let mut allow_super = true;
         let mut second_binding = None;
@@ -3321,7 +3345,7 @@ impl<'a> Resolver<'a> {
                         return PathResult::NonModule(err_path_resolution());
                     } else if opt_ns.is_some() && (is_last || maybe_assoc) {
                         self.lint_if_path_starts_with_module(
-                            node_id,
+                            crate_lint,
                             path,
                             path_span,
                             second_binding,
@@ -3366,19 +3390,22 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        self.lint_if_path_starts_with_module(node_id, path, path_span, second_binding);
+        self.lint_if_path_starts_with_module(crate_lint, path, path_span, second_binding);
 
         PathResult::Module(module.unwrap_or(self.graph_root))
     }
 
-    fn lint_if_path_starts_with_module(&self,
-                                       id: Option<NodeId>,
-                                       path: &[Ident],
-                                       path_span: Span,
-                                       second_binding: Option<&NameBinding>) {
-        let id = match id {
-            Some(id) => id,
-            None => return,
+    fn lint_if_path_starts_with_module(
+        &self,
+        crate_lint: CrateLint,
+        path: &[Ident],
+        path_span: Span,
+        second_binding: Option<&NameBinding>,
+    ) {
+        let (diag_id, diag_span) = match crate_lint {
+            CrateLint::No => return,
+            CrateLint::SimplePath(id) => (id, path_span),
+            CrateLint::UsePath { root_id, root_span } => (root_id, root_span),
         };
 
         let first_name = match path.get(0) {
@@ -3414,7 +3441,7 @@ impl<'a> Resolver<'a> {
             }
         }
 
-        self.lint_path_starts_with_module(id, path_span);
+        self.lint_path_starts_with_module(diag_id, diag_span);
     }
 
     fn lint_path_starts_with_module(&self, id: NodeId, span: Span) {
@@ -3650,7 +3677,7 @@ impl<'a> Resolver<'a> {
             // Search in module.
             let mod_path = &path[..path.len() - 1];
             if let PathResult::Module(module) = self.resolve_path(mod_path, Some(TypeNS),
-                                                                  false, span, None) {
+                                                                  false, span, CrateLint::No) {
                 add_module_candidates(module, &mut names);
             }
         }
@@ -4427,4 +4454,19 @@ pub enum MakeGlobMap {
     No,
 }
 
+enum CrateLint {
+    /// Do not issue the lint
+    No,
+
+    /// This lint applies to some random path like `impl ::foo::Bar`
+    /// or whatever. In this case, we can take the span of that path.
+    SimplePath(NodeId),
+
+    /// This lint comes from a `use` statement. In this case, what we
+    /// care about really is the *root* `use` statement; e.g., if we
+    /// have nested things like `use a::{b, c}`, we care about the
+    /// `use a` part.
+    UsePath { root_id: NodeId, root_span: Span },
+}
+
 __build_diagnostic_array! { librustc_resolve, DIAGNOSTICS }

src/librustc_resolve/macros.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use {AmbiguityError, Resolver, ResolutionError, resolve_error};
+use {AmbiguityError, CrateLint, Resolver, ResolutionError, resolve_error};
 use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult};
 use Namespace::{self, MacroNS};
 use build_reduced_graph::BuildReducedGraphVisitor;
@@ -436,7 +436,7 @@ impl<'a> Resolver<'a> {
                 return Err(Determinacy::Determined);
             }
 
-            let def = match self.resolve_path(&path, Some(MacroNS), false, span, None) {
+            let def = match self.resolve_path(&path, Some(MacroNS), false, span, CrateLint::No) {
                 PathResult::NonModule(path_res) => match path_res.base_def() {
                     Def::Err => Err(Determinacy::Determined),
                     def @ _ => {
@@ -613,7 +613,7 @@ impl<'a> Resolver<'a> {
     pub fn finalize_current_module_macro_resolutions(&mut self) {
         let module = self.current_module;
         for &(ref path, span) in module.macro_resolutions.borrow().iter() {
-            match self.resolve_path(&path, Some(MacroNS), true, span, None) {
+            match self.resolve_path(&path, Some(MacroNS), true, span, CrateLint::No) {
                 PathResult::NonModule(_) => {},
                 PathResult::Failed(span, msg, _) => {
                     resolve_error(self, span, ResolutionError::FailedToResolve(&msg));