Merge #7944

7944: Selecting `&mut foo` completion now actually inserts `&mut` r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/hir/src/lib.rs

@@ -1614,10 +1614,9 @@ impl Type {
     }
 
     pub fn remove_ref(&self) -> Option<Type> {
-        if let Ty::Ref(.., substs) = &self.ty.value {
-            Some(self.derived(substs[0].clone()))
-        } else {
-            None
+        match &self.ty.value {
+            Ty::Ref(.., substs) => Some(self.derived(substs[0].clone())),
+            _ => None,
         }
     }
 

crates/ide_completion/src/item.rs

@@ -68,7 +68,7 @@ pub struct CompletionItem {
 
     /// Indicates that a reference or mutable reference to this variable is a
     /// possible match.
-    ref_match: Option<(Mutability, CompletionScore)>,
+    ref_match: Option<Mutability>,
 
     /// The import data to add to completion's edits.
     import_to_add: Option<ImportEdit>,
@@ -104,6 +104,9 @@ impl fmt::Debug for CompletionItem {
         if let Some(score) = &self.score {
             s.field("score", score);
         }
+        if let Some(mutability) = &self.ref_match {
+            s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref()));
+        }
         if self.trigger_call_info {
             s.field("trigger_call_info", &true);
         }
@@ -261,7 +264,7 @@ impl CompletionItem {
         self.trigger_call_info
     }
 
-    pub fn ref_match(&self) -> Option<(Mutability, CompletionScore)> {
+    pub fn ref_match(&self) -> Option<Mutability> {
         self.ref_match
     }
 
@@ -311,7 +314,7 @@ pub(crate) struct Builder {
     deprecated: bool,
     trigger_call_info: Option<bool>,
     score: Option<CompletionScore>,
-    ref_match: Option<(Mutability, CompletionScore)>,
+    ref_match: Option<Mutability>,
 }
 
 impl Builder {
@@ -430,8 +433,8 @@ impl Builder {
         self.import_to_add = import_to_add;
         self
     }
-    pub(crate) fn ref_match(mut self, ref_match: (Mutability, CompletionScore)) -> Builder {
-        self.ref_match = Some(ref_match);
+    pub(crate) fn ref_match(mut self, mutability: Mutability) -> Builder {
+        self.ref_match = Some(mutability);
         self
     }
 }

crates/ide_completion/src/render.rs

@@ -254,10 +254,17 @@ impl<'a> Render<'a> {
                 {
                     item = item.set_score(score);
                 }
-                if let Some(ref_match) =
-                    refed_type_matches(&active_type, &active_name, &ty, &local_name)
-                {
-                    item = item.ref_match(ref_match);
+
+                if let Some(ty_without_ref) = active_type.remove_ref() {
+                    if ty_without_ref == ty {
+                        cov_mark::hit!(suggest_ref);
+                        let mutability = if active_type.is_mutable_reference() {
+                            Mutability::Mut
+                        } else {
+                            Mutability::Shared
+                        };
+                        item = item.ref_match(mutability)
+                    }
                 }
             }
         }
@@ -340,19 +347,6 @@ fn compute_score_from_active(
 
     Some(res)
 }
-fn refed_type_matches(
-    active_type: &Type,
-    active_name: &str,
-    ty: &Type,
-    name: &str,
-) -> Option<(Mutability, CompletionScore)> {
-    let derefed_active = active_type.remove_ref()?;
-    let score = compute_score_from_active(&derefed_active, &active_name, &ty, &name)?;
-    Some((
-        if active_type.is_mutable_reference() { Mutability::Mut } else { Mutability::Shared },
-        score,
-    ))
-}
 
 fn compute_score(ctx: &RenderContext, ty: &Type, name: &str) -> Option<CompletionScore> {
     let (active_name, active_type) = ctx.active_name_and_type()?;
@@ -947,4 +941,66 @@ fn f(foo: &Foo) { f(foo, w$0) }
             "#]],
         );
     }
+
+    #[test]
+    fn suggest_ref_mut() {
+        cov_mark::check!(suggest_ref);
+        check(
+            r#"
+struct S;
+fn foo(s: &mut S) {}
+fn main() {
+    let mut s = S;
+    foo($0);
+}
+            "#,
+            expect![[r#"
+                [
+                    CompletionItem {
+                        label: "S",
+                        source_range: 70..70,
+                        delete: 70..70,
+                        insert: "S",
+                        kind: SymbolKind(
+                            Struct,
+                        ),
+                    },
+                    CompletionItem {
+                        label: "foo(…)",
+                        source_range: 70..70,
+                        delete: 70..70,
+                        insert: "foo(${1:&mut s})$0",
+                        kind: SymbolKind(
+                            Function,
+                        ),
+                        lookup: "foo",
+                        detail: "-> ()",
+                        trigger_call_info: true,
+                    },
+                    CompletionItem {
+                        label: "main()",
+                        source_range: 70..70,
+                        delete: 70..70,
+                        insert: "main()$0",
+                        kind: SymbolKind(
+                            Function,
+                        ),
+                        lookup: "main",
+                        detail: "-> ()",
+                    },
+                    CompletionItem {
+                        label: "s",
+                        source_range: 70..70,
+                        delete: 70..70,
+                        insert: "s",
+                        kind: SymbolKind(
+                            Local,
+                        ),
+                        detail: "S",
+                        ref_match: "&mut ",
+                    },
+                ]
+            "#]],
+        )
+    }
 }

crates/rust-analyzer/src/to_proto.rs

@@ -175,12 +175,6 @@ pub(crate) fn completion_item(
     line_index: &LineIndex,
     item: CompletionItem,
 ) -> Vec<lsp_types::CompletionItem> {
-    fn set_score(lsp_item: &mut lsp_types::CompletionItem, label: &str) {
-        lsp_item.preselect = Some(true);
-        // HACK: sort preselect items first
-        lsp_item.sort_text = Some(format!(" {}", label));
-    }
-
     let mut additional_text_edits = Vec::new();
     let mut text_edit = None;
     // LSP does not allow arbitrary edits in completion, so we have to do a
@@ -220,7 +214,9 @@ pub(crate) fn completion_item(
     };
 
     if item.score().is_some() {
-        set_score(&mut lsp_item, item.label());
+        lsp_item.preselect = Some(true);
+        // HACK: sort preselect items first
+        lsp_item.sort_text = Some(format!(" {}", item.label()));
     }
 
     if item.deprecated() {
@@ -232,19 +228,23 @@ pub(crate) fn completion_item(
     }
 
     let mut res = match item.ref_match() {
-        Some(ref_match) => {
-            let mut refed = lsp_item.clone();
-            let (mutability, _score) = ref_match;
-            let label = format!("&{}{}", mutability.as_keyword_for_ref(), refed.label);
-            set_score(&mut refed, &label);
-            refed.label = label;
-            vec![lsp_item, refed]
+        Some(mutability) => {
+            let mut lsp_item_with_ref = lsp_item.clone();
+            lsp_item.preselect = Some(true);
+            lsp_item.sort_text = Some(format!(" {}", item.label()));
+            lsp_item_with_ref.label =
+                format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label);
+            if let Some(lsp_types::CompletionTextEdit::Edit(it)) = &mut lsp_item_with_ref.text_edit
+            {
+                it.new_text = format!("&{}{}", mutability.as_keyword_for_ref(), it.new_text);
+            }
+            vec![lsp_item_with_ref, lsp_item]
         }
         None => vec![lsp_item],
     };
 
-    for mut r in res.iter_mut() {
-        r.insert_text_format = Some(insert_text_format(item.insert_text_format()));
+    for lsp_item in res.iter_mut() {
+        lsp_item.insert_text_format = Some(insert_text_format(item.insert_text_format()));
     }
     res
 }
@@ -1105,13 +1105,13 @@ mod tests {
         expect_test::expect![[r#"
             [
                 (
-                    "arg",
+                    "&arg",
                     None,
                 ),
                 (
-                    "&arg",
+                    "arg",
                     Some(
-                        " &arg",
+                        " arg",
                     ),
                 ),
             ]