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 515a88b
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 59 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();
}
89 changes: 79 additions & 10 deletions tests/run-make/crate-loading/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,88 @@ fn main() {
.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",
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(
"two types coming from two different versions of the same crate are different types \
even if they look the same",
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("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",
r#"
3 | pub struct Type;
| ^^^^^^^^^^^^^^^ this type doesn't implement the required trait"#,
)
.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[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(
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
--> 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`"#,
);
}

0 comments on commit 515a88b

Please sign in to comment.