Add a note when a type implements a trait with the same name as the required one

This is useful when you have two dependencies that use different trait for
the same thing and with the same name. The user can accidentally implement
the bad one which might be confusing. This commits refactorizes existing
diagnostics about multiple different crates with the same version and adds
a note when similarly named traits are found. Both diagnostics are merged
into a single function.

Author: rperier

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -4089,7 +4089,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         err: &mut Diag<'_>,
         item_def_id: DefId,
         hir_id: hir::HirId,
-        rcvr_ty: Option<Ty<'_>>,
+        rcvr_ty: Option<Ty<'tcx>>,
     ) -> bool {
         let hir_id = self.tcx.parent_hir_id(hir_id);
         let Some(traits) = self.tcx.in_scope_traits(hir_id) else { return false };
@@ -4100,49 +4100,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         if !self.tcx.is_trait(trait_def_id) {
             return false;
         }
-        let krate = self.tcx.crate_name(trait_def_id.krate);
-        let name = self.tcx.item_name(trait_def_id);
-        let candidates: Vec<_> = traits
-            .iter()
-            .filter(|c| {
-                c.def_id.krate != trait_def_id.krate
-                    && self.tcx.crate_name(c.def_id.krate) == krate
-                    && self.tcx.item_name(c.def_id) == name
-            })
-            .map(|c| (c.def_id, c.import_ids.get(0).cloned()))
-            .collect();
-        if candidates.is_empty() {
+        let hir::Node::Expr(rcvr) = self.tcx.hir_node(hir_id) else {
             return false;
-        }
-        let item_span = self.tcx.def_span(item_def_id);
-        let msg = format!(
-            "there are multiple different versions of crate `{krate}` in the dependency graph",
-        );
-        let trait_span = self.tcx.def_span(trait_def_id);
-        let mut multi_span: MultiSpan = trait_span.into();
-        multi_span.push_span_label(trait_span, "this is the trait that is needed".to_string());
-        let descr = self.tcx.associated_item(item_def_id).descr();
-        let rcvr_ty =
-            rcvr_ty.map(|t| format!("`{t}`")).unwrap_or_else(|| "the receiver".to_string());
-        multi_span
-            .push_span_label(item_span, format!("the {descr} is available for {rcvr_ty} here"));
-        for (def_id, import_def_id) in candidates {
-            if let Some(import_def_id) = import_def_id {
-                multi_span.push_span_label(
-                    self.tcx.def_span(import_def_id),
-                    format!(
-                        "`{name}` imported here doesn't correspond to the right version of crate \
-                         `{krate}`",
-                    ),
-                );
-            }
-            multi_span.push_span_label(
-                self.tcx.def_span(def_id),
-                "this is the trait that was imported".to_string(),
-            );
-        }
-        err.span_note(multi_span, msg);
-        true
+        };
+        let trait_ref = ty::TraitRef::new(self.tcx, trait_def_id, rcvr_ty.into_iter());
+        let trait_pred = ty::Binder::dummy(ty::TraitPredicate {
+            trait_ref,
+            polarity: ty::PredicatePolarity::Positive,
+        });
+        let obligation = Obligation::new(self.tcx, self.misc(rcvr.span), self.param_env, trait_ref);
+        self.err_ctxt().note_different_trait_with_same_name(err, &obligation, trait_pred)
     }
 
     /// issue #102320, for `unwrap_or` with closure as argument, suggest `unwrap_or_else`

compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs

@@ -1,3 +1,4 @@
+// ignore-tidy-filelength
 use core::ops::ControlFlow;
 use std::borrow::Cow;
 use std::path::PathBuf;
@@ -467,7 +468,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                             span,
                             leaf_trait_predicate,
                         );
-                        self.note_trait_version_mismatch(&mut err, leaf_trait_predicate);
+                        self.note_different_trait_with_same_name(&mut err, &obligation, leaf_trait_predicate);
                         self.note_adt_version_mismatch(&mut err, leaf_trait_predicate);
                         self.suggest_remove_await(&obligation, &mut err);
                         self.suggest_derive(&obligation, &mut err, leaf_trait_predicate);
@@ -1949,115 +1950,6 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             impl_candidates
         };
 
