Auto merge of #117661 - TheLazyDutchman:point_out_shadowed_associated_types, r=petrochenkov

Added shadowed hint for overlapping associated types

Previously, when you tried to set an associated type that is shadowed by an associated type in a subtrait, like this:

```rust
trait A {
    type X;
}

trait B: A {
    type X; // note: this is legal
}

impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
    fn clone(&self) -> Self {
        todo!()
    }
}

you got a confusing error message, that says nothing about the shadowing:

error[E0719]: the value of the associated type `X` (from trait `B`) is already specified
 --> test.rs:9:34
  |
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                             ---  ^^^ re-bound here
  |                             |
  |                             `X` bound here first

error[E0191]: the value of the associated type `X` (from trait `A`) must be specified
 --> test.rs:9:27
  |
2 |     type X;
  |     ------ `X` defined here
...
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                           ^^^^^^^^^^^ help: specify the associated type: `B<X=Y, X=Y, X = Type>`

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0191, E0719.
For more information about an error, try `rustc --explain E0191`.
```

Now instead, the error shows that the associated type is shadowed, and suggests renaming as a potential fix.

```rust
error[E0719]: the value of the associated type `X` in trait `B` is already specified
 --> test.rs:9:34
  |
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                             ---  ^^^ re-bound here
  |                             |
  |                             `X` bound here first

error[E0191]: the value of the associated type `X` in `A` must be specified
 --> test.rs:9:27
  |
2 |     type X;
  |     ------ `A::X` defined here
...
6 |     type X; // note: this is legal
  |     ------ `A::X` shadowed here
...
9 | impl<Y> Clone for Box<dyn B<X=Y, X=Y>> {
  |                           ^^^^^^^^^^^ associated type `X` must be specified
  |
help: consider renaming this associated type
 --> test.rs:2:5
  |
2 |     type X;
  |     ^^^^^^
help: consider renaming this associated type
 --> test.rs:6:5
  |
6 |     type X; // note: this is legal
  |     ^^^^^^
```

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0191, E0719.
For more information about an error, try `rustc --explain E0191`.

The rename help message is only emitted when the trait is local. This is true both for the supertrait as for the subtrait.

There might be cases where you can use the fully qualified path (for instance, in a where clause), but this PR currently does not deal with that.

fixes #100109

(continues from #117642, because I didn't know renaming the branch would close the PR)

Author: bors

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`.