Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix trait method completions not acknowledging Deref impls #17958

Merged
merged 1 commit into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/hir-def/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ pub type StaticLoc = AssocItemLoc<Static>;
impl_intern!(StaticId, StaticLoc, intern_static, lookup_intern_static);
impl_loc!(StaticLoc, id: Static, container: ItemContainerId);

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct TraitId(salsa::InternId);
pub type TraitLoc = ItemLoc<Trait>;
impl_intern!(TraitId, TraitLoc, intern_trait, lookup_intern_trait);
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-ty/src/method_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use crate::{
};

/// This is used as a key for indexing impls.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum TyFingerprint {
// These are lang item impls:
Str,
Expand Down
36 changes: 36 additions & 0 deletions crates/ide-completion/src/tests/flyimport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1633,3 +1633,39 @@ fn main() {
"#]],
);
}

#[test]
fn trait_impl_on_slice_method_on_deref_slice_type() {
check(
r#"
//- minicore: deref, sized
struct SliceDeref;
impl core::ops::Deref for SliceDeref {
type Target = [()];

fn deref(&self) -> &Self::Target {
&[]
}
}
fn main() {
SliceDeref.choose$0();
}
mod module {
pub(super) trait SliceRandom {
type Item;

fn choose(&self);
}

impl<T> SliceRandom for [T] {
type Item = T;

fn choose(&self) {}
}
}
"#,
expect![[r#"
me choose (use module::SliceRandom) fn(&self)
"#]],
);
}
81 changes: 51 additions & 30 deletions crates/ide-db/src/imports/import_assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,40 +531,61 @@ fn trait_applicable_items(
})
.collect();

trait_candidates.retain(|&candidate_trait_id| {
// we care about the following cases:
// 1. Trait's definition crate
// 2. Definition crates for all trait's generic arguments
// a. This is recursive for fundamental types: `Into<Box<A>> for ()`` is OK, but
// `Into<Vec<A>> for ()`` is *not*.
// 3. Receiver type definition crate
// a. This is recursive for fundamental types
let defining_crate_for_trait = Trait::from(candidate_trait_id).krate(db);
let Some(receiver) = trait_candidate.receiver_ty.fingerprint_for_trait_impl() else {
return false;
};

// in order to handle implied bounds through an associated type, keep any
// method receiver that matches `TyFingerprint::Unnameable`. this receiver
// won't be in `TraitImpls` anyways, as `TraitImpls` only contains actual
// implementations.
if matches!(receiver, TyFingerprint::Unnameable) {
return true;
let autoderef_method_receiver = {
let mut deref_chain = trait_candidate.receiver_ty.autoderef(db).collect::<Vec<_>>();
// As a last step, we can do array unsizing (that's the only unsizing that rustc does for method receivers!)
if let Some((ty, _len)) = deref_chain.last().and_then(|ty| ty.as_array(db)) {
let slice = Type::new_slice(ty);
deref_chain.push(slice);
}
deref_chain
.into_iter()
.filter_map(|ty| Some((ty.krate(db).into(), ty.fingerprint_for_trait_impl()?)))
.sorted()
.unique()
.collect::<Vec<_>>()
};

let definitions_exist_in_trait_crate = db
.trait_impls_in_crate(defining_crate_for_trait.into())
.has_impls_for_trait_and_self_ty(candidate_trait_id, receiver);
// can be empty if the entire deref chain is has no valid trait impl fingerprints
if autoderef_method_receiver.is_empty() {
return Default::default();
}

// this is a closure for laziness: if `definitions_exist_in_trait_crate` is true,
// we can avoid a second db lookup.
let definitions_exist_in_receiver_crate = || {
db.trait_impls_in_crate(trait_candidate.receiver_ty.krate(db).into())
.has_impls_for_trait_and_self_ty(candidate_trait_id, receiver)
};
// in order to handle implied bounds through an associated type, keep all traits if any
// type in the deref chain matches `TyFingerprint::Unnameable`. This fingerprint
// won't be in `TraitImpls` anyways, as `TraitImpls` only contains actual implementations.
if !autoderef_method_receiver
.iter()
.any(|(_, fingerprint)| matches!(fingerprint, TyFingerprint::Unnameable))
{
trait_candidates.retain(|&candidate_trait_id| {
// we care about the following cases:
// 1. Trait's definition crate
// 2. Definition crates for all trait's generic arguments
// a. This is recursive for fundamental types: `Into<Box<A>> for ()`` is OK, but
// `Into<Vec<A>> for ()`` is *not*.
// 3. Receiver type definition crate
// a. This is recursive for fundamental types
let defining_crate_for_trait = Trait::from(candidate_trait_id).krate(db);

let trait_impls_in_crate = db.trait_impls_in_crate(defining_crate_for_trait.into());
let definitions_exist_in_trait_crate =
autoderef_method_receiver.iter().any(|&(_, fingerprint)| {
trait_impls_in_crate
.has_impls_for_trait_and_self_ty(candidate_trait_id, fingerprint)
});
// this is a closure for laziness: if `definitions_exist_in_trait_crate` is true,
// we can avoid a second db lookup.
let definitions_exist_in_receiver_crate = || {
autoderef_method_receiver.iter().any(|&(krate, fingerprint)| {
db.trait_impls_in_crate(krate)
.has_impls_for_trait_and_self_ty(candidate_trait_id, fingerprint)
})
};

definitions_exist_in_trait_crate || definitions_exist_in_receiver_crate()
});
definitions_exist_in_trait_crate || definitions_exist_in_receiver_crate()
});
}

let mut located_imports = FxIndexSet::default();
let mut trait_import_paths = FxHashMap::default();
Expand Down
2 changes: 1 addition & 1 deletion crates/ide-diagnostics/src/handlers/unused_variables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) fn unused_variables(
return None;
}
let diagnostic_range = ctx.sema.diagnostics_display_range(ast);
// The range for the Actual Name. We don't want to replace the entire declarition. Using the diagnostic range causes issues within in Array Destructuring.
// The range for the Actual Name. We don't want to replace the entire declaration. Using the diagnostic range causes issues within in Array Destructuring.
let name_range = d
.local
.primary_source(ctx.sema.db)
Expand Down