Skip to content

Commit

Permalink
fix: Fix trait method completions not acknowledging Deref impls
Browse files Browse the repository at this point in the history
  • Loading branch information
Veykril committed Aug 25, 2024
1 parent 191949e commit 737d508
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 33 deletions.
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

0 comments on commit 737d508

Please sign in to comment.