Auto merge of #144674 - rperier:add_note_if_a_type_impl_a_trait_with_the_same_name, r=lcnr

Add a diagnostic for similarly named traits

cc #133123

This is a first proposal, suggestions are welcome

Author: bors

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -4212,7 +4212,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 };
@@ -4223,49 +4223,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/infer/mod.rs

@@ -51,11 +51,8 @@ use std::path::PathBuf;
 use std::{cmp, fmt, iter};
 
 use rustc_abi::ExternAbi;
-use rustc_ast::join_path_syms;
 use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
-use rustc_errors::{
-    Applicability, Diag, DiagStyledString, IntoDiagArg, MultiSpan, StringPart, pluralize,
-};
+use rustc_errors::{Applicability, Diag, DiagStyledString, IntoDiagArg, StringPart, pluralize};
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::Visitor;
@@ -66,15 +63,12 @@ use rustc_middle::bug;
 use rustc_middle::dep_graph::DepContext;
 use rustc_middle::traits::PatternOriginExpr;
 use rustc_middle::ty::error::{ExpectedFound, TypeError, TypeErrorToStringExt};
-use rustc_middle::ty::print::{
-    PrintError, PrintTraitRefExt as _, WrapBinderMode, with_forced_trimmed_paths,
-};
+use rustc_middle::ty::print::{PrintTraitRefExt as _, WrapBinderMode, with_forced_trimmed_paths};
 use rustc_middle::ty::{
     self, List, ParamEnv, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable,
     TypeVisitableExt,
 };
-use rustc_span::def_id::LOCAL_CRATE;
-use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Pos, Span, Symbol, sym};
+use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Pos, Span, sym};
 use tracing::{debug, instrument};
 
 use crate::error_reporting::TypeErrCtxt;
