Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #15026 - lowr:fix/deduplicate-compl-fields, r=Veykril
fix: deduplicate fields and types in completion
Fixes #15024
- `hir_ty::autoderef()` (which is only meant to be used outside `hir-ty`) now deduplicates types and completely resolves inference variables within.
- field completion now deduplicates fields of the same name and only picks such field of the first type in the deref chain.
| -rw-r--r-- | crates/hir-ty/src/autoderef.rs | 24 | ||||
| -rw-r--r-- | crates/hir/src/lib.rs | 8 | ||||
| -rw-r--r-- | crates/ide-completion/src/completions/dot.rs | 92 |
3 files changed, 118 insertions, 6 deletions
diff --git a/crates/hir-ty/src/autoderef.rs b/crates/hir-ty/src/autoderef.rs index f5b3f176b1..3860bccec8 100644 --- a/crates/hir-ty/src/autoderef.rs +++ b/crates/hir-ty/src/autoderef.rs @@ -22,17 +22,37 @@ pub(crate) enum AutoderefKind { Overloaded, } +/// Returns types that `ty` transitively dereferences to. This function is only meant to be used +/// outside `hir-ty`. +/// +/// It is guaranteed that: +/// - the yielded types don't contain inference variables (but may contain `TyKind::Error`). +/// - a type won't be yielded more than once; in other words, the returned iterator will stop if it +/// detects a cycle in the deref chain. pub fn autoderef( db: &dyn HirDatabase, env: Arc<TraitEnvironment>, ty: Canonical<Ty>, -) -> impl Iterator<Item = Canonical<Ty>> + '_ { +) -> impl Iterator<Item = Ty> { let mut table = InferenceTable::new(db, env); let ty = table.instantiate_canonical(ty); let mut autoderef = Autoderef::new(&mut table, ty); let mut v = Vec::new(); while let Some((ty, _steps)) = autoderef.next() { - v.push(autoderef.table.canonicalize(ty).value); + // `ty` may contain unresolved inference variables. Since there's no chance they would be + // resolved, just replace with fallback type. + let resolved = autoderef.table.resolve_completely(ty); + + // If the deref chain contains a cycle (e.g. `A` derefs to `B` and `B` derefs to `A`), we + // would revisit some already visited types. Stop here to avoid duplication. + // + // XXX: The recursion limit for `Autoderef` is currently 10, so `Vec::contains()` shouldn't + // be too expensive. Replace this duplicate check with `FxHashSet` if it proves to be more + // performant. + if v.contains(&resolved) { + break; + } + v.push(resolved); } v.into_iter() } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index a11e25c79a..cf7d7f6c4f 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -3818,14 +3818,16 @@ impl Type { } } - pub fn autoderef<'a>(&'a self, db: &'a dyn HirDatabase) -> impl Iterator<Item = Type> + 'a { + /// Returns types that this type dereferences to (including this type itself). The returned + /// iterator won't yield the same type more than once even if the deref chain contains a cycle. + pub fn autoderef(&self, db: &dyn HirDatabase) -> impl Iterator<Item = Type> + '_ { self.autoderef_(db).map(move |ty| self.derived(ty)) } - fn autoderef_<'a>(&'a self, db: &'a dyn HirDatabase) -> impl Iterator<Item = Ty> + 'a { + fn autoderef_(&self, db: &dyn HirDatabase) -> impl Iterator<Item = Ty> { // There should be no inference vars in types passed here let canonical = hir_ty::replace_errors_with_variables(&self.ty); - autoderef(db, self.env.clone(), canonical).map(|canonical| canonical.value) + autoderef(db, self.env.clone(), canonical) } // This would be nicer if it just returned an iterator, but that runs into diff --git a/crates/ide-completion/src/completions/dot.rs b/crates/ide-completion/src/completions/dot.rs index 57a784c45b..a00c6ecb91 100644 --- a/crates/ide-completion/src/completions/dot.rs +++ b/crates/ide-completion/src/completions/dot.rs @@ -105,9 +105,12 @@ fn complete_fields( mut named_field: impl FnMut(&mut Completions, hir::Field, hir::Type), mut tuple_index: impl FnMut(&mut Completions, usize, hir::Type), ) { + let mut seen_names = FxHashSet::default(); for receiver in receiver.autoderef(ctx.db) { for (field, ty) in receiver.fields(ctx.db) { - named_field(acc, field, ty); + if seen_names.insert(field.name(ctx.db)) { + named_field(acc, field, ty); + } } for (i, ty) in receiver.tuple_fields(ctx.db).into_iter().enumerate() { // Tuple fields are always public (tuple struct fields are handled above). @@ -672,6 +675,52 @@ impl T { } #[test] + fn test_field_no_same_name() { + check( + r#" +//- minicore: deref +struct A { field: u8 } +struct B { field: u16, another: u32 } +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd another u32 + fd field u8 + me deref() (use core::ops::Deref) fn(&self) -> &<Self as Deref>::Target + "#]], + ); + } + + #[test] + fn test_tuple_field_no_same_index() { + check( + r#" +//- minicore: deref +struct A(u8); +struct B(u16, u32); +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd 0 u8 + fd 1 u32 + me deref() (use core::ops::Deref) fn(&self) -> &<Self as Deref>::Target + "#]], + ); + } + + #[test] fn test_completion_works_in_consts() { check( r#" @@ -979,4 +1028,45 @@ fn test(thing: impl Encrypt) { "#]], ) } + + #[test] + fn only_consider_same_type_once() { + check( + r#" +//- minicore: deref +struct A(u8); +struct B(u16); +impl core::ops::Deref for A { + type Target = B; + fn deref(&self) -> &Self::Target { loop {} } +} +impl core::ops::Deref for B { + type Target = A; + fn deref(&self) -> &Self::Target { loop {} } +} +fn test(a: A) { + a.$0 +} +"#, + expect![[r#" + fd 0 u8 + me deref() (use core::ops::Deref) fn(&self) -> &<Self as Deref>::Target + "#]], + ); + } + + #[test] + fn no_inference_var_in_completion() { + check( + r#" +struct S<T>(T); +fn test(s: S<Unknown>) { + s.$0 +} +"#, + expect![[r#" + fd 0 {unknown} + "#]], + ); + } } |