Skip to content

Commit 4aade70

Browse files
Implement Lint + change tests.
1 parent 68f11a1 commit 4aade70

File tree

8 files changed

+447
-20
lines changed

8 files changed

+447
-20
lines changed

compiler/rustc_hir/src/hir.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4564,6 +4564,11 @@ pub struct Upvar {
45644564
pub struct TraitCandidate {
45654565
pub def_id: DefId,
45664566
pub import_ids: SmallVec<[LocalDefId; 1]>,
4567+
// Indicates whether this trait candidate is ambiguously glob imported
4568+
// in it's scope. Related to the AMBIGUOUS_GLOB_IMPORTED_TRAIT lint.
4569+
// If this is set to true and the trait is used as a result of method lookup, this
4570+
// lint is thrown.
4571+
pub lint_ambiguous: bool,
45674572
}
45684573

45694574
#[derive(Copy, Clone, Debug, HashStable_Generic)]

compiler/rustc_hir_typeck/src/method/confirm.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::fmt::Debug;
12
use std::ops::Deref;
23

34
use rustc_hir as hir;
@@ -12,7 +13,9 @@ use rustc_hir_analysis::hir_ty_lowering::{
1213
use rustc_infer::infer::{
1314
BoundRegionConversionTime, DefineOpaqueTypes, InferOk, RegionVariableOrigin,
1415
};
15-
use rustc_lint::builtin::RESOLVING_TO_ITEMS_SHADOWING_SUPERTRAIT_ITEMS;
16+
use rustc_lint::builtin::{
17+
AMBIGUOUS_GLOB_IMPORTED_TRAIT, RESOLVING_TO_ITEMS_SHADOWING_SUPERTRAIT_ITEMS,
18+
};
1619
use rustc_middle::traits::ObligationCauseCode;
1720
use rustc_middle::ty::adjustment::{
1821
Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, PointerCoercion,
@@ -149,6 +152,9 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
149152
// Lint when an item is shadowing a supertrait item.
150153
self.lint_shadowed_supertrait_items(pick, segment);
151154

155+
// Lint when a trait is ambiguously imported
156+
self.lint_ambiguously_glob_imported_trait(pick, segment);
157+
152158
// Add any trait/regions obligations specified on the method's type parameters.
153159
// We won't add these if we encountered an illegal sized bound, so that we can use
154160
// a custom error in that case.
@@ -322,7 +328,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
322328
})
323329
}
324330

325-
probe::TraitPick => {
331+
probe::TraitPick(_) => {
326332
let trait_def_id = pick.item.container_id(self.tcx);
327333

328334
// Make a trait reference `$0 : Trait<$1...$n>`
@@ -716,6 +722,25 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
716722
);
717723
}
718724