-        // We'll check for the case where the reason for the mismatch is that the trait comes from
-        // one crate version and the type comes from another crate version, even though they both
-        // are from the same crate.
-        let trait_def_id = trait_pred.def_id();
-        let trait_name = self.tcx.item_name(trait_def_id);
-        let crate_name = self.tcx.crate_name(trait_def_id.krate);
-        if let Some(other_trait_def_id) = self.tcx.all_traits_including_private().find(|def_id| {
-            trait_name == self.tcx.item_name(trait_def_id)
-                && trait_def_id.krate != def_id.krate
-                && crate_name == self.tcx.crate_name(def_id.krate)
-        }) {
-            // We've found two different traits with the same name, same crate name, but
-            // different crate `DefId`. We highlight the traits.
-
-            let found_type =
-                if let ty::Adt(def, _) = trait_pred.self_ty().skip_binder().peel_refs().kind() {
-                    Some(def.did())
-                } else {
-                    None
-                };
-            let candidates = if impl_candidates.is_empty() {
-                alternative_candidates(trait_def_id)
-            } else {
-                impl_candidates.into_iter().map(|cand| cand.trait_ref).collect()
-            };
-            let mut span: MultiSpan = self.tcx.def_span(trait_def_id).into();
-            span.push_span_label(self.tcx.def_span(trait_def_id), "this is the required trait");
-            for (sp, label) in [trait_def_id, other_trait_def_id]
-                .iter()
-                // The current crate-version might depend on another version of the same crate
-                // (Think "semver-trick"). Do not call `extern_crate` in that case for the local
-                // crate as that doesn't make sense and ICEs (#133563).
-                .filter(|def_id| !def_id.is_local())
-                .filter_map(|def_id| self.tcx.extern_crate(def_id.krate))
-                .map(|data| {
-                    let dependency = if data.dependency_of == LOCAL_CRATE {
-                        "direct dependency of the current crate".to_string()
-                    } else {
-                        let dep = self.tcx.crate_name(data.dependency_of);
-                        format!("dependency of crate `{dep}`")
-                    };
-                    (
-                        data.span,
-                        format!("one version of crate `{crate_name}` used here, as a {dependency}"),
-                    )
-                })
-            {
-                span.push_span_label(sp, label);
-            }
-            let mut points_at_type = false;
-            if let Some(found_type) = found_type {
-                span.push_span_label(
-                    self.tcx.def_span(found_type),
-                    "this type doesn't implement the required trait",
-                );
-                for trait_ref in candidates {
-                    if let ty::Adt(def, _) = trait_ref.self_ty().peel_refs().kind()
-                        && let candidate_def_id = def.did()
-                        && let Some(name) = self.tcx.opt_item_name(candidate_def_id)
-                        && let Some(found) = self.tcx.opt_item_name(found_type)
-                        && name == found
-                        && candidate_def_id.krate != found_type.krate
-                        && self.tcx.crate_name(candidate_def_id.krate)
-                            == self.tcx.crate_name(found_type.krate)
-                    {
-                        // A candidate was found of an item with the same name, from two separate
-                        // versions of the same crate, let's clarify.
-                        let candidate_span = self.tcx.def_span(candidate_def_id);
-                        span.push_span_label(
-                            candidate_span,
-                            "this type implements the required trait",
-                        );
-                        points_at_type = true;
-                    }
-                }
-            }
-            span.push_span_label(self.tcx.def_span(other_trait_def_id), "this is the found trait");
-            err.highlighted_span_note(
-                span,
-                vec![
-                    StringPart::normal("there are ".to_string()),
-                    StringPart::highlighted("multiple different versions".to_string()),
-                    StringPart::normal(" of crate `".to_string()),
-                    StringPart::highlighted(format!("{crate_name}")),
-                    StringPart::normal("` in the dependency graph".to_string()),
-                ],
-            );
-            if points_at_type {
-                // We only clarify that the same type from different crate versions are not the
-                // same when we *find* the same type coming from different crate versions, otherwise
-                // it could be that it was a type provided by a different crate than the one that
-                // provides the trait, and mentioning this adds verbosity without clarification.
-                err.highlighted_note(vec![
-                    StringPart::normal(
-                        "two types coming from two different versions of the same crate are \
-                         different types "
-                            .to_string(),
-                    ),
-                    StringPart::highlighted("even if they look the same".to_string()),
-                ]);
-            }
-            err.highlighted_help(vec![
-                StringPart::normal("you can use `".to_string()),
-                StringPart::highlighted("cargo tree".to_string()),
-                StringPart::normal("` to explore your dependency tree".to_string()),
-            ]);
-            return true;
-        }
-
         if let [single] = &impl_candidates {
             // If we have a single implementation, try to unify it with the trait ref
             // that failed. This should uncover a better hint for what *is* implemented.
@@ -2422,10 +2314,18 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
         }
     }
 
