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.

Author: bors

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()
 }

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

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).
@@ -671,6 +674,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(
@@ -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}
+            "#]],
+        );
+    }
 }