Implement lint for definition site item shadowing too

rust-lang-ci · Jan 25, 2025 · 8409aaa · 8409aaa
1 parent f406879
commit 8409aaa
Show file tree

Hide file tree

Showing 14 changed files with 288 additions and 20 deletions.
diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl
@@ -497,6 +497,12 @@ hir_analysis_specialization_trait = implementing `rustc_specialization_trait` tr
 
 hir_analysis_static_specialize = cannot specialize on `'static` lifetime
 
+hir_analysis_supertrait_item_multiple_shadowee = items from several supertraits are shadowed: {$traits}
+
+hir_analysis_supertrait_item_shadowee = item from `{$supertrait}` is shadowed by a subtrait item
+
+hir_analysis_supertrait_item_shadowing = trait item `{$item}` from `{$subtrait}` shadows identically named item from supertrait
+
 hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
     .note = this item must mention the opaque type in its signature in order to be able to register hidden types
 

diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs
@@ -12,6 +12,7 @@ use rustc_hir::lang_items::LangItem;
 use rustc_hir::{AmbigArg, ItemKind};
 use rustc_infer::infer::outlives::env::OutlivesEnvironment;
 use rustc_infer::infer::{self, InferCtxt, TyCtxtInferExt};
+use rustc_lint_defs::builtin::SUPERTRAIT_ITEM_SHADOWING_DEFINITION;
 use rustc_macros::LintDiagnostic;
 use rustc_middle::mir::interpret::ErrorHandled;
 use rustc_middle::query::Providers;
@@ -377,7 +378,10 @@ fn check_trait_item<'tcx>(
         hir::TraitItemKind::Type(_bounds, Some(ty)) => (None, ty.span),
         _ => (None, trait_item.span),
     };
+
     check_dyn_incompatible_self_trait_by_name(tcx, trait_item);
+    check_item_shadowed_by_supertrait(tcx, def_id);
+
     let mut res = check_associated_item(tcx, def_id, span, method_sig);
 
     if matches!(trait_item.kind, hir::TraitItemKind::Fn(..)) {
@@ -892,6 +896,45 @@ fn check_dyn_incompatible_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitI
     }
 }
 
+fn check_item_shadowed_by_supertrait<'tcx>(tcx: TyCtxt<'tcx>, trait_item_def_id: LocalDefId) {
+    let item_name = tcx.item_name(trait_item_def_id.to_def_id());
+    let trait_def_id = tcx.local_parent(trait_item_def_id);
+
+    let shadowed: Vec<_> = traits::supertrait_def_ids(tcx, trait_def_id.to_def_id())
+        .skip(1)
+        .flat_map(|supertrait_def_id| {
+            tcx.associated_items(supertrait_def_id).filter_by_name_unhygienic(item_name)
+        })
+        .collect();
+    if !shadowed.is_empty() {
+        let shadowee = if let [shadowed] = shadowed[..] {
+            errors::SupertraitItemShadowee::Labeled {
+                span: tcx.def_span(shadowed.def_id),
+                supertrait: tcx.item_name(shadowed.trait_container(tcx).unwrap()),
+            }
+        } else {
+            let (traits, spans): (Vec<_>, Vec<_>) = shadowed
+                .iter()
+                .map(|item| {
+                    (tcx.item_name(item.trait_container(tcx).unwrap()), tcx.def_span(item.def_id))
+                })
+                .unzip();
+            errors::SupertraitItemShadowee::Several { traits: traits.into(), spans: spans.into() }
+        };
+
+        tcx.emit_node_span_lint(
+            SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
+            tcx.local_def_id_to_hir_id(trait_item_def_id),
+            tcx.def_span(trait_item_def_id),
+            errors::SupertraitItemShadowing {
+                item: item_name,
+                subtrait: tcx.item_name(trait_def_id.to_def_id()),
+                shadowee,
+            },
+        );
+    }
+}
+
 fn check_impl_item<'tcx>(
     tcx: TyCtxt<'tcx>,
     impl_item: &'tcx hir::ImplItem<'tcx>,

diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs
@@ -2,7 +2,8 @@
 
 use rustc_errors::codes::*;
 use rustc_errors::{
-    Applicability, Diag, DiagCtxtHandle, Diagnostic, EmissionGuarantee, Level, MultiSpan,
+    Applicability, Diag, DiagCtxtHandle, DiagSymbolList, Diagnostic, EmissionGuarantee, Level,
+    MultiSpan,
 };
 use rustc_macros::{Diagnostic, LintDiagnostic, Subdiagnostic};
 use rustc_middle::ty::Ty;
@@ -1702,3 +1703,28 @@ pub(crate) struct RegisterTypeUnstable<'a> {
     pub span: Span,
     pub ty: Ty<'a>,
 }