-    /// If the `Self` type of the unsatisfied trait `trait_ref` implements a trait
-    /// with the same path as `trait_ref`, a help message about
-    /// a probable version mismatch is added to `err`
-    fn note_trait_version_mismatch(
+    fn get_extern_crate_renamed_symbol(&self, trait_def_id: DefId) -> Option<Symbol> {
+        if !trait_def_id.is_local()
+            && let Some(data) = self.tcx.extern_crate(trait_def_id.krate)
+            && let rustc_session::cstore::ExternCrateSource::Extern(def_id) = data.src
+        {
+            self.tcx.opt_item_name(def_id)
+        } else {
+            None
+        }
+    }
+
+    fn check_same_trait_different_version(
         &self,
         err: &mut Diag<'_>,
         trait_pred: ty::PolyTraitPredicate<'tcx>,
@@ -2442,40 +2342,72 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             trait_impls
         };
 
-        let required_trait_path = self.tcx.def_path_str(trait_pred.def_id());
+        let krate = self.tcx.crate_name(trait_pred.def_id().krate);
+        let name = self.tcx.item_name(trait_pred.def_id());
+        let locally_renamed_krate = self
+            .get_extern_crate_renamed_symbol(trait_pred.def_id())
+            .map_or(None, |s| if s != krate { Some(s) } else { None });
         let traits_with_same_path: UnordSet<_> = self
             .tcx
             .visible_traits()
-            .filter(|trait_def_id| *trait_def_id != trait_pred.def_id())
+            .filter(|trait_def_id| {
+                trait_def_id.krate != trait_pred.def_id().krate
+                    && (locally_renamed_krate
+                        == self.get_extern_crate_renamed_symbol(*trait_def_id)
+                        || self.tcx.crate_name(trait_def_id.krate) == krate)
+                    && self.tcx.item_name(trait_def_id) == name
+            })
             .map(|trait_def_id| (self.tcx.def_path_str(trait_def_id), trait_def_id))
-            .filter(|(p, _)| *p == required_trait_path)
             .collect();
 
         let traits_with_same_path =
             traits_with_same_path.into_items().into_sorted_stable_ord_by_key(|(p, _)| p);
         let mut suggested = false;
-        for (_, trait_with_same_path) in traits_with_same_path {
-            let trait_impls = get_trait_impls(trait_with_same_path);
-            if trait_impls.is_empty() {
-                continue;
-            }
-            let impl_spans: Vec<_> =
-                trait_impls.iter().map(|impl_def_id| self.tcx.def_span(*impl_def_id)).collect();
-            err.span_help(
-                impl_spans,
-                format!("trait impl{} with same name found", pluralize!(trait_impls.len())),
+        let mut trait_is_impl = false;
+
+        if !traits_with_same_path.is_empty() {
+            let mut span: MultiSpan = self.tcx.def_span(trait_pred.def_id()).into();
+            span.push_span_label(
+                self.tcx.def_span(trait_pred.def_id()),
+                "this is the required trait",
             );
-            self.note_two_crate_versions(trait_with_same_path, err);
             suggested = true;
+            for (_, trait_with_same_path) in &traits_with_same_path {
+                let trait_impls = get_trait_impls(*trait_with_same_path);
+                if trait_impls.is_empty() {
+                    continue;
+                }
+
+                for candidate_def_id in trait_impls {
+                    let Some(impl_trait_header) = self.tcx.impl_trait_header(candidate_def_id)
+                    else {
+                        continue;
+                    };
+                    let candidate_span =
+                        self.tcx.def_span(impl_trait_header.trait_ref.skip_binder().def_id);
+                    span.push_span_label(candidate_span, "this is the implemented trait");
+                    trait_is_impl = true;
+                }
+            }
+            if !trait_is_impl {
+                for (_, def_id) in traits_with_same_path {
+                    span.push_span_label(
+                        self.tcx.def_span(def_id),
+                        "this is the trait that was imported",
+                    );
+                }
+            }
+            self.note_two_crate_versions(trait_pred.def_id(), span, err);
         }
         suggested
     }
 
-    fn note_two_crate_versions(&self, did: DefId, err: &mut Diag<'_>) {
+    fn note_two_crate_versions(&self, did: DefId, sp: impl Into<MultiSpan>, err: &mut Diag<'_>) {
         let crate_name = self.tcx.crate_name(did.krate);
-        let crate_msg =
-            format!("perhaps two different versions of crate `{crate_name}` are being used?");
-        err.note(crate_msg);
+        let crate_msg = format!(
+            "there are multiple different versions of crate `{crate_name}` in the dependency graph"
+        );
+        err.span_note(sp, crate_msg);
     }
 
     fn note_adt_version_mismatch(
@@ -2536,8 +2468,76 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
 
         for (similar_item, _) in similar_items {
             err.span_help(self.tcx.def_span(similar_item), "item with same name found");
-            self.note_two_crate_versions(similar_item, err);
+            self.note_two_crate_versions(similar_item, MultiSpan::new(), err);
+        }
+    }
+
+    fn check_same_name_different_path(
+        &self,
+        err: &mut Diag<'_>,
+        obligation: &PredicateObligation<'tcx>,
+        trait_pred: ty::PolyTraitPredicate<'tcx>,
+    ) -> bool {
+        let mut suggested = false;
+        let trait_def_id = trait_pred.def_id();
+        let trait_has_same_params = |other_trait_def_id: DefId| -> bool {
+            let trait_generics = self.tcx.generics_of(trait_def_id);
+            let other_trait_generics = self.tcx.generics_of(other_trait_def_id);
+
+            if trait_generics.count() != other_trait_generics.count() {
+                return false;
+            }
+            trait_generics.own_params.iter().zip(other_trait_generics.own_params.iter()).all(
+                |(a, b)| {
+                    (matches!(a.kind, ty::GenericParamDefKind::Type { .. })
+                        && matches!(b.kind, ty::GenericParamDefKind::Type { .. }))
+                        || (matches!(a.kind, ty::GenericParamDefKind::Lifetime,)
+                            && matches!(b.kind, ty::GenericParamDefKind::Lifetime))
+                        || (matches!(a.kind, ty::GenericParamDefKind::Const { .. })
+                            && matches!(b.kind, ty::GenericParamDefKind::Const { .. }))
+                },
+            )
+        };
+        let trait_name = self.tcx.item_name(trait_def_id);
+        if let Some(other_trait_def_id) = self.tcx.all_traits_including_private().find(|def_id| {
+            trait_def_id != *def_id
+                && trait_name == self.tcx.item_name(def_id)
+                && trait_has_same_params(*def_id)
+                && self.predicate_must_hold_modulo_regions(&Obligation::new(
+                    self.tcx,
+                    obligation.cause.clone(),
+                    obligation.param_env,
+                    trait_pred.map_bound(|tr| ty::TraitPredicate {
+                        trait_ref: ty::TraitRef::new(self.tcx, *def_id, tr.trait_ref.args),
+                        ..tr
+                    }),
+                ))
+        }) {
+            err.note(format!(
+                "`{}` implements similarly named `{}`, but not `{}`",
+                trait_pred.self_ty(),
+                self.tcx.def_path_str(other_trait_def_id),
+                trait_pred.print_modifiers_and_trait_path()
+            ));
+            suggested = true;
+        }
+        suggested
+    }
+
+    /// If the `Self` type of the unsatisfied trait `trait_ref` implements a trait
+    /// with the same path as `trait_ref`, a help message about a multiple different
+    /// versions of the same crate is added to `err`. Otherwise if it implements another
+    /// trait with the same name, a note message about a similarly named trait is added to `err`.
+    pub fn note_different_trait_with_same_name(
+        &self,
+        err: &mut Diag<'_>,
+        obligation: &PredicateObligation<'tcx>,
+        trait_pred: ty::PolyTraitPredicate<'tcx>,
+    ) -> bool {
+        if self.check_same_trait_different_version(err, trait_pred) {
+            return true;
         }
+        self.check_same_name_different_path(err, obligation, trait_pred)
     }
 
     /// Add a `::` prefix when comparing paths so that paths with just one item

compiler/rustc_trait_selection/src/error_reporting/traits/mod.rs

@@ -1,6 +1,6 @@
 pub mod ambiguity;
 pub mod call_kind;
-mod fulfillment_errors;
+pub mod fulfillment_errors;
 pub mod on_unimplemented;
 pub mod on_unimplemented_condition;
 pub mod on_unimplemented_format;