From adc7b8ea2d0ff1dafecaa697638825463f4b8891 Mon Sep 17 00:00:00 2001
From: Florian Diebold <flodiebold@gmail.com>
Date: Tue, 10 Mar 2020 20:56:26 +0100
Subject: Fix completion with a partially unknown type

To test whether the receiver type matches for the impl, we unify the given self
type (in this case `HashSet<{unknown}>`) with the self type of the
impl (`HashSet<?0>`), but if the given self type contains Unknowns, they won't
be unified with the variables in those places. So we got a receiver type that
was different from the expected one, and concluded the impl doesn't match.

The fix is slightly hacky; if after the unification, our variables are still
there, we make them fall back to Unknown. This does make some sense though,
since we don't want to 'leak' the variables.

Fixes #3547.
---
 crates/ra_hir_ty/src/method_resolution.rs    | 28 ++++++++++++++++++++++---
 crates/ra_ide/src/completion/complete_dot.rs | 31 ++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/crates/ra_hir_ty/src/method_resolution.rs b/crates/ra_hir_ty/src/method_resolution.rs
index b7e8855fb..7f5e1469e 100644
--- a/crates/ra_hir_ty/src/method_resolution.rs
+++ b/crates/ra_hir_ty/src/method_resolution.rs
@@ -516,9 +516,31 @@ pub(crate) fn inherent_impl_substs(
     let self_ty_with_vars =
         Canonical { num_vars: vars.len() + self_ty.num_vars, value: self_ty_with_vars };
     let substs = super::infer::unify(&self_ty_with_vars, self_ty);
-    // we only want the substs for the vars we added, not the ones from self_ty
-    let result = substs.map(|s| s.suffix(vars.len()));
-    result
+    // We only want the substs for the vars we added, not the ones from self_ty.
+    // Also, if any of the vars we added are still in there, we replace them by
+    // Unknown. I think this can only really happen if self_ty contained
+    // Unknown, and in that case we want the result to contain Unknown in those
+    // places again.
+    substs.map(|s| fallback_bound_vars(s.suffix(vars.len()), self_ty.num_vars))
+}
+
+/// This replaces any 'free' Bound vars in `s` (i.e. those with indices past
+/// num_vars_to_keep) by `Ty::Unknown`.
+fn fallback_bound_vars(s: Substs, num_vars_to_keep: usize) -> Substs {
+    s.fold_binders(
+        &mut |ty, binders| {
+            if let Ty::Bound(idx) = &ty {
+                if *idx >= binders as u32 {
+                    Ty::Unknown
+                } else {
+                    ty
+                }
+            } else {
+                ty
+            }
+        },
+        num_vars_to_keep,
+    )
 }
 
 fn transform_receiver_ty(
diff --git a/crates/ra_ide/src/completion/complete_dot.rs b/crates/ra_ide/src/completion/complete_dot.rs
index f275305e2..d8f6f0d9d 100644
--- a/crates/ra_ide/src/completion/complete_dot.rs
+++ b/crates/ra_ide/src/completion/complete_dot.rs
@@ -718,4 +718,35 @@ mod tests {
         "###
         );
     }
+
+    #[test]
+    fn test_method_completion_3547() {
+        assert_debug_snapshot!(
+            do_ref_completion(
+                r"
+            struct HashSet<T> {}
+            impl<T> HashSet<T> {
+                pub fn the_method(&self) {}
+            }
+            fn foo() {
+                let s: HashSet<_>;
+                s.<|>
+            }
+            ",
+            ),
+            @r###"
+        [
+            CompletionItem {
+                label: "the_method()",
+                source_range: [201; 201),
+                delete: [201; 201),
+                insert: "the_method()$0",
+                kind: Method,
+                lookup: "the_method",
+                detail: "pub fn the_method(&self)",
+            },
+        ]
+        "###
+        );
+    }
 }
-- 
cgit v1.2.3