Skip to content

Commit

Permalink
Rework suggestion method
Browse files Browse the repository at this point in the history
Make checking slightly cheaper (by restricting to the right item only).

Add tests.
  • Loading branch information
estebank committed Aug 10, 2024
1 parent 5c537df commit 882f4a4
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 65 deletions.
96 changes: 47 additions & 49 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3498,8 +3498,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.detect_and_explain_multiple_crate_versions(
err,
pick.item.def_id,
pick.item.ident(self.tcx).span,
rcvr.hir_id.owner,
rcvr.hir_id,
*rcvr_ty,
);
if pick.autoderefs == 0 && !trait_in_other_version_found {
Expand Down Expand Up @@ -3706,8 +3705,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
trait_in_other_version_found = self.detect_and_explain_multiple_crate_versions(
err,
assoc.def_id,
self.tcx.def_span(assoc.def_id),
ty.hir_id.owner,
ty.hir_id,
rcvr_ty,
);
}
Expand Down Expand Up @@ -4082,55 +4080,55 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
err: &mut Diag<'_>,
item_def_id: DefId,
item_span: Span,
owner: hir::OwnerId,
hir_id: hir::HirId,
rcvr_ty: Ty<'_>,
) -> bool {
let pick_name = self.tcx.crate_name(item_def_id.krate);
let trait_did = self.tcx.parent(item_def_id);
let trait_name = self.tcx.item_name(trait_did);
if let Some(map) = self.tcx.in_scope_traits_map(owner) {
for trait_candidate in map.to_sorted_stable_ord().into_iter().flat_map(|v| v.1.iter()) {
let crate_name = self.tcx.crate_name(trait_candidate.def_id.krate);
if trait_candidate.def_id.krate != item_def_id.krate && crate_name == pick_name {
let msg = format!(
"there are multiple different versions of crate `{crate_name}` in the \
dependency graph",
);
let candidate_name = self.tcx.item_name(trait_candidate.def_id);
if candidate_name == trait_name
&& let Some(def_id) = trait_candidate.import_ids.get(0)
{
let span = self.tcx.def_span(*def_id);
let mut multi_span: MultiSpan = span.into();
multi_span.push_span_label(
span,
format!(
"`{crate_name}` imported here doesn't correspond to the right \
crate version",
),
);
multi_span.push_span_label(
self.tcx.def_span(trait_candidate.def_id),
format!("this is the trait that was imported"),
);
multi_span.push_span_label(
self.tcx.def_span(trait_did),
format!("this is the trait that is needed"),
);
multi_span.push_span_label(
item_span,
format!("the method is available for `{rcvr_ty}` here"),
);
err.span_note(multi_span, msg);
} else {
err.note(msg);
}
return true;
}
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"));
multi_span
.push_span_label(item_span, format!("the method 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"),
);
}
false
err.span_note(multi_span, msg);
true
}

/// issue #102320, for `unwrap_or` with closure as argument, suggest `unwrap_or_else`
Expand Down
2 changes: 2 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
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) {}
2 changes: 2 additions & 0 deletions tests/run-make/crate-loading/multiple-dep-versions-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
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) {}
1 change: 1 addition & 0 deletions tests/run-make/crate-loading/multiple-dep-versions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ use dependency::{do_something, Trait};
fn main() {
do_something(Type);
Type.foo();
Type::bar();
}
79 changes: 63 additions & 16 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,67 @@ fn main() {
.extern_("dependency", rust_lib_name("dependency"))
.extern_("dep_2_reexport", rust_lib_name("foo"))
.run_fail()
.assert_stderr_contains(
"there are multiple different versions of crate `dependency` in the dependency graph",
)
.assert_stderr_contains(
"two types coming from two different versions of the same crate are different types \
even if they look the same",
)
.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(
"`dependency` imported here doesn't correspond to the right crate version",
)
.assert_stderr_contains("this is the trait that was imported")
.assert_stderr_contains("this is the trait that is needed")
.assert_stderr_contains("the method is available for `dep_2_reexport::Type` here");
.assert_stderr_contains(r#"error[E0277]: the trait bound `dep_2_reexport::Type: Trait` is not satisfied

Check failure on line 21 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
--> 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`

Check failure on line 33 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
2 | extern crate dependency;
| ^^^^^^^^^^^^^^^^^^^^^^^^ one version of crate `dependency` is used here, as a direct dependency of the current crate"#)

Check failure on line 35 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
.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`"#)

Check failure on line 61 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
.assert_stderr_contains(r#"
4 | pub trait Trait {
| --------------- this is the trait that was imported"#)
.assert_stderr_contains(r#"
error[E0599]: no function or associated item named `bar` found for struct `dep_2_reexport::Type` in the current scope

Check failure on line 66 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
--> 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 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`"#);

Check failure on line 83 in tests/run-make/crate-loading/rmake.rs

View workflow job for this annotation

GitHub Actions / PR - mingw-check-tidy

line longer than 100 chars
}

0 comments on commit 882f4a4

Please sign in to comment.