Skip to content

Commit

Permalink
Rollup merge of rust-lang#128786 - estebank:multiple-crate-versions, …
Browse files Browse the repository at this point in the history
…r=fee1-dead

Detect multiple crate versions on method not found

When a type comes indirectly from one crate version but the imported trait comes from a separate crate version, the called method won't be found. We now show additional context:

```
error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope
 --> multiple-dep-versions.rs:8:10
  |
8 |     Type.foo();
  |          ^^^ method not found in `Type`
  |
note: there are multiple different versions of crate `dependency` in the dependency graph
 --> multiple-dep-versions.rs:4:32
  |
4 | use dependency::{do_something, Trait};
  |                                ^^^^^ `dependency` imported here doesn't correspond to the right crate version
  |
 ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1
  |
4 | pub trait Trait {
  | --------------- this is the trait that was imported
  |
 ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:4:1
  |
4 | pub trait Trait {
  | --------------- this is the trait that is needed
5 |     fn foo(&self);
  |        --- the method is available for `dep_2_reexport::Type` here
```

Fix rust-lang#128569, fix rust-lang#110926, fix rust-lang#109161, fix rust-lang#81659, fix rust-lang#51458, fix rust-lang#32611. Follow up to rust-lang#124944.
  • Loading branch information
tgross35 committed Aug 17, 2024
2 parents 426a60a + 110b19b commit 02cf872
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 20 deletions.
97 changes: 94 additions & 3 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3448,6 +3448,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
trait_missing_method: bool,
) {
let mut alt_rcvr_sugg = false;
let mut trait_in_other_version_found = false;
if let (SelfSource::MethodCall(rcvr), false) = (source, unsatisfied_bounds) {
debug!(
"suggest_traits_to_import: span={:?}, item_name={:?}, rcvr_ty={:?}, rcvr={:?}",
Expand Down Expand Up @@ -3489,8 +3490,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// self types and rely on the suggestion to `use` the trait from
// `suggest_valid_traits`.
let did = Some(pick.item.container_id(self.tcx));
let skip = skippable.contains(&did);
if pick.autoderefs == 0 && !skip {
if skippable.contains(&did) {
continue;
}
trait_in_other_version_found = self
.detect_and_explain_multiple_crate_versions(
err,
pick.item.def_id,
rcvr.hir_id,
*rcvr_ty,
);
if pick.autoderefs == 0 && !trait_in_other_version_found {
err.span_label(
pick.item.ident(self.tcx).span,
format!("the method is available for `{rcvr_ty}` here"),
Expand Down Expand Up @@ -3675,7 +3685,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}
if self.suggest_valid_traits(err, item_name, valid_out_of_scope_traits, true) {

if let SelfSource::QPath(ty) = source
&& !valid_out_of_scope_traits.is_empty()
&& let hir::TyKind::Path(path) = ty.kind
&& let hir::QPath::Resolved(_, path) = path
&& let Some(def_id) = path.res.opt_def_id()
&& let Some(assoc) = self
.tcx
.associated_items(valid_out_of_scope_traits[0])
.filter_by_name_unhygienic(item_name.name)
.next()
{
// See if the `Type::function(val)` where `function` wasn't found corresponds to a
// `Trait` that is imported directly, but `Type` came from a different version of the
// same crate.
let rcvr_ty = self.tcx.type_of(def_id).instantiate_identity();
trait_in_other_version_found = self.detect_and_explain_multiple_crate_versions(
err,
assoc.def_id,
ty.hir_id,
rcvr_ty,
);
}
if !trait_in_other_version_found
&& self.suggest_valid_traits(err, item_name, valid_out_of_scope_traits, true)
{
return;
}

Expand Down Expand Up @@ -4040,6 +4075,62 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn detect_and_explain_multiple_crate_versions(
&self,
err: &mut Diag<'_>,
item_def_id: DefId,
hir_id: hir::HirId,
rcvr_ty: Ty<'_>,
) -> bool {
let hir_id = self.tcx.parent_hir_id(hir_id);
let Some(traits) = self.tcx.in_scope_traits(hir_id) else { return false };
if traits.is_empty() {
return false;
}
let trait_def_id = self.tcx.parent(item_def_id);
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() {
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, format!("this is the trait that is needed"));
let descr = self.tcx.associated_item(item_def_id).descr();
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),
format!("this is the trait that was imported"),
);
}
err.span_note(multi_span, msg);
true
}

/// issue #102320, for `unwrap_or` with closure as argument, suggest `unwrap_or_else`
/// FIXME: currently not working for suggesting `map_or_else`, see #102408
pub(crate) fn suggest_else_fn_with_closure(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1689,11 +1689,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
err.highlighted_span_help(
span,
vec![
StringPart::normal("you have ".to_string()),
StringPart::normal("there are ".to_string()),
StringPart::highlighted("multiple different versions".to_string()),
StringPart::normal(" of crate `".to_string()),
StringPart::highlighted(format!("{name}")),
StringPart::normal("` in your dependency graph".to_string()),
StringPart::normal("` the your dependency graph".to_string()),
],
);
let candidates = if impl_candidates.is_empty() {
Expand Down
12 changes: 9 additions & 3 deletions tests/run-make/crate-loading/multiple-dep-versions-1.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
#![crate_name = "dependency"]
#![crate_type = "rlib"]
pub struct Type;
pub trait Trait {}
impl Trait for Type {}
pub struct Type(pub i32);
pub trait Trait {
fn foo(&self);
fn bar();
}
impl Trait for Type {
fn foo(&self) {}
fn bar() {}
}
pub fn do_something<X: Trait>(_: X) {}
12 changes: 9 additions & 3 deletions tests/run-make/crate-loading/multiple-dep-versions-2.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
#![crate_name = "dependency"]
#![crate_type = "rlib"]
pub struct Type(pub i32);
pub trait Trait {}
impl Trait for Type {}
pub struct Type;
pub trait Trait {
fn foo(&self);
fn bar();
}
impl Trait for Type {
fn foo(&self) {}
fn bar() {}
}
pub fn do_something<X: Trait>(_: X) {}
5 changes: 5 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-3.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#![crate_name = "foo"]
#![crate_type = "rlib"]

extern crate dependency;
pub use dependency::Type;
6 changes: 4 additions & 2 deletions tests/run-make/crate-loading/multiple-dep-versions.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
extern crate dep_2_reexport;
extern crate dependency;
use dep_2_reexport::do_something;
use dependency::Type;
use dep_2_reexport::Type;
use dependency::{do_something, Trait};

fn main() {
do_something(Type);
Type.foo();
Type::bar();
}
88 changes: 81 additions & 7 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,100 @@
//@ only-linux
//@ ignore-wasm32
//@ ignore-wasm64
// ignore-tidy-linelength

use run_make_support::{rust_lib_name, rustc};

fn main() {
rustc().input("multiple-dep-versions-1.rs").run();
rustc().input("multiple-dep-versions-2.rs").extra_filename("2").metadata("2").run();
rustc()
.input("multiple-dep-versions-3.rs")
.extern_("dependency", rust_lib_name("dependency2"))
.run();

rustc()
.input("multiple-dep-versions.rs")
.extern_("dependency", rust_lib_name("dependency"))
.extern_("dep_2_reexport", rust_lib_name("dependency2"))
.extern_("dep_2_reexport", rust_lib_name("foo"))
.run_fail()
.assert_stderr_contains(
"you have multiple different versions of crate `dependency` in your dependency graph",
r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied
--> multiple-dep-versions.rs:7:18
|
7 | do_something(Type);
| ------------ ^^^^ the trait `Trait` is not implemented for `dep_2_reexport::Type`
| |
| required by a bound introduced by this call
|
help: there are multiple different versions of crate `dependency` the your dependency graph
--> multiple-dep-versions.rs:1:1
|
1 | extern crate dep_2_reexport;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a dependency of crate `foo`
2 | extern crate dependency;
| ^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a direct dependency of the current crate"#,
)
.assert_stderr_contains(
r#"
3 | pub struct Type(pub i32);
| ^^^^^^^^^^^^^^^ this type implements the required trait
4 | pub trait Trait {
| --------------- this is the required trait"#,
)
.assert_stderr_contains(
r#"
3 | pub struct Type;
| ^^^^^^^^^^^^^^^ this type doesn't implement the required trait"#,
)
.assert_stderr_contains(
r#"
error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope
--> multiple-dep-versions.rs:8:10
|
8 | Type.foo();
| ^^^ method not found in `Type`
|
note: there are multiple different versions of crate `dependency` in the dependency graph"#,
)
.assert_stderr_contains(
r#"
4 | pub trait Trait {
| ^^^^^^^^^^^^^^^ this is the trait that is needed
5 | fn foo(&self);
| -------------- the method is available for `dep_2_reexport::Type` here
|
::: multiple-dep-versions.rs:4:32
|
4 | use dependency::{do_something, Trait};
| ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#,
)
.assert_stderr_contains(
"two types coming from two different versions of the same crate are different types \
even if they look the same",
r#"
4 | pub trait Trait {
| --------------- this is the trait that was imported"#,
)
.assert_stderr_contains("this type doesn't implement the required trait")
.assert_stderr_contains("this type implements the required trait")
.assert_stderr_contains("this is the required trait");
.assert_stderr_contains(
r#"
error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope
--> multiple-dep-versions.rs:9:11
|
9 | Type::bar();
| ^^^ function or associated item not found in `Type`
|
note: there are multiple different versions of crate `dependency` in the dependency graph"#,
)
.assert_stderr_contains(
r#"
4 | pub trait Trait {
| ^^^^^^^^^^^^^^^ this is the trait that is needed
5 | fn foo(&self);
6 | fn bar();
| --------- the associated function is available for `dep_2_reexport::Type` here
|
::: multiple-dep-versions.rs:4:32
|
4 | use dependency::{do_something, Trait};
| ----- `Trait` imported here doesn't correspond to the right version of crate `dependency`"#,
);
}

0 comments on commit 02cf872

Please sign in to comment.