Lint ambiguous glob re-exports

3 files changed, 85 insertions, 20 deletions

diff --git a/compiler/rustc_resolve/src/effective_visibilities.rs b/compiler/rustc_resolve/src/effective_visibilities.rs
index 4bb252bfb29..a1ae9b8a521 100644
--- a/compiler/rustc_resolve/src/effective_visibilities.rs
+++ b/compiler/rustc_resolve/src/effective_visibilities.rs
@@ -4,6 +4,7 @@ use rustc_ast::visit;
 use rustc_ast::visit::Visitor;
 use rustc_ast::Crate;
 use rustc_ast::EnumDef;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::intern::Interned;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::def_id::CRATE_DEF_ID;
@@ -70,11 +71,11 @@ impl Resolver<'_, '_> {
 impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
     /// Fills the `Resolver::effective_visibilities` table with public & exported items
     /// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we
-    /// need access to a TyCtxt for that.
+    /// need access to a TyCtxt for that. Returns the set of ambiguous re-exports.
     pub(crate) fn compute_effective_visibilities<'c>(
         r: &'r mut Resolver<'a, 'tcx>,
         krate: &'c Crate,
-    ) {
+    ) -> FxHashSet<Interned<'a, NameBinding<'a>>> {
         let mut visitor = EffectiveVisibilitiesVisitor {
             r,
             def_effective_visibilities: Default::default(),
@@ -93,18 +94,26 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
         }
         visitor.r.effective_visibilities = visitor.def_effective_visibilities;
 
+        let mut exported_ambiguities = FxHashSet::default();
+
         // Update visibilities for import def ids. These are not used during the
         // `EffectiveVisibilitiesVisitor` pass, because we have more detailed binding-based
         // information, but are used by later passes. Effective visibility of an import def id
         // is the maximum value among visibilities of bindings corresponding to that def id.
         for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {
             let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
-            if let Some(node_id) = import.id() {
-                r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
+            if !binding.is_ambiguity() {
+                if let Some(node_id) = import.id() {
+                    r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
+                }
+            } else if binding.ambiguity.is_some() && eff_vis.is_public_at_level(Level::Reexported) {
+                exported_ambiguities.insert(*binding);
             }
         }
 
         info!("resolve::effective_visibilities: {:#?}", r.effective_visibilities);
+
+        exported_ambiguities
     }
 
     /// Update effective visibilities of bindings in the given module,
@@ -115,21 +124,44 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
         let resolutions = self.r.resolutions(module);
 
         for (_, name_resolution) in resolutions.borrow().iter() {
-            if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() {
-                // Set the given effective visibility level to `Level::Direct` and
-                // sets the rest of the `use` chain to `Level::Reexported` until
-                // we hit the actual exported item.
-                let mut parent_id = ParentId::Def(module_id);
-                while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
-                    let binding_id = ImportId::new_unchecked(binding);
-                    self.update_import(binding_id, parent_id);
-
-                    parent_id = ParentId::Import(binding_id);
-                    binding = nested_binding;
-                }
+            if let Some(mut binding) = name_resolution.borrow().binding() {
+                if !binding.is_ambiguity() {
+                    // Set the given effective visibility level to `Level::Direct` and
+                    // sets the rest of the `use` chain to `Level::Reexported` until
+                    // we hit the actual exported item.
+                    let mut parent_id = ParentId::Def(module_id);
+                    while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
+                    {
+                        let binding_id = ImportId::new_unchecked(binding);
+                        self.update_import(binding_id, parent_id);
+
+                        parent_id = ParentId::Import(binding_id);
+                        binding = nested_binding;
+                    }
+
+                    if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
+                        self.update_def(def_id, binding.vis.expect_local(), parent_id);
+                    }
+                } else {
+                    // Put the root ambiguity binding and all reexports leading to it into the
+                    // table. They are used by the `ambiguous_glob_reexports` lint. For all
+                    // bindings added to the table here `is_ambiguity` returns true.
+                    let mut parent_id = ParentId::Def(module_id);
+                    while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
+                    {
+                        let binding_id = ImportId::new_unchecked(binding);
+                        self.update_import(binding_id, parent_id);
 
-                if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
-                    self.update_def(def_id, binding.vis.expect_local(), parent_id);
+                        if binding.ambiguity.is_some() {
+                            // Stop at the root ambiguity, further bindings in the chain should not
+                            // be reexported because the root ambiguity blocks any access to them.
+                            // (Those further bindings are most likely not ambiguities themselves.)
+                            break;
+                        }
+
+                        parent_id = ParentId::Import(binding_id);
+                        binding = nested_binding;
+                    }
                 }
             }
         }
diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs
index 6e27bcc5bf3..b36fca9bb96 100644
--- a/compiler/rustc_resolve/src/imports.rs
+++ b/compiler/rustc_resolve/src/imports.rs
@@ -19,7 +19,9 @@ use rustc_hir::def::{self, DefKind, PartialRes};
 use rustc_middle::metadata::ModChild;
 use rustc_middle::span_bug;
 use rustc_middle::ty;
-use rustc_session::lint::builtin::{PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS};
+use rustc_session::lint::builtin::{
+    AMBIGUOUS_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS,
+};
 use rustc_session::lint::BuiltinLintDiagnostics;
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::hygiene::LocalExpnId;
@@ -506,6 +508,34 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
     }
 
+    pub(crate) fn check_reexport_ambiguities(
+        &mut self,
+        exported_ambiguities: FxHashSet<Interned<'a, NameBinding<'a>>>,
+    ) {
+        for module in self.arenas.local_modules().iter() {
+            module.for_each_child(self, |this, ident, ns, binding| {
+                if let NameBindingKind::Import { import, .. } = binding.kind
+                && let Some((amb_binding, _)) = binding.ambiguity
+                && binding.res() != Res::Err
+                && exported_ambiguities.contains(&Interned::new_unchecked(binding))
+                {
+                    this.lint_buffer.buffer_lint_with_diagnostic(
+                        AMBIGUOUS_GLOB_REEXPORTS,
+                        import.root_id,
+                        import.root_span,
+                        "ambiguous glob re-exports",
+                        BuiltinLintDiagnostics::AmbiguousGlobReexports {
+                            name: ident.to_string(),
+                            namespace: ns.descr().to_string(),
+                            first_reexport_span: import.root_span,
+                            duplicate_reexport_span: amb_binding.span,
+                        },
+                    );
+                }
+            });
+        }
+    }
+
     fn throw_unresolved_import_error(&self, errors: Vec<(&Import<'_>, UnresolvedImportError)>) {
         if errors.is_empty() {
             return;
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index cd90fd3ef84..de603d87e6d 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -1474,9 +1474,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
     pub fn resolve_crate(&mut self, krate: &Crate) {
         self.tcx.sess.time("resolve_crate", || {
             self.tcx.sess.time("finalize_imports", || self.finalize_imports());
-            self.tcx.sess.time("compute_effective_visibilities", || {
+            let exported_ambiguities = self.tcx.sess.time("compute_effective_visibilities", || {
                 EffectiveVisibilitiesVisitor::compute_effective_visibilities(self, krate)
             });
+            self.tcx.sess.time("check_reexport_ambiguities", || {
+                self.check_reexport_ambiguities(exported_ambiguities)
+            });
             self.tcx.sess.time("finalize_macro_resolutions", || self.finalize_macro_resolutions());
             self.tcx.sess.time("late_resolve_crate", || self.late_resolve_crate(krate));
             self.tcx.sess.time("resolve_main", || self.resolve_main());