Point out shadowed associated types

Shadowing the associated type of a supertrait is allowed.
This however makes it impossible to set the associated type
of the supertrait in a dyn object.

This PR makes the error message for that case clearer, like
adding a note that shadowing is happening, as well as suggesting
renaming of one of the associated types.

r=petrochenckov

Author: TheLazyDutchman

compiler/rustc_hir_analysis/src/astconv/errors.rs

@@ -9,7 +9,7 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, Diagnostic, ErrorG
 use rustc_hir as hir;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_infer::traits::FulfillmentError;
-use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt};
+use rustc_middle::ty::{self, suggest_constraining_type_param, AssocItem, AssocKind, Ty, TyCtxt};
 use rustc_session::parse::feature_err;
 use rustc_span::edit_distance::find_best_match_for_name;
 use rustc_span::symbol::{sym, Ident};
@@ -509,6 +509,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
         if associated_types.values().all(|v| v.is_empty()) {
             return;
         }
+
         let tcx = self.tcx();
         // FIXME: Marked `mut` so that we can replace the spans further below with a more
         // appropriate one, but this should be handled earlier in the span assignment.
@@ -581,6 +582,32 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
             }
         }
 
+        // We get all the associated items that _are_ set,
+        // so that we can check if any of their names match one of the ones we are missing.
+        // This would mean that they are shadowing the associated type we are missing,
+        // and we can then use their span to indicate this to the user.
+        let bound_names = trait_bounds
+            .iter()
+            .filter_map(|poly_trait_ref| {
+                let path = poly_trait_ref.trait_ref.path.segments.last()?;
+                let args = path.args?;
+
+                Some(args.bindings.iter().filter_map(|binding| {
+                    let ident = binding.ident;
+                    let trait_def = path.res.def_id();
+                    let assoc_item = tcx.associated_items(trait_def).find_by_name_and_kind(
+                        tcx,
+                        ident,
+                        AssocKind::Type,
+                        trait_def,
+                    );
+
+                    Some((ident.name, assoc_item?))
+                }))
+            })
+            .flatten()
+            .collect::<FxHashMap<Symbol, &AssocItem>>();
+
         let mut names = names
             .into_iter()
             .map(|(trait_, mut assocs)| {
@@ -621,25 +648,51 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 *names.entry(item.name).or_insert(0) += 1;
             }
             let mut dupes = false;
+            let mut shadows = false;
             for item in assoc_items {
                 let prefix = if names[&item.name] > 1 {
                     let trait_def_id = item.container_id(tcx);
                     dupes = true;
                     format!("{}::", tcx.def_path_str(trait_def_id))
+                } else if bound_names.get(&item.name).is_some_and(|x| x != &item) {
+                    let trait_def_id = item.container_id(tcx);
+                    shadows = true;
+                    format!("{}::", tcx.def_path_str(trait_def_id))
                 } else {
                     String::new()
                 };
+
+                let mut is_shadowed = false;
+
+                if let Some(assoc_item) = bound_names.get(&item.name)
+                    && assoc_item != &item
+                {
+                    is_shadowed = true;
+
+                    let rename_message =
+                        if assoc_item.def_id.is_local() { ", consider renaming it" } else { "" };
+                    err.span_label(
+                        tcx.def_span(assoc_item.def_id),
+                        format!("`{}{}` shadowed here{}", prefix, item.name, rename_message),
+                    );
+                }
+
+                let rename_message = if is_shadowed { ", consider renaming it" } else { "" };
+
                 if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
-                    err.span_label(sp, format!("`{}{}` defined here", prefix, item.name));
+                    err.span_label(
+                        sp,
+                        format!("`{}{}` defined here{}", prefix, item.name, rename_message),
+                    );
                 }
             }
             if potential_assoc_types.len() == assoc_items.len() {
                 // When the amount of missing associated types equals the number of
                 // extra type arguments present. A suggesting to replace the generic args with
                 // associated types is already emitted.
                 already_has_generics_args_suggestion = true;
-            } else if let (Ok(snippet), false) =
-                (tcx.sess.source_map().span_to_snippet(*span), dupes)
+            } else if let (Ok(snippet), false, false) =
+                (tcx.sess.source_map().span_to_snippet(*span), dupes, shadows)
             {
                 let types: Vec<_> =
                     assoc_items.iter().map(|item| format!("{} = Type", item.name)).collect();
@@ -721,6 +774,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 err.span_help(where_constraints, where_msg);
             }
         }
+
         err.emit();
     }
 }

tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.rs

@@ -0,0 +1,15 @@
+// Test that no help message is emitted that suggests renaming the
+// associated type from a non-local trait
+
+pub trait NewIter: Iterator {
+    type Item;
+}
+
+impl<T> Clone for Box<dyn NewIter<Item = T>> {
+    //~^ ERROR the value of the associated type `Item` in `Iterator` must be specified
+    fn clone(&self) -> Self {
+        unimplemented!();
+    }
+}
+
+pub fn main() {}

tests/ui/associated-types/associated-type-shadowed-from-non-local-supertrait.stderr

@@ -0,0 +1,12 @@
+error[E0191]: the value of the associated type `Item` in `Iterator` must be specified
+  --> $DIR/associated-type-shadowed-from-non-local-supertrait.rs:8:27
+   |
+LL |     type Item;
+   |     --------- `Iterator::Item` shadowed here, consider renaming it
+...
+LL | impl<T> Clone for Box<dyn NewIter<Item = T>> {
+   |                           ^^^^^^^^^^^^^^^^^ associated type `Item` must be specified
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0191`.

tests/ui/associated-types/associated-type-shadowed-from-supertrait.rs

@@ -0,0 +1,19 @@
+// Test Setting the value of an associated type
+// that is shadowed from a supertrait
+
+pub trait Super {
+    type X;
+}
+
+pub trait Sub: Super {
+    type X;
+}
+
+impl<T> Clone for Box<dyn Sub<X = T>> {
+    //~^ ERROR value of the associated type `X` in `Super` must be specified
+    fn clone(&self) -> Self {
+        unimplemented!();
+    }
+}
+
+pub fn main() {}

tests/ui/associated-types/associated-type-shadowed-from-supertrait.stderr

@@ -0,0 +1,15 @@
+error[E0191]: the value of the associated type `X` in `Super` must be specified
+  --> $DIR/associated-type-shadowed-from-supertrait.rs:12:27
+   |
+LL |     type X;
+   |     ------ `Super::X` defined here, consider renaming it
+...
+LL |     type X;
+   |     ------ `Super::X` shadowed here, consider renaming it
+...
+LL | impl<T> Clone for Box<dyn Sub<X = T>> {
+   |                           ^^^^^^^^^^ associated type `X` must be specified
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0191`.