Auto merge of #146874 - Enselic:multiple-adt-versions, r=<try>

compiler: Hint at multiple crate versions if trait impl is for wrong ADT

try-job: dist-various-1
try-job: aarch64-msvc-1

Author: rust-bors[bot]

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

@@ -467,7 +467,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                             span,
                             leaf_trait_predicate,
                         );
-                        self.note_version_mismatch(&mut err, leaf_trait_predicate);
+                        self.note_trait_version_mismatch(&mut err, 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);
 
@@ -2424,7 +2425,7 @@ 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_version_mismatch(
+    fn note_trait_version_mismatch(
         &self,
         err: &mut Diag<'_>,
         trait_pred: ty::PolyTraitPredicate<'tcx>,
@@ -2464,15 +2465,87 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
                 impl_spans,
                 format!("trait impl{} with same name found", pluralize!(trait_impls.len())),
             );
-            let trait_crate = self.tcx.crate_name(trait_with_same_path.krate);
-            let crate_msg =
-                format!("perhaps two different versions of crate `{trait_crate}` are being used?");
-            err.note(crate_msg);
+            self.note_two_crate_versions(trait_with_same_path, err);
             suggested = true;
         }
         suggested
     }
 
+    fn note_two_crate_versions(&self, did: DefId, 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);
+    }
+
+    fn note_adt_version_mismatch(
+        &self,
+        err: &mut Diag<'_>,
+        trait_pred: ty::PolyTraitPredicate<'tcx>,
+    ) {
+        let ty::Adt(impl_self_def, _) = trait_pred.self_ty().skip_binder().peel_refs().kind()
+        else {
+            return;
+        };
+
+        let impl_self_did = impl_self_def.did();
+
+        // We only want to warn about different versions of a dependency.
+        // If no dependency is involved, bail.
+        if impl_self_did.krate == LOCAL_CRATE {
+            return;
+        }
+
+        let impl_self_path = self.comparable_path(impl_self_did);
+        let impl_self_crate_name = self.tcx.crate_name(impl_self_did.krate);
+        let similar_items: UnordSet<_> = self
+            .tcx
+            .visible_parent_map(())
+            .items()
+            .filter_map(|(&item, _)| {
+                // If we found ourselves, ignore.
+                if impl_self_did == item {
+                    return None;
+                }
+                // We only want to warn about different versions of a dependency.
+                // Ignore items from our own crate.
+                if item.krate == LOCAL_CRATE {
+                    return None;
+                }
+                // We want to warn about different versions of a dependency.
+                // So make sure the crate names are the same.
+                if impl_self_crate_name != self.tcx.crate_name(item.krate) {
+                    return None;
+                }
+                // Filter out e.g. constructors that often have the same path
+                // str as the relevant ADT.
+                if !self.tcx.def_kind(item).is_adt() {
+                    return None;
+                }
+                let path = self.comparable_path(item);
+                // We don't know if our item or the one we found is the re-exported one.
+                // Check both cases.
+                let is_similar = path.ends_with(&impl_self_path) || impl_self_path.ends_with(&path);
+                is_similar.then_some((item, path))
+            })
+            .collect();
+
+        let mut similar_items =
+            similar_items.into_items().into_sorted_stable_ord_by_key(|(_, path)| path);
+        similar_items.dedup();
+
+        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);
+        }
+    }
+
+    /// Add a `::` prefix when comparing paths so that paths with just one item
+    /// like "Foo" does not equal the end of "OtherFoo".
+    fn comparable_path(&self, did: DefId) -> String {
+        format!("::{}", self.tcx.def_path_str(did))
+    }
+
     /// Creates a `PredicateObligation` with `new_self_ty` replacing the existing type in the
     /// `trait_ref`.
     ///

tests/run-make/duplicate-dependency/foo-v1.rs

@@ -0,0 +1 @@
+pub struct Foo;

tests/run-make/duplicate-dependency/foo-v2.rs

@@ -0,0 +1 @@
+pub struct Foo;

tests/run-make/duplicate-dependency/main.rs

@@ -0,0 +1,15 @@
+struct Bar;
+
+impl From<Bar> for foo::Foo {
+    fn from(_: Bar) -> Self {
+        foo::Foo
+    }
+}
+
+fn main() {
+    // The user might wrongly expect this to work since From<Bar> for Foo
+    // implies Into<Foo> for Bar. What the user missed is that different
+    // versions of Foo exist in the dependency graph, and the impl is for the
+    // wrong version.
+    re_export_foo::into_foo(Bar);
+}

tests/run-make/duplicate-dependency/main.stderr

@@ -0,0 +1,24 @@
+error[E0277]: the trait bound `re_export_foo::foo::Foo: From<Bar>` is not satisfied
+  --> main.rs:14:29
+   |
+LL |     re_export_foo::into_foo(Bar);
+   |     ----------------------- ^^^ the trait `From<Bar>` is not implemented for `re_export_foo::foo::Foo`
+   |     |
+   |     required by a bound introduced by this call
+   |
+help: item with same name found
+  --> $DIR/foo-v1.rs:1:1
+   |
+LL | pub struct Foo;
+   | ^^^^^^^^^^^^^^
+   = note: perhaps two different versions of crate `foo` are being used?
+   = note: required for `Bar` to implement `Into<re_export_foo::foo::Foo>`
+note: required by a bound in `into_foo`
+  --> $DIR/re-export-foo.rs:3:25
+   |
+LL | pub fn into_foo(_: impl Into<foo::Foo>) {}
+   |                         ^^^^^^^^^^^^^^ required by this bound in `into_foo`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

tests/run-make/duplicate-dependency/re-export-foo.rs

@@ -0,0 +1,3 @@
+pub use foo;
+
+pub fn into_foo(_: impl Into<foo::Foo>) {}

tests/run-make/duplicate-dependency/rmake.rs

@@ -0,0 +1,45 @@
+//@ needs-target-std
+
+use run_make_support::{Rustc, cwd, diff, rust_lib_name, rustc};
+
+fn rustc_with_common_args() -> Rustc {
+    let mut rustc = rustc();
+    rustc.remap_path_prefix(cwd(), "$DIR");
+    rustc.edition("2018"); // Don't require `extern crate`
+    rustc
+}
+
+fn main() {
+    rustc_with_common_args()
+        .input("foo-v1.rs")
+        .crate_type("rlib")
+        .crate_name("foo")
+        .extra_filename("-v1")
+        .metadata("-v1")
+        .run();
+
+    rustc_with_common_args()
+        .input("foo-v2.rs")
+        .crate_type("rlib")
+        .crate_name("foo")
+        .extra_filename("-v2")
+        .metadata("-v2")
+        .run();
+
+    rustc_with_common_args()
+        .input("re-export-foo.rs")
+        .crate_type("rlib")
+        .extern_("foo", rust_lib_name("foo-v2"))
+        .run();
+
+    let stderr = rustc_with_common_args()
+        .input("main.rs")
+        .extern_("foo", rust_lib_name("foo-v1"))
+        .extern_("re_export_foo", rust_lib_name("re_export_foo"))
+        .library_search_path(cwd())
+        .ui_testing()
+        .run_fail()
+        .stderr_utf8();
+
+    diff().expected_file("main.stderr").normalize(r"\\", "/").actual_text("(rustc)", &stderr).run();
+}