+
+#[derive(LintDiagnostic)]
+#[diag(hir_analysis_supertrait_item_shadowing)]
+pub(crate) struct SupertraitItemShadowing {
+    pub item: Symbol,
+    pub subtrait: Symbol,
+    #[subdiagnostic]
+    pub shadowee: SupertraitItemShadowee,
+}
+
+#[derive(Subdiagnostic)]
+pub(crate) enum SupertraitItemShadowee {
+    #[note(hir_analysis_supertrait_item_shadowee)]
+    Labeled {
+        #[primary_span]
+        span: Span,
+        supertrait: Symbol,
+    },
+    #[note(hir_analysis_supertrait_item_multiple_shadowee)]
+    Several {
+        #[primary_span]
+        spans: MultiSpan,
+        traits: DiagSymbolList,
+    },
+}
diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs
@@ -101,6 +101,7 @@ declare_lint_pass! {
         SINGLE_USE_LIFETIMES,
         SOFT_UNSTABLE,
         STABLE_FEATURES,
+        SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
         SUPERTRAIT_ITEM_SHADOWING_USAGE,
         TAIL_EXPR_DROP_ORDER,
         TEST_UNSTABLE_LINT,
@@ -5009,6 +5010,45 @@ declare_lint! {
     @feature_gate = supertrait_item_shadowing;
 }
 
+declare_lint! {
+    /// The `supertrait_item_shadowing_usage` lint detects when the
+    /// definition of an item that is provided by both a subtrait and
+    /// supertrait is shadowed, preferring the subtrait.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// #![feature(supertrait_item_shadowing)]
+    /// #![deny(supertrait_item_shadowing_definition)]
+    ///
+    /// trait Upstream {
+    ///     fn hello(&self) {}
+    /// }
+    /// impl<T> Upstream for T {}
+    ///
+    /// trait Downstream: Upstream {
+    ///     fn hello(&self) {}
+    /// }
+    /// impl<T> Downstream for T {}
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// RFC 3624 specified a heuristic in which a supertrait item would be
+    /// shadowed by a subtrait item when ambiguity occurs during item
+    /// selection. In order to mitigate side-effects of this happening
+    /// silently, this lint detects these cases when users want to deny them
+    /// or fix their trait definitions.
+    pub SUPERTRAIT_ITEM_SHADOWING_DEFINITION,
+    // FIXME(supertrait_item_shadowing): It is not decided if this should
+    // warn by default at the usage site.
+    Allow,
+    "detects when a supertrait item is shadowed by a subtrait item",
+    @feature_gate = supertrait_item_shadowing;
+}
+
 declare_lint! {
     /// The `ptr_to_integer_transmute_in_consts` lint detects pointer to integer
     /// transmute in const functions and associated constants.

diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor-2.rs b/tests/ui/methods/supertrait-shadowing/common-ancestor-2.rs
@@ -3,6 +3,7 @@
 
 #![feature(supertrait_item_shadowing)]
 #![warn(supertrait_item_shadowing_usage)]
+#![warn(supertrait_item_shadowing_definition)]
 #![allow(dead_code)]
 
 trait A {
@@ -21,6 +22,7 @@ impl<T> B for T {}
 
 trait C: A + B {
     fn hello(&self) {
+        //~^ WARN trait item `hello` from `C` shadows identically named item
         println!("C");
     }
 }

diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor-2.stderr b/tests/ui/methods/supertrait-shadowing/common-ancestor-2.stderr
@@ -1,16 +1,36 @@
 warning: trait item `hello` from `C` shadows identically named item from supertrait
-  --> $DIR/common-ancestor-2.rs:30:8
+  --> $DIR/common-ancestor-2.rs:24:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+   |
+note: items from several supertraits are shadowed: `B` and `A`
+  --> $DIR/common-ancestor-2.rs:10:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+...
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+note: the lint level is defined here
+  --> $DIR/common-ancestor-2.rs:6:9
+   |
+LL | #![warn(supertrait_item_shadowing_definition)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: trait item `hello` from `C` shadows identically named item from supertrait
+  --> $DIR/common-ancestor-2.rs:32:8
    |
 LL |     ().hello();
    |        ^^^^^
    |
 note: item from `C` shadows a supertrait item
-  --> $DIR/common-ancestor-2.rs:23:5
+  --> $DIR/common-ancestor-2.rs:24:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
 note: items from several supertraits are shadowed: `A` and `B`
-  --> $DIR/common-ancestor-2.rs:9:5
+  --> $DIR/common-ancestor-2.rs:10:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
@@ -23,5 +43,5 @@ note: the lint level is defined here
 LL | #![warn(supertrait_item_shadowing_usage)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: 1 warning emitted
+warning: 2 warnings emitted
 
diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor-3.rs b/tests/ui/methods/supertrait-shadowing/common-ancestor-3.rs
@@ -3,6 +3,7 @@
 
 #![feature(supertrait_item_shadowing)]
 #![warn(supertrait_item_shadowing_usage)]
+#![warn(supertrait_item_shadowing_definition)]
 #![allow(dead_code)]
 
 trait A {
@@ -21,6 +22,7 @@ impl<T> B for T {}
 
 trait C: A + B {
     fn hello(&self) {
+        //~^ WARN trait item `hello` from `C` shadows identically named item
         println!("C");
     }
 }
@@ -30,6 +32,7 @@ impl<T> C for T {}
 
 trait D: C {
     fn hello(&self) {
+        //~^ WARN trait item `hello` from `D` shadows identically named item
         println!("D");
     }
 }

diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor-3.stderr b/tests/ui/methods/supertrait-shadowing/common-ancestor-3.stderr
@@ -1,16 +1,54 @@
+warning: trait item `hello` from `C` shadows identically named item from supertrait
+  --> $DIR/common-ancestor-3.rs:24:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+   |
+note: items from several supertraits are shadowed: `B` and `A`
+  --> $DIR/common-ancestor-3.rs:10:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+...
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+note: the lint level is defined here
+  --> $DIR/common-ancestor-3.rs:6:9
+   |
+LL | #![warn(supertrait_item_shadowing_definition)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: trait item `hello` from `D` shadows identically named item from supertrait
+  --> $DIR/common-ancestor-3.rs:34:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+   |
+note: items from several supertraits are shadowed: `C`, `B`, and `A`
+  --> $DIR/common-ancestor-3.rs:10:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+...
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+...
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+
 warning: trait item `hello` from `D` shadows identically named item from supertrait
-  --> $DIR/common-ancestor-3.rs:39:8
+  --> $DIR/common-ancestor-3.rs:42:8
    |
 LL |     ().hello();
    |        ^^^^^
    |
 note: item from `D` shadows a supertrait item
-  --> $DIR/common-ancestor-3.rs:32:5
+  --> $DIR/common-ancestor-3.rs:34:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
 note: items from several supertraits are shadowed: `A`, `B`, and `C`
-  --> $DIR/common-ancestor-3.rs:9:5
+  --> $DIR/common-ancestor-3.rs:10:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
@@ -26,5 +64,5 @@ note: the lint level is defined here
 LL | #![warn(supertrait_item_shadowing_usage)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: 1 warning emitted
+warning: 3 warnings emitted
 
diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor.rs b/tests/ui/methods/supertrait-shadowing/common-ancestor.rs
@@ -3,6 +3,7 @@
 
 #![feature(supertrait_item_shadowing)]
 #![warn(supertrait_item_shadowing_usage)]
+#![warn(supertrait_item_shadowing_definition)]
 #![allow(dead_code)]
 
 trait A {
@@ -14,6 +15,7 @@ impl<T> A for T {}
 
 trait B: A {
     fn hello(&self) {
+        //~^ WARN trait item `hello` from `B` shadows identically named item
         println!("B");
     }
 }

diff --git a/tests/ui/methods/supertrait-shadowing/common-ancestor.stderr b/tests/ui/methods/supertrait-shadowing/common-ancestor.stderr
@@ -1,16 +1,33 @@
 warning: trait item `hello` from `B` shadows identically named item from supertrait
-  --> $DIR/common-ancestor.rs:23:8
+  --> $DIR/common-ancestor.rs:17:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+   |
+note: item from `A` is shadowed by a subtrait item
+  --> $DIR/common-ancestor.rs:10:5
+   |
+LL |     fn hello(&self) {
+   |     ^^^^^^^^^^^^^^^
+note: the lint level is defined here
+  --> $DIR/common-ancestor.rs:6:9
+   |
+LL | #![warn(supertrait_item_shadowing_definition)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: trait item `hello` from `B` shadows identically named item from supertrait
+  --> $DIR/common-ancestor.rs:25:8
    |
 LL |     ().hello();
    |        ^^^^^
    |
 note: item from `B` shadows a supertrait item
-  --> $DIR/common-ancestor.rs:16:5
+  --> $DIR/common-ancestor.rs:17:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
 note: item from `A` is shadowed by a subtrait item
-  --> $DIR/common-ancestor.rs:9:5
+  --> $DIR/common-ancestor.rs:10:5
    |
 LL |     fn hello(&self) {
    |     ^^^^^^^^^^^^^^^
@@ -20,5 +37,5 @@ note: the lint level is defined here
 LL | #![warn(supertrait_item_shadowing_usage)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-warning: 1 warning emitted
+warning: 2 warnings emitted