ambiguous traits from globs are all in scope + lint this

Author: LorrensP-2158466

compiler/rustc_hir/src/hir.rs

@@ -4555,6 +4555,8 @@ pub struct Upvar {
 pub struct TraitCandidate {
     pub def_id: DefId,
     pub import_ids: SmallVec<[LocalDefId; 1]>,
+    // FIXME: explain why this is needed for now.
+    pub lint_ambiguous: bool,
 }
 
 #[derive(Copy, Clone, Debug, HashStable_Generic)]

compiler/rustc_hir_typeck/src/method/confirm.rs

@@ -1,3 +1,4 @@
+use std::fmt::Debug;
 use std::ops::Deref;
 
 use rustc_hir as hir;
@@ -12,7 +13,7 @@ use rustc_hir_analysis::hir_ty_lowering::{
 use rustc_infer::infer::{
     BoundRegionConversionTime, DefineOpaqueTypes, InferOk, RegionVariableOrigin,
 };
-use rustc_lint::builtin::SUPERTRAIT_ITEM_SHADOWING_USAGE;
+use rustc_lint::builtin::{AMBIGUOUS_TRAIT_GLOB_IMPORTS, SUPERTRAIT_ITEM_SHADOWING_USAGE};
 use rustc_middle::traits::ObligationCauseCode;
 use rustc_middle::ty::adjustment::{
     Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
@@ -149,6 +150,9 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         // Lint when an item is shadowing a supertrait item.
         self.lint_shadowed_supertrait_items(pick, segment);
 
+        // Lint when a trait is ambiguously imported
+        self.lint_ambiguously_imported_trait(pick, segment);
+
         // Add any trait/regions obligations specified on the method's type parameters.
         // We won't add these if we encountered an illegal sized bound, so that we can use
         // a custom error in that case.
@@ -322,7 +326,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
                 })
             }
 
-            probe::TraitPick => {
+            probe::TraitPick(_) => {
                 let trait_def_id = pick.item.container_id(self.tcx);
 
                 // Make a trait reference `$0 : Trait<$1...$n>`
@@ -716,6 +720,26 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         );
     }
 