725+
fn lint_ambiguously_glob_imported_trait(
726+
&self,
727+
pick: &probe::Pick<'_>,
728+
segment: &hir::PathSegment<'tcx>,
729+
) {
730+
if pick.kind != probe::PickKind::TraitPick(true) {
731+
return;
732+
}
733+
let trait_name = self.tcx.item_name(pick.item.container_id(self.tcx));
734+
let import_span = self.tcx.hir_span_if_local(pick.import_ids[0].to_def_id()).unwrap();
735+
736+
self.tcx.node_lint(AMBIGUOUS_GLOB_IMPORTED_TRAIT, segment.hir_id, |diag| {
737+
diag.primary_message(format!("Use of ambiguously glob imported trait `{trait_name}`"))
738+
.span(segment.ident.span)
739+
.span_label(import_span, format!("`{trait_name}`imported ambiguously here"))
740+
.help(format!("Import `{trait_name}` explicitly"));
741+
});
742+
}
743+
719744
fn upcast(
720745
&mut self,
721746
source_trait_ref: ty::PolyTraitRef<'tcx>,

compiler/rustc_hir_typeck/src/method/probe.rs

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ pub(crate) struct Candidate<'tcx> {
106106
pub(crate) enum CandidateKind<'tcx> {
107107
InherentImplCandidate { impl_def_id: DefId, receiver_steps: usize },
108108
ObjectCandidate(ty::PolyTraitRef<'tcx>),
109-
TraitCandidate(ty::PolyTraitRef<'tcx>),
109+
TraitCandidate(ty::PolyTraitRef<'tcx>, bool /* lint_ambiguous */),
110110
WhereClauseCandidate(ty::PolyTraitRef<'tcx>),
111111
}
112112

@@ -235,7 +235,10 @@ pub(crate) struct Pick<'tcx> {
235235
pub(crate) enum PickKind<'tcx> {
236236
InherentImplPick,
237237
ObjectPick,
238-
TraitPick,
238+
TraitPick(
239+
// Is Ambiguously Imported
240+
bool,
241+
),
239242
WhereClausePick(
240243
// Trait
241244
ty::PolyTraitRef<'tcx>,
@@ -560,7 +563,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
560563
probe_cx.push_candidate(
561564
Candidate {
562565
item,
563-
kind: CandidateKind::TraitCandidate(ty::Binder::dummy(trait_ref)),
566+
kind: CandidateKind::TraitCandidate(
567+
ty::Binder::dummy(trait_ref),
568+
false,
569+
),
564570
import_ids: smallvec![],
565571
},
566572
false,
@@ -1018,6 +1024,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
10181024
self.assemble_extension_candidates_for_trait(
10191025
&trait_candidate.import_ids,
10201026
trait_did,
1027+
trait_candidate.lint_ambiguous,
10211028
);
10221029
}
10231030
}
@@ -1029,7 +1036,11 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
10291036
let mut duplicates = FxHashSet::default();
10301037
for trait_info in suggest::all_traits(self.tcx) {
10311038
if duplicates.insert(trait_info.def_id) {
1032-
self.assemble_extension_candidates_for_trait(&smallvec![], trait_info.def_id);
1039+
self.assemble_extension_candidates_for_trait(
1040+
&smallvec![],
1041+
trait_info.def_id,
1042+
false,
1043+
);
10331044
}
10341045
}
10351046
}
@@ -1055,6 +1066,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
10551066
&mut self,
10561067
import_ids: &SmallVec<[LocalDefId; 1]>,
10571068
trait_def_id: DefId,
1069+
lint_ambiguous: bool,
10581070
) {
10591071
let trait_args = self.fresh_args_for_item(self.span, trait_def_id);
10601072
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> {
10761088
Candidate {
10771089
item,
10781090
import_ids: import_ids.clone(),
1079-
kind: TraitCandidate(bound_trait_ref),
1091+
kind: TraitCandidate(bound_trait_ref, lint_ambiguous),
10801092
},
10811093
false,
10821094
);
@@ -1099,7 +1111,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
10991111
Candidate {
11001112
item,
11011113
import_ids: import_ids.clone(),
1102-
kind: TraitCandidate(ty::Binder::dummy(trait_ref)),
1114+
kind: TraitCandidate(ty::Binder::dummy(trait_ref), lint_ambiguous),
11031115
},
11041116
false,
11051117
);
@@ -1842,7 +1854,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
18421854
ObjectCandidate(_) | WhereClauseCandidate(_) => {
18431855
CandidateSource::Trait(candidate.item.container_id(self.tcx))
18441856
}
1845-
TraitCandidate(trait_ref) => self.probe(|_| {
1857+
TraitCandidate(trait_ref, _) => self.probe(|_| {
18461858
let trait_ref = self.instantiate_binder_with_fresh_vars(
18471859
self.span,
18481860
BoundRegionConversionTime::FnCall,
@@ -1872,7 +1884,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
18721884
fn candidate_source_from_pick(&self, pick: &Pick<'tcx>) -> CandidateSource {
18731885
match pick.kind {
18741886
InherentImplPick => CandidateSource::Impl(pick.item.container_id(self.tcx)),
1875-
ObjectPick | WhereClausePick(_) | TraitPick => {
1887+
ObjectPick | WhereClausePick(_) | TraitPick(_) => {
18761888
CandidateSource::Trait(pick.item.container_id(self.tcx))
18771889
}
18781890
}
@@ -1948,7 +1960,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
19481960
impl_bounds,
19491961
));
19501962
}
1951-
TraitCandidate(poly_trait_ref) => {
1963+
TraitCandidate(poly_trait_ref, _) => {
19521964
// Some trait methods are excluded for arrays before 2021.
19531965
// (`array.into_iter()` wants a slice iterator for compatibility.)
19541966
if let Some(method_name) = self.method_name {
@@ -2274,11 +2286,16 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
22742286
}
22752287
}
22762288

2289+
let lint_ambiguous = match probes[0].0.kind {
2290+
TraitCandidate(_, lint) => lint,
2291+
_ => false,
2292+
};
2293+
22772294
// FIXME: check the return type here somehow.
22782295
// If so, just use this trait and call it a day.
22792296
Some(Pick {
22802297
item: probes[0].0.item,
2281-
kind: TraitPick,
2298+
kind: TraitPick(lint_ambiguous),
22822299
import_ids: probes[0].0.import_ids.clone(),
22832300
autoderefs: 0,
22842301
autoref_or_ptr_adjustment: None,
@@ -2348,9 +2365,14 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
23482365
}
23492366
}
23502367

2368+
let lint_ambiguous = match probes[0].0.kind {
2369+
TraitCandidate(_, lint) => lint,
2370+
_ => false,
2371+
};
2372+
23512373
Some(Pick {
23522374
item: child_candidate.item,
2353-
kind: TraitPick,
2375+
kind: TraitPick(lint_ambiguous),
23542376
import_ids: child_candidate.import_ids.clone(),
23552377
autoderefs: 0,
23562378
autoref_or_ptr_adjustment: None,
@@ -2613,7 +2635,7 @@ impl<'tcx> Candidate<'tcx> {
26132635
kind: match self.kind {
26142636
InherentImplCandidate { .. } => InherentImplPick,
26152637
ObjectCandidate(_) => ObjectPick,
2616-
TraitCandidate(_) => TraitPick,
2638+
TraitCandidate(_, lint_ambiguous) => TraitPick(lint_ambiguous),
26172639
WhereClauseCandidate(trait_ref) => {
26182640
// Only trait derived from where-clauses should
26192641
// appear here, so they should not contain any

compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ declare_lint_pass! {
1717
AARCH64_SOFTFLOAT_NEON,
1818
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
1919
AMBIGUOUS_ASSOCIATED_ITEMS,
20+
AMBIGUOUS_GLOB_IMPORTED_TRAIT,
2021
AMBIGUOUS_GLOB_IMPORTS,
2122
AMBIGUOUS_GLOB_REEXPORTS,
2223
ARITHMETIC_OVERFLOW,
@@ -4474,6 +4475,60 @@ declare_lint! {
44744475
};
44754476
}
44764477

4478+
declare_lint! {
4479+
/// The `ambiguous_glob_imported_trait` lint reports uses of traits that are
4480+
/// imported ambiguously via glob imports. Previously, this was not enforced
4481+
/// due to a bug in rustc.
4482+
///
4483+
/// ### Example
4484+
///
4485+
/// ```rust,compile_fail
4486+
/// #![deny(ambiguous_glob_imported_trait)]
4487+
/// mod m1 {
4488+
/// pub trait Trait {
4489+
/// fn method1(&self) {}
4490+
/// }
4491+
/// impl Trait for u8 {}
4492+
/// }
4493+
/// mod m2 {
4494+
/// pub trait Trait {
4495+
/// fn method2(&self) {}
4496+
/// }
4497+
/// impl Trait for u8 {}
4498+
/// }
4499+
///
4500+
/// fn main() {
4501+
/// use m1::*;
4502+
/// use m2::*;
4503+
/// 0u8.method1();
4504+
/// 0u8.method2();
4505+
/// }
4506+
/// ```
4507+
///
4508+
/// {{produces}}
4509+
///
4510+
/// ### Explanation
4511+
///
4512+
/// When multiple traits with the same name are brought into scope through glob imports,
4513+
/// one trait becomes the "primary" one while the others are shadowed. Methods from the
4514+
/// shadowed traits (e.g. `method2`) become inaccessible, while methods from the "primary"
4515+
/// trait (e.g. `method1`) still resolve. Ideally, none of the ambiguous traits would be in scope,
4516+
/// but we have to allow this for now because of backwards compatibility.
4517+
/// This lint reports uses of these "primary" traits that are ambiguous.
4518+
///
4519+
/// This is a [future-incompatible] lint to transition this to a
4520+
/// hard error in the future.
4521+
///
4522+
/// [future-incompatible]: ../index.md#future-incompatible-lints
4523+
pub AMBIGUOUS_GLOB_IMPORTED_TRAIT,
4524+
Warn,
4525+
"detects uses of ambiguously glob imported traits",
4526+
@future_incompatible = FutureIncompatibleInfo {
4527+
reason: fcw!(FutureReleaseError #147992),
4528+
report_in_deps: false,
4529+
};
4530+
}
4531+
44774532
declare_lint! {
44784533
/// The `refining_impl_trait_reachable` lint detects `impl Trait` return
44794534
/// types in method signatures that are refined by a publically reachable

compiler/rustc_resolve/src/lib.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -609,8 +609,18 @@ struct ModuleData<'ra> {
609609
globs: CmRefCell<Vec<Import<'ra>>>,
610610

611611
/// Used to memoize the traits in this module for faster searches through all traits in scope.
612-
traits:
613-
CmRefCell<Option<Box<[(Macros20NormalizedIdent, NameBinding<'ra>, Option<Module<'ra>>)]>>>,
612+
traits: CmRefCell<
613+
Option<
614+
Box<
615+
[(
616+
Macros20NormalizedIdent,
617+
NameBinding<'ra>,
618+
Option<Module<'ra>>,
619+
bool, /* lint_ambiguous*/
620+
)],
621+
>,
622+
>,
623+
>,
614624

615625
/// Span of the module itself. Used for error reporting.
616626
span: Span,
@@ -707,7 +717,12 @@ impl<'ra> Module<'ra> {
707717
return;
708718
}
709719
if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() {
710-
collected_traits.push((name, binding, r.as_ref().get_module(def_id)))
720+
collected_traits.push((
721+
name,
722+
binding,
723+
r.as_ref().get_module(def_id),
724+
binding.is_ambiguity_recursive(),
725+
));
711726
}
712727
});
713728
*traits = Some(collected_traits.into_boxed_slice());
@@ -1918,7 +1933,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19181933
if let Some(module) = current_trait {
19191934
if self.trait_may_have_item(Some(module), assoc_item) {
19201935
let def_id = module.def_id();
1921-
found_traits.push(TraitCandidate { def_id, import_ids: smallvec![] });
1936+
found_traits.push(TraitCandidate {
1937+
def_id,
1938+
import_ids: smallvec![],
1939+
lint_ambiguous: false,
1940+
});
19221941
}
19231942
}
19241943

@@ -1953,11 +1972,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
19531972
) {
19541973
module.ensure_traits(self);
19551974
let traits = module.traits.borrow();
1956-
for &(trait_name, trait_binding, trait_module) in traits.as_ref().unwrap().iter() {
1975+
for &(trait_name, trait_binding, trait_module, lint_ambiguous) in
1976+
traits.as_ref().unwrap().iter()
1977+
{
19571978
if self.trait_may_have_item(trait_module, assoc_item) {
19581979
let def_id = trait_binding.res().def_id();
19591980
let import_ids = self.find_transitive_imports(&trait_binding.kind, trait_name.0);
1960-
found_traits.push(TraitCandidate { def_id, import_ids });
1981+
found_traits.push(TraitCandidate { def_id, import_ids, lint_ambiguous });
19611982
}
19621983
}
19631984
}

0 commit comments

Comments
 (0)