Auto merge of #149058 - LorrensP-2158466:amb-trait-lint, r=<try>

FCW Lint when using an ambiguously glob imported trait

Author: rust-bors[bot]

compiler/rustc_hir/src/hir.rs

@@ -4503,6 +4503,11 @@ pub struct Upvar {
 pub struct TraitCandidate {
     pub def_id: DefId,
     pub import_ids: SmallVec<[LocalDefId; 1]>,
+    // Indicates whether this trait candidate is ambiguously glob imported
+    // in it's scope. Related to the AMBIGUOUS_GLOB_IMPORTED_TRAIT lint.
+    // If this is set to true and the trait is used as a result of method lookup, this
+    // lint is thrown.
+    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_GLOB_IMPORTED_TRAIT, 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_glob_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,25 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
         );
     }
 
+    fn lint_ambiguously_glob_imported_trait(
+        &self,
+        pick: &probe::Pick<'_>,
+        segment: &hir::PathSegment<'tcx>,
+    ) {
+        if pick.kind != probe::PickKind::TraitPick(true) {
+            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_GLOB_IMPORTED_TRAIT, segment.hir_id, |diag| {
+            diag.primary_message(format!("Use of ambiguously glob 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,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             }
         }
 
+        let lint_ambiguous = match probes[0].0.kind {
+            TraitCandidate(_, lint) => lint,
+            _ => false,
+        };
+
         // 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 +2365,14 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
             }
         }
 
+        let lint_ambiguous = match probes[0].0.kind {
+            TraitCandidate(_, lint) => lint,
+            _ => false,
+        };
+
         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 +2633,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

@@ -19,6 +19,7 @@ declare_lint_pass! {
         AARCH64_SOFTFLOAT_NEON,
         ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
         AMBIGUOUS_ASSOCIATED_ITEMS,
+        AMBIGUOUS_GLOB_IMPORTED_TRAIT,
         AMBIGUOUS_GLOB_IMPORTS,
         AMBIGUOUS_GLOB_REEXPORTS,
         ARITHMETIC_OVERFLOW,
@@ -4488,6 +4489,61 @@ declare_lint! {
     };
 }
 
+declare_lint! {
+    /// The `ambiguous_glob_imported_trait` lint reports uses of traits that are
+    /// imported ambiguously via glob imports. Previously, this was not enforced
+    /// due to a bug in rustc.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(ambiguous_glob_imported_trait)]
+    /// 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
+    ///
+    /// When multiple traits with the same name are brought into scope through glob imports,
+    /// one trait becomes the "primary" one while the others are shadowed. Methods from the
+    /// shadowed traits (e.g. `method2`) become inaccessible, while methods from the "primary"
+    /// trait (e.g. `method1`) still resolve. Ideally, none of the ambiguous traits would be in scope,
+    /// but we have to allow this for now because of backwards compatibility.
+    /// This lint reports uses of these "primary" traits that are ambiguous.
+    ///
+    /// This is a [future-incompatible] lint to transition this to a
+    /// hard error in the future.
+    ///
+    /// [future-incompatible]: ../index.md#future-incompatible-lints
+    pub AMBIGUOUS_GLOB_IMPORTED_TRAIT,
+    Warn,
+    "detects usages of ambiguously glob imported traits",
+    @future_incompatible = FutureIncompatibleInfo {
+        reason: FutureIncompatibilityReason::FutureReleaseError,
+        reference: "issue #147992 <https://github.com/rust-lang/rust/issues/147992>",
+        report_in_deps: false,
+    };
+}
+
 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

@@ -608,8 +608,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,
@@ -706,7 +716,12 @@ impl<'ra> Module<'ra> {
                     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)))
+                    collected_traits.push((
+                        name,
+                        binding,
+                        r.as_ref().get_module(def_id),
+                        binding.is_ambiguity_recursive(),
+                    ));
                 }
             });
             *traits = Some(collected_traits.into_boxed_slice());
@@ -1891,7 +1906,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,
+                });
             }
         }
 
@@ -1926,11 +1945,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 });
             }
         }
     }