+    fn lint_ambiguously_imported_trait(
+        &self,
+        pick: &probe::Pick<'_>,
+        segment: &hir::PathSegment<'tcx>,
+    ) {
+        if let probe::PickKind::TraitPick(true) = pick.kind {
+        } else {
+            return;
+        };
+        let trait_name = self.tcx.item_name(pick.item.container_id(self.tcx));
+        let import_span = self.tcx.hir_span_if_local(pick.import_ids[0].to_def_id()).unwrap();
+
+        self.tcx.node_lint(AMBIGUOUS_TRAIT_GLOB_IMPORTS, segment.hir_id, |diag| {
+            diag.primary_message(format!("Usage of ambiguously imported trait `{trait_name}`"))
+                .span(segment.ident.span)
+                .span_label(import_span, format!("`{trait_name}`imported ambiguously here"))
+                .help(format!("Import `{trait_name}` explicitly"));
+        });
+    }
+
     fn upcast(
         &mut self,
         source_trait_ref: ty::PolyTraitRef<'tcx>,

compiler/rustc_hir_typeck/src/method/probe.rs

@@ -106,7 +106,7 @@ pub(crate) struct Candidate<'tcx> {
 pub(crate) enum CandidateKind<'tcx> {
     InherentImplCandidate { impl_def_id: DefId, receiver_steps: usize },
     ObjectCandidate(ty::PolyTraitRef<'tcx>),
-    TraitCandidate(ty::PolyTraitRef<'tcx>),
+    TraitCandidate(ty::PolyTraitRef<'tcx>, bool /* lint_ambiguous */),
     WhereClauseCandidate(ty::PolyTraitRef<'tcx>),
 }
 
@@ -235,7 +235,10 @@ pub(crate) struct Pick<'tcx> {
 pub(crate) enum PickKind<'tcx> {
     InherentImplPick,
     ObjectPick,
-    TraitPick,
+    TraitPick(
+        // Is Ambiguously Imported
+        bool,
+    ),
     WhereClausePick(
         // Trait
         ty::PolyTraitRef<'tcx>,
@@ -560,7 +563,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     probe_cx.push_candidate(
                         Candidate {
                             item,
-                            kind: CandidateKind::TraitCandidate(ty::Binder::dummy(trait_ref)),
+                            kind: CandidateKind::TraitCandidate(
+                                ty::Binder::dummy(trait_ref),
+                                false,
+                            ),
                             import_ids: smallvec![],
                         },
                         false,
@@ -1018,6 +1024,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                     self.assemble_extension_candidates_for_trait(
                         &trait_candidate.import_ids,
                         trait_did,
+                        trait_candidate.lint_ambiguous,
                     );
                 }
             }
@@ -1029,7 +1036,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         let mut duplicates = FxHashSet::default();
         for trait_info in suggest::all_traits(self.tcx) {
             if duplicates.insert(trait_info.def_id) {
-                self.assemble_extension_candidates_for_trait(&smallvec![], trait_info.def_id);
+                self.assemble_extension_candidates_for_trait(
+                    &smallvec![],
+                    trait_info.def_id,
+                    false,
+                );
             }
         }
     }
@@ -1055,6 +1066,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
         &mut self,
         import_ids: &SmallVec<[LocalDefId; 1]>,
         trait_def_id: DefId,
+        lint_ambiguous: bool,
     ) {
         let trait_args = self.fresh_args_for_item(self.span, trait_def_id);
         let trait_ref = ty::TraitRef::new_from_args(self.tcx, trait_def_id, trait_args);
@@ -1076,7 +1088,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                             Candidate {
                                 item,
                                 import_ids: import_ids.clone(),
-                                kind: TraitCandidate(bound_trait_ref),
+                                kind: TraitCandidate(bound_trait_ref, lint_ambiguous),
                             },
                             false,
                         );
@@ -1099,7 +1111,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                     Candidate {
                         item,
                         import_ids: import_ids.clone(),
-                        kind: TraitCandidate(ty::Binder::dummy(trait_ref)),
+                        kind: TraitCandidate(ty::Binder::dummy(trait_ref), lint_ambiguous),
                     },
                     false,
                 );
@@ -1842,7 +1854,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             ObjectCandidate(_) | WhereClauseCandidate(_) => {
                 CandidateSource::Trait(candidate.item.container_id(self.tcx))
             }
-            TraitCandidate(trait_ref) => self.probe(|_| {
+            TraitCandidate(trait_ref, _) => self.probe(|_| {
                 let trait_ref = self.instantiate_binder_with_fresh_vars(
                     self.span,
                     BoundRegionConversionTime::FnCall,
@@ -1872,7 +1884,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
     fn candidate_source_from_pick(&self, pick: &Pick<'tcx>) -> CandidateSource {
         match pick.kind {
             InherentImplPick => CandidateSource::Impl(pick.item.container_id(self.tcx)),
-            ObjectPick | WhereClausePick(_) | TraitPick => {
+            ObjectPick | WhereClausePick(_) | TraitPick(_) => {
                 CandidateSource::Trait(pick.item.container_id(self.tcx))
             }
         }
@@ -1948,7 +1960,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
                         impl_bounds,
                     ));
                 }
-                TraitCandidate(poly_trait_ref) => {
+                TraitCandidate(poly_trait_ref, _) => {
                     // Some trait methods are excluded for arrays before 2021.
                     // (`array.into_iter()` wants a slice iterator for compatibility.)
                     if let Some(method_name) = self.method_name {
@@ -2274,11 +2286,19 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             }
         }
 
+        let lint_ambiguous = match probes[0].0.kind {
+            TraitCandidate(_, lint) => lint,
+            WhereClauseCandidate(_) => false,
+            InherentImplCandidate { .. } | ObjectCandidate(_) => {
+                unreachable!()
+            }
+        };
+
         // FIXME: check the return type here somehow.
         // If so, just use this trait and call it a day.
         Some(Pick {
             item: probes[0].0.item,
-            kind: TraitPick,
+            kind: TraitPick(lint_ambiguous),
             import_ids: probes[0].0.import_ids.clone(),
             autoderefs: 0,
             autoref_or_ptr_adjustment: None,
@@ -2348,9 +2368,17 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             }
         }
 
+        let lint_ambiguous = match probes[0].0.kind {
+            TraitCandidate(_, lint) => lint,
+            WhereClauseCandidate(_) => false,
+            InherentImplCandidate { .. } | ObjectCandidate(_) => {
+                unreachable!()
+            }
+        };
+
         Some(Pick {
             item: child_candidate.item,
-            kind: TraitPick,
+            kind: TraitPick(lint_ambiguous),
             import_ids: child_candidate.import_ids.clone(),
             autoderefs: 0,
             autoref_or_ptr_adjustment: None,
@@ -2611,7 +2639,7 @@ impl<'tcx> Candidate<'tcx> {
             kind: match self.kind {
                 InherentImplCandidate { .. } => InherentImplPick,
                 ObjectCandidate(_) => ObjectPick,
-                TraitCandidate(_) => TraitPick,
+                TraitCandidate(_, lint_ambiguous) => TraitPick(lint_ambiguous),
                 WhereClauseCandidate(trait_ref) => {
                     // Only trait derived from where-clauses should
                     // appear here, so they should not contain any

compiler/rustc_lint_defs/src/builtin.rs

@@ -21,6 +21,7 @@ declare_lint_pass! {
         AMBIGUOUS_ASSOCIATED_ITEMS,
         AMBIGUOUS_GLOB_IMPORTS,
         AMBIGUOUS_GLOB_REEXPORTS,
+        AMBIGUOUS_TRAIT_GLOB_IMPORTS,
         ARITHMETIC_OVERFLOW,
         ASM_SUB_REGISTER,
         BAD_ASM_STYLE,
@@ -4437,6 +4438,47 @@ declare_lint! {
     };
 }
 
+declare_lint! {
+    /// The `ambiguous_trait_glob_imports` lint report traits that are imported ambiguously by glob imports,
+    /// but this wasn't done previously due to a rustc bug.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(ambiguous_trait_glob_imports)]
+    /// mod m1 {
+    ///    pub trait Trait {
+    ///            fn method1(&self) {}
+    ///        }
+    ///        impl Trait for u8 {}
+    ///    }
+    ///    mod m2 {
+    ///        pub trait Trait {
+    ///            fn method2(&self) {}
+    ///        }
+    ///        impl Trait for u8 {}
+    ///    }
+    ///
+    ///  fn main() {
+    ///      use m1::*;
+    ///      use m2::*;
+    ///      0u8.method1();
+    ///      0u8.method2();
+    ///  }
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// Previous versions of Rust compile this wrong and actually report that `method2`
+    /// can't be found. Now both traits are imported but a warning is emitted that they are ambiguous.
+    ///
+    pub AMBIGUOUS_TRAIT_GLOB_IMPORTS,
+    Warn,
+    "detects certain glob imports that import traits ambiguously and reports them.",
+}
+
 declare_lint! {
     /// The `refining_impl_trait_reachable` lint detects `impl Trait` return
     /// types in method signatures that are refined by a publically reachable

compiler/rustc_resolve/src/lib.rs

@@ -609,8 +609,18 @@ struct ModuleData<'ra> {
     globs: CmRefCell<Vec<Import<'ra>>>,
 
     /// Used to memoize the traits in this module for faster searches through all traits in scope.
-    traits:
-        CmRefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
+    traits: CmRefCell<
+        Option<
+            Box<
+                [(
+                    Macros20NormalizedIdent,
+                    NameBinding<'ra>,
+                    Option<Module<'ra>>,
+                    bool, /* lint_ambiguous*/
+                )],
+            >,
+        >,
+    >,
 
     /// Span of the module itself. Used for error reporting.
     span: Span,
@@ -702,12 +712,29 @@ impl<'ra> Module<'ra> {
         let mut traits = self.traits.borrow_mut(resolver.as_ref());
         if traits.is_none() {
             let mut collected_traits = Vec::new();
-            self.for_each_child(resolver, |r, name, ns, binding| {
-                if ns != TypeNS || binding.is_ambiguity_recursive() {
+            self.for_each_child(resolver, |r, name, ns, mut binding| {
+                if ns != TypeNS {
                     return;
                 }
-                if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() {
-                    collected_traits.push((name, binding, r.as_ref().get_module(def_id)))
+                let lint_ambiguous = binding.is_ambiguity_recursive();
+                loop {
+                    // Add to collected_traits if it's a trait
+                    if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() {
+                        collected_traits.push((
+                            name,
+                            binding,
+                            r.as_ref().get_module(def_id),
+                            lint_ambiguous,
+                        ));
+                    }
+
+                    // Break if no ambiguity
+                    let Some((ambiguous_binding, AmbiguityKind::GlobVsGlob)) = binding.ambiguity
+                    else {
+                        break;
+                    };
+
+                    binding = ambiguous_binding;
                 }
             });
             *traits = Some(collected_traits.into_boxed_slice());
@@ -1893,7 +1920,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         if let Some(module) = current_trait {
             if self.trait_may_have_item(Some(module), assoc_item) {
                 let def_id = module.def_id();
-                found_traits.push(TraitCandidate { def_id, import_ids: smallvec![] });
+                found_traits.push(TraitCandidate {
+                    def_id,
+                    import_ids: smallvec![],
+                    lint_ambiguous: false,
+                });
             }
         }
 
@@ -1928,11 +1959,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
     ) {
         module.ensure_traits(self);
         let traits = module.traits.borrow();
-        for &(trait_name, trait_binding, trait_module) in traits.as_ref().unwrap().iter() {
+        for &(trait_name, trait_binding, trait_module, lint_ambiguous) in
+            traits.as_ref().unwrap().iter()
+        {
             if self.trait_may_have_item(trait_module, assoc_item) {
                 let def_id = trait_binding.res().def_id();
                 let import_ids = self.find_transitive_imports(&trait_binding.kind, trait_name.0);
-                found_traits.push(TraitCandidate { def_id, import_ids });
+                found_traits.push(TraitCandidate { def_id, import_ids, lint_ambiguous });
             }
         }
     }

tests/ui/imports/ambiguous-trait-in-scope.rs

@@ -1,3 +1,5 @@
+//@ check-pass
+
 mod m1 {
     pub trait Trait {
         fn method1(&self) {}
@@ -11,20 +13,20 @@ mod m2 {
     impl Trait for u8 {}
 }
 
-fn test1() {
+pub fn test1() {
     // Create an ambiguous import for `Trait` in one order
     use m1::*;
     use m2::*;
-    0u8.method1(); //~ ERROR no method named `method1` found for type `u8` in the current scope
-    0u8.method2(); //~ ERROR no method named `method2` found for type `u8` in the current scope
+    0u8.method1(); //~ WARNING Usage of ambiguously imported trait `Trait` [ambiguous_trait_glob_imports]
+    0u8.method2(); //~ WARNING Usage of ambiguously imported trait `Trait` [ambiguous_trait_glob_imports]
 }
 
 fn test2() {
     // Create an ambiguous import for `Trait` in another order
     use m2::*;
     use m1::*;
-    0u8.method1(); //~ ERROR no method named `method1` found for type `u8` in the current scope
-    0u8.method2(); //~ ERROR no method named `method2` found for type `u8` in the current scope
+    0u8.method1(); //~ WARNING Usage of ambiguously imported trait `Trait` [ambiguous_trait_glob_imports]
+    0u8.method2(); //~ WARNING Usage of ambiguously imported trait `Trait` [ambiguous_trait_glob_imports]
 }
 
 fn main() {}