@@ -216,213 +210,30 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
 
     /// Adds a note if the types come from similarly named crates
     fn check_and_note_conflicting_crates(&self, err: &mut Diag<'_>, terr: TypeError<'tcx>) -> bool {
-        // FIXME(estebank): unify with `report_similar_impl_candidates`. The message is similar,
-        // even if the logic needed to detect the case is very different.
-        use hir::def_id::CrateNum;
-        use rustc_hir::definitions::DisambiguatedDefPathData;
-        use ty::GenericArg;
-        use ty::print::Printer;
-
-        struct ConflictingPathPrinter<'tcx> {
-            tcx: TyCtxt<'tcx>,
-            segments: Vec<Symbol>,
-        }
-
-        impl<'tcx> Printer<'tcx> for ConflictingPathPrinter<'tcx> {
-            fn tcx<'a>(&'a self) -> TyCtxt<'tcx> {
-                self.tcx
-            }
-
-            fn print_region(&mut self, _region: ty::Region<'_>) -> Result<(), PrintError> {
-                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
-            }
-
-            fn print_type(&mut self, _ty: Ty<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
-            }
-
-            fn print_dyn_existential(
-                &mut self,
-                _predicates: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
-            ) -> Result<(), PrintError> {
-                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
-            }
-
-            fn print_const(&mut self, _ct: ty::Const<'tcx>) -> Result<(), PrintError> {
-                unreachable!(); // because `print_path_with_generic_args` ignores the `GenericArgs`
-            }
-
-            fn print_crate_name(&mut self, cnum: CrateNum) -> Result<(), PrintError> {
-                self.segments = vec![self.tcx.crate_name(cnum)];
-                Ok(())
-            }
-
-            fn print_path_with_qualified(
-                &mut self,
-                _self_ty: Ty<'tcx>,
-                _trait_ref: Option<ty::TraitRef<'tcx>>,
-            ) -> Result<(), PrintError> {
-                Err(fmt::Error)
-            }
-
-            fn print_path_with_impl(
-                &mut self,
-                _print_prefix: impl FnOnce(&mut Self) -> Result<(), PrintError>,
-                _self_ty: Ty<'tcx>,
-                _trait_ref: Option<ty::TraitRef<'tcx>>,
-            ) -> Result<(), PrintError> {
-                Err(fmt::Error)
-            }
-
-            fn print_path_with_simple(
-                &mut self,
-                print_prefix: impl FnOnce(&mut Self) -> Result<(), PrintError>,
-                disambiguated_data: &DisambiguatedDefPathData,
-            ) -> Result<(), PrintError> {
-                print_prefix(self)?;
-                self.segments.push(disambiguated_data.as_sym(true));
-                Ok(())
-            }
-
-            fn print_path_with_generic_args(
-                &mut self,
-                print_prefix: impl FnOnce(&mut Self) -> Result<(), PrintError>,
-                _args: &[GenericArg<'tcx>],
-            ) -> Result<(), PrintError> {
-                print_prefix(self)
-            }
-        }
-
-        let report_path_match = |err: &mut Diag<'_>, did1: DefId, did2: DefId, ty: &str| -> bool {
-            // Only report definitions from different crates. If both definitions
-            // are from a local module we could have false positives, e.g.
-            // let _ = [{struct Foo; Foo}, {struct Foo; Foo}];
-            if did1.krate != did2.krate {
-                let abs_path = |def_id| {
-                    let mut p = ConflictingPathPrinter { tcx: self.tcx, segments: vec![] };
-                    p.print_def_path(def_id, &[]).map(|_| p.segments)
-                };
-
-                // We compare strings because DefPath can be different for imported and
-                // non-imported crates.
-                let expected_str = self.tcx.def_path_str(did1);
-                let found_str = self.tcx.def_path_str(did2);
-                let Ok(expected_abs) = abs_path(did1) else { return false };
-                let Ok(found_abs) = abs_path(did2) else { return false };
-                let same_path = expected_str == found_str || expected_abs == found_abs;
-                if same_path {
-                    // We want to use as unique a type path as possible. If both types are "locally
-                    // known" by the same name, we use the "absolute path" which uses the original
-                    // crate name instead.
-                    let (expected, found) = if expected_str == found_str {
-                        (join_path_syms(&expected_abs), join_path_syms(&found_abs))
-                    } else {
-                        (expected_str, found_str)
-                    };
-
-                    // We've displayed "expected `a::b`, found `a::b`". We add context to
-                    // differentiate the different cases where that might happen.
-                    let expected_crate_name = self.tcx.crate_name(did1.krate);
-                    let found_crate_name = self.tcx.crate_name(did2.krate);
-                    let same_crate = expected_crate_name == found_crate_name;
-                    let expected_sp = self.tcx.def_span(did1);
-                    let found_sp = self.tcx.def_span(did2);
-
-                    let both_direct_dependencies = if !did1.is_local()
-                        && !did2.is_local()
-                        && let Some(data1) = self.tcx.extern_crate(did1.krate)
-                        && let Some(data2) = self.tcx.extern_crate(did2.krate)
-                        && data1.dependency_of == LOCAL_CRATE
-                        && data2.dependency_of == LOCAL_CRATE
-                    {
-                        // If both crates are directly depended on, we don't want to mention that
-                        // in the final message, as it is redundant wording.
-                        // We skip the case of semver trick, where one version of the local crate
-                        // depends on another version of itself by checking that both crates at play
-                        // are not the current one.
-                        true
-                    } else {
-                        false
-                    };
-
-                    let mut span: MultiSpan = vec![expected_sp, found_sp].into();
-                    span.push_span_label(
-                        self.tcx.def_span(did1),
-                        format!("this is the expected {ty} `{expected}`"),
-                    );
-                    span.push_span_label(
-                        self.tcx.def_span(did2),
-                        format!("this is the found {ty} `{found}`"),
-                    );
-                    for def_id in [did1, did2] {
-                        let crate_name = self.tcx.crate_name(def_id.krate);
-                        if !def_id.is_local()
-                            && let Some(data) = self.tcx.extern_crate(def_id.krate)
-                        {
-                            let descr = if same_crate {
-                                "one version of".to_string()
-                            } else {
-                                format!("one {ty} comes from")
-                            };
-                            let dependency = if both_direct_dependencies {
-                                if let rustc_session::cstore::ExternCrateSource::Extern(def_id) =
-                                    data.src
-                                    && let Some(name) = self.tcx.opt_item_name(def_id)
-                                {
-                                    format!(", which is renamed locally to `{name}`")
-                                } else {
-                                    String::new()
-                                }
-                            } else if data.dependency_of == LOCAL_CRATE {
-                                ", as a direct dependency of the current crate".to_string()
-                            } else {
-                                let dep = self.tcx.crate_name(data.dependency_of);
-                                format!(", as a dependency of crate `{dep}`")
-                            };
-                            span.push_span_label(
-                                data.span,
-                                format!("{descr} crate `{crate_name}` used here{dependency}"),
-                            );
-                        }
-                    }
-                    let msg = if (did1.is_local() || did2.is_local()) && same_crate {
-                        format!(
-                            "the crate `{expected_crate_name}` is compiled multiple times, \
-                             possibly with different configurations",
-                        )
-                    } else if same_crate {
-                        format!(
-                            "two different versions of crate `{expected_crate_name}` are being \
-                             used; two types coming from two different versions of the same crate \
-                             are different types even if they look the same",
-                        )
-                    } else {
-                        format!(
-                            "two types coming from two different crates are different types even \
-                             if they look the same",
-                        )
-                    };
-                    err.span_note(span, msg);
-                    if same_crate {
-                        err.help("you can use `cargo tree` to explore your dependency tree");
-                    }
-                    return true;
-                }
-            }
-            false
-        };
         match terr {
             TypeError::Sorts(ref exp_found) => {
                 // if they are both "path types", there's a chance of ambiguity
                 // due to different versions of the same crate
                 if let (&ty::Adt(exp_adt, _), &ty::Adt(found_adt, _)) =
                     (exp_found.expected.kind(), exp_found.found.kind())
                 {
-                    return report_path_match(err, exp_adt.did(), found_adt.did(), "type");
+                    return self.check_same_definition_different_crate(
+                        err,
+                        exp_adt.did(),
+                        [found_adt.did()].into_iter(),
+                        |did| vec![self.tcx.def_span(did)],
+                        "type",
+                    );
                 }
             }
             TypeError::Traits(ref exp_found) => {
-                return report_path_match(err, exp_found.expected, exp_found.found, "trait");
+                return self.check_same_definition_different_crate(
+                    err,
+                    exp_found.expected,
+                    [exp_found.found].into_iter(),
+                    |did| vec![self.tcx.def_span(did)],
+                    "trait",
+                );
             }
             _ => (), // FIXME(#22750) handle traits and stuff
         }