fix: Fix auto-ref completions inserting into wrong locations

10 files changed, 98 insertions, 94 deletions

diff --git a/crates/ide-completion/src/completions/flyimport.rs b/crates/ide-completion/src/completions/flyimport.rs
index 129910465c2..101784fc14c 100644
--- a/crates/ide-completion/src/completions/flyimport.rs
+++ b/crates/ide-completion/src/completions/flyimport.rs
@@ -169,6 +169,8 @@ pub(crate) fn import_on_the_fly_pat(
             has_macro_bang: false,
             qualified: Qualified::No,
             parent: None,
+            // FIXME
+            path: syntax::ast::make::ext::ident_path("dummy__"),
             kind: crate::context::PathKind::Pat { pat_ctx: pat_ctx.clone() },
             has_type_args: false,
             use_tree_parent: false,
diff --git a/crates/ide-completion/src/completions/pattern.rs b/crates/ide-completion/src/completions/pattern.rs
index 91d53565410..67dbbf2a29c 100644
--- a/crates/ide-completion/src/completions/pattern.rs
+++ b/crates/ide-completion/src/completions/pattern.rs
@@ -88,6 +88,8 @@ pub(crate) fn complete_pattern(
                             has_call_parens: false,
                             has_macro_bang: false,
                             qualified: Qualified::No,
+                            // FIXME
+                            path: syntax::ast::make::ext::ident_path("dummy__"),
                             parent: None,
                             kind: crate::context::PathKind::Pat { pat_ctx: pattern_ctx.clone() },
                             has_type_args: false,
@@ -95,7 +97,7 @@ pub(crate) fn complete_pattern(
                         },
                         mac,
                         name,
-                    )
+                    );
                 }
                 _ => false,
             },
diff --git a/crates/ide-completion/src/context.rs b/crates/ide-completion/src/context.rs
index 441f7ad70bf..8ea03358aeb 100644
--- a/crates/ide-completion/src/context.rs
+++ b/crates/ide-completion/src/context.rs
@@ -61,6 +61,8 @@ pub(crate) struct PathCompletionCtx {
     pub(super) qualified: Qualified,
     /// The parent of the path we are completing.
     pub(super) parent: Option<ast::Path>,
+    /// The path of which we are completing the segment
+    pub(super) path: ast::Path,
     pub(super) kind: PathKind,
     /// Whether the path segment has type args or not.
     pub(super) has_type_args: bool,
diff --git a/crates/ide-completion/src/context/analysis.rs b/crates/ide-completion/src/context/analysis.rs
index 7e6c842b1e0..e13950d56a9 100644
--- a/crates/ide-completion/src/context/analysis.rs
+++ b/crates/ide-completion/src/context/analysis.rs
@@ -556,6 +556,7 @@ impl<'a> CompletionContext<'a> {
             has_macro_bang: false,
             qualified: Qualified::No,
             parent: path.parent_path(),
+            path: path.clone(),
             kind: PathKind::Item { kind: ItemListKind::SourceFile },
             has_type_args: false,
             use_tree_parent: false,
diff --git a/crates/ide-completion/src/item.rs b/crates/ide-completion/src/item.rs
index 4774fe9db73..2b10dccb800 100644
--- a/crates/ide-completion/src/item.rs
+++ b/crates/ide-completion/src/item.rs
@@ -6,7 +6,7 @@ use hir::{Documentation, Mutability};
 use ide_db::{imports::import_assets::LocatedImport, SnippetCap, SymbolKind};
 use smallvec::SmallVec;
 use stdx::{impl_from, never};
-use syntax::{SmolStr, TextRange};
+use syntax::{SmolStr, TextRange, TextSize};
 use text_edit::TextEdit;
 
 use crate::{
@@ -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>,
+    ref_match: Option<(Mutability, TextSize)>,
 
     /// The import data to add to completion's edits.
     import_to_add: SmallVec<[LocatedImport; 1]>,
@@ -104,8 +104,8 @@ impl fmt::Debug for CompletionItem {
             s.field("relevance", &self.relevance);
         }
 
-        if let Some(mutability) = &self.ref_match {
-            s.field("ref_match", &format!("&{}", mutability.as_keyword_for_ref()));
+        if let Some((mutability, offset)) = &self.ref_match {
+            s.field("ref_match", &format!("&{}@{offset:?}", mutability.as_keyword_for_ref()));
         }
         if self.trigger_call_info {
             s.field("trigger_call_info", &true);
@@ -395,14 +395,14 @@ impl CompletionItem {
         self.trigger_call_info
     }
 
-    pub fn ref_match(&self) -> Option<(Mutability, CompletionRelevance)> {
+    pub fn ref_match(&self) -> Option<(Mutability, TextSize, CompletionRelevance)> {
         // Relevance of the ref match should be the same as the original
         // match, but with exact type match set because self.ref_match
         // is only set if there is an exact type match.
         let mut relevance = self.relevance;
         relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact);
 
-        self.ref_match.map(|mutability| (mutability, relevance))
+        self.ref_match.map(|(mutability, offset)| (mutability, offset, relevance))
     }
 
     pub fn imports_to_add(&self) -> &[LocatedImport] {
@@ -428,7 +428,7 @@ pub(crate) struct Builder {
     deprecated: bool,
     trigger_call_info: bool,
     relevance: CompletionRelevance,
-    ref_match: Option<Mutability>,
+    ref_match: Option<(Mutability, TextSize)>,
 }
 
 impl Builder {
@@ -548,8 +548,8 @@ impl Builder {
         self.imports_to_add.push(import_to_add);
         self
     }
-    pub(crate) fn ref_match(&mut self, mutability: Mutability) -> &mut Builder {
-        self.ref_match = Some(mutability);
+    pub(crate) fn ref_match(&mut self, mutability: Mutability, offset: TextSize) -> &mut Builder {
+        self.ref_match = Some((mutability, offset));
         self
     }
 }
diff --git a/crates/ide-completion/src/render.rs b/crates/ide-completion/src/render.rs
index 9c339a13e7b..643f34b22e5 100644
--- a/crates/ide-completion/src/render.rs
+++ b/crates/ide-completion/src/render.rs
@@ -14,7 +14,7 @@ use hir::{AsAssocItem, HasAttrs, HirDisplay, ScopeDef};
 use ide_db::{
     helpers::item_name, imports::import_assets::LocatedImport, RootDatabase, SnippetCap, SymbolKind,
 };
-use syntax::{SmolStr, SyntaxKind, TextRange};
+use syntax::{AstNode, SmolStr, SyntaxKind, TextRange};
 
 use crate::{
     context::{PathCompletionCtx, PathKind},
@@ -167,7 +167,7 @@ pub(crate) fn render_resolution_simple(
     local_name: hir::Name,
     resolution: ScopeDef,
 ) -> Builder {
-    render_resolution_simple_(ctx, local_name, None, resolution)
+    render_resolution_simple_(ctx, None, local_name, None, resolution)
 }
 
 pub(crate) fn render_resolution_with_import(
@@ -235,7 +235,8 @@ fn render_resolution_simple_type(
     let db = ctx.completion.db;
     let config = ctx.completion.config;
     let name = local_name.to_smol_str();
-    let mut item = render_resolution_simple_(ctx, local_name, import_to_add, resolution);
+    let mut item =
+        render_resolution_simple_(ctx, Some(path_ctx), local_name, import_to_add, resolution);
     // Add `<>` for generic types
     let type_path_no_ty_args = matches!(
         path_ctx,
@@ -264,6 +265,7 @@ fn render_resolution_simple_type(
 
 fn render_resolution_simple_(
     ctx: RenderContext<'_>,
+    path_ctx: Option<&PathCompletionCtx>,
     local_name: hir::Name,
     import_to_add: Option<LocatedImport>,
     resolution: ScopeDef,
@@ -317,8 +319,10 @@ fn render_resolution_simple_(
             ..CompletionRelevance::default()
         });
 
-        if let Some(ref_match) = compute_ref_match(ctx.completion, &ty) {
-            item.ref_match(ref_match);
+        if let Some(path_ctx) = path_ctx {
+            if let Some(ref_match) = compute_ref_match(ctx.completion, &ty) {
+                item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
+            }
         }
     };
 
@@ -455,7 +459,7 @@ mod tests {
                 let relevance = display_relevance(it.relevance());
                 items.push(format!("{} {} {}\n", tag, it.label(), relevance));
 
-                if let Some((mutability, relevance)) = it.ref_match() {
+                if let Some((mutability, _offset, relevance)) = it.ref_match() {
                     let label = format!("&{}{}", mutability.as_keyword_for_ref(), it.label());
                     let relevance = display_relevance(relevance);
 
@@ -1495,7 +1499,7 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
 "#,
             &[CompletionItemKind::Method, CompletionItemKind::SymbolKind(SymbolKind::Field)],
             // FIXME
-            // Ideally we'd also suggest &f.bar and &f.baz() as exact
+            // Ideally we'd also suggest &f.bar as exact
             // type matches. See #8058.
             expect![[r#"
                 [
@@ -1507,6 +1511,7 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
                         kind: Method,
                         lookup: "baz",
                         detail: "fn(&self) -> u32",
+                        ref_match: "&@96",
                     },
                     CompletionItem {
                         label: "bar",
@@ -1525,7 +1530,6 @@ fn foo(f: Foo) { let _: &u32 = f.b$0 }
 
     #[test]
     fn qualified_path_ref() {
-        // disabled right now because it doesn't render correctly, #8058
         check_kinds(
             r#"
 struct S;
@@ -1554,6 +1558,7 @@ fn main() {
                         ),
                         lookup: "foo",
                         detail: "fn() -> S",
+                        ref_match: "&@92",
                     },
                 ]
             "#]],
diff --git a/crates/ide-completion/src/render/function.rs b/crates/ide-completion/src/render/function.rs
index 3666ee40e2f..37486e4d93e 100644
--- a/crates/ide-completion/src/render/function.rs
+++ b/crates/ide-completion/src/render/function.rs
@@ -4,20 +4,19 @@ use hir::{db::HirDatabase, AsAssocItem, HirDisplay};
 use ide_db::{SnippetCap, SymbolKind};
 use itertools::Itertools;
 use stdx::{format_to, to_lower_snake_case};
-use syntax::SmolStr;
+use syntax::{AstNode, SmolStr};
 
 use crate::{
-    context::{
-        CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind, Qualified,
-    },
+    context::{CompletionContext, DotAccess, DotAccessKind, PathCompletionCtx, PathKind},
     item::{Builder, CompletionItem, CompletionItemKind, CompletionRelevance},
     render::{compute_exact_name_match, compute_ref_match, compute_type_match, RenderContext},
     CallableSnippets,
 };
 
-enum FuncKind {
-    Function,
-    Method(Option<hir::Name>),
+#[derive(Debug)]
+enum FuncKind<'ctx> {
+    Function(&'ctx PathCompletionCtx),
+    Method(&'ctx DotAccess, Option<hir::Name>),
 }
 
 pub(crate) fn render_fn(
@@ -27,29 +26,7 @@ pub(crate) fn render_fn(
     func: hir::Function,
 ) -> Builder {
     let _p = profile::span("render_fn");
-    let func_kind = FuncKind::Function;
-    let params = match ctx.completion.config.snippet_cap {
-        Some(_) => {
-            if !matches!(
-                path_ctx,
-                PathCompletionCtx { kind: PathKind::Expr { .. }, has_call_parens: true, .. }
-                    | PathCompletionCtx { kind: PathKind::Use | PathKind::Type { .. }, .. }
-            ) {
-                params(ctx.completion, func, &func_kind, false)
-            } else {
-                None
-            }
-        }
-        _ => None,
-    };
-    render(
-        ctx,
-        local_name,
-        func,
-        func_kind,
-        params,
-        matches!(path_ctx.qualified, Qualified::With { .. }),
-    )
+    render(ctx, local_name, func, FuncKind::Function(path_ctx))
 }
 
 pub(crate) fn render_method(
@@ -60,16 +37,7 @@ pub(crate) fn render_method(
     func: hir::Function,
 ) -> Builder {
     let _p = profile::span("render_method");
-    let func_kind = FuncKind::Method(receiver);
-    let params = match ctx.completion.config.snippet_cap {
-        Some(_) => match dot_access {
-            DotAccess { kind: DotAccessKind::Method { has_parens: true }, .. } => None,
-            _ => params(ctx.completion, func, &func_kind, true),
-        },
-        _ => None,
-    };
-
-    render(ctx, local_name, func, func_kind, params, false)
+    render(ctx, local_name, func, FuncKind::Method(dot_access, receiver))
 }
 
 fn render(
@@ -77,15 +45,13 @@ fn render(
     local_name: Option<hir::Name>,
     func: hir::Function,
     func_kind: FuncKind,
-    params: Option<(Option<hir::SelfParam>, Vec<hir::Param>)>,
-    qualified_path: bool,
 ) -> Builder {
     let db = completion.db;
 
     let name = local_name.unwrap_or_else(|| func.name(db));
 
     let call = match &func_kind {
-        FuncKind::Method(Some(receiver)) => format!("{}.{}", receiver, &name).into(),
+        FuncKind::Method(_, Some(receiver)) => format!("{}.{}", receiver, &name).into(),
         _ => name.to_smol_str(),
     };
     let mut item = CompletionItem::new(
@@ -111,11 +77,14 @@ fn render(
     });
 
     if let Some(ref_match) = compute_ref_match(completion, &ret_type) {
-        // FIXME For now we don't properly calculate the edits for ref match
-        // completions on methods or qualified paths, so we've disabled them.
-        // See #8058.
-        if matches!(func_kind, FuncKind::Function) && !qualified_path {
-            item.ref_match(ref_match);
+        match func_kind {
+            FuncKind::Function(path_ctx) => {
+                item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
+            }
+            FuncKind::Method(DotAccess { receiver: Some(receiver), .. }, _) => {
+                item.ref_match(ref_match, receiver.syntax().text_range().start());
+            }
+            _ => (),
         }
     }
 
@@ -124,12 +93,34 @@ fn render(
         .detail(detail(db, func))
         .lookup_by(name.to_smol_str());
 
-    match completion.config.snippet_cap.zip(params) {
-        Some((cap, (self_param, params))) => {
-            add_call_parens(&mut item, completion, cap, call, self_param, params);
+    match ctx.completion.config.snippet_cap {
+        Some(cap) => {
+            let complete_params = match func_kind {
+                FuncKind::Function(PathCompletionCtx {
+                    kind: PathKind::Expr { .. },
+                    has_call_parens: false,
+                    ..
+                }) => Some(false),
+                FuncKind::Method(
+                    DotAccess {
+                        kind:
+                            DotAccessKind::Method { has_parens: false } | DotAccessKind::Field { .. },
+                        ..
+                    },
+                    _,
+                ) => Some(true),
+                _ => None,
+            };
+            if let Some(has_dot_receiver) = complete_params {
+                if let Some((self_param, params)) =
+                    params(ctx.completion, func, &func_kind, has_dot_receiver)
+                {
+                    add_call_parens(&mut item, completion, cap, call, self_param, params);
+                }
+            }
         }
         _ => (),
-    }
+    };
 
     match ctx.import_to_add {
         Some(import_to_add) => {
@@ -291,7 +282,7 @@ fn params(
         }
     }
 
-    let self_param = if has_dot_receiver || matches!(func_kind, FuncKind::Method(Some(_))) {
+    let self_param = if has_dot_receiver || matches!(func_kind, FuncKind::Method(_, Some(_))) {
         None
     } else {
         func.self_param(ctx.db)
diff --git a/crates/ide-completion/src/render/literal.rs b/crates/ide-completion/src/render/literal.rs
index b89030990e4..7b0555d5a4c 100644
--- a/crates/ide-completion/src/render/literal.rs
+++ b/crates/ide-completion/src/render/literal.rs
@@ -2,6 +2,7 @@
 
 use hir::{db::HirDatabase, Documentation, HasAttrs, StructKind};
 use ide_db::SymbolKind;
+use syntax::AstNode;
 
 use crate::{
     context::{CompletionContext, PathCompletionCtx, PathKind},
@@ -117,7 +118,7 @@ fn render(
         ..ctx.completion_relevance()
     });
     if let Some(ref_match) = compute_ref_match(completion, &ty) {
-        item.ref_match(ref_match);
+        item.ref_match(ref_match, path_ctx.path.syntax().text_range().start());
     }
 
     if let Some(import_to_add) = ctx.import_to_add {
diff --git a/crates/ide-completion/src/tests/pattern.rs b/crates/ide-completion/src/tests/pattern.rs
index da9e5e20288..8953f1b2fdf 100644
--- a/crates/ide-completion/src/tests/pattern.rs
+++ b/crates/ide-completion/src/tests/pattern.rs
@@ -399,6 +399,7 @@ fn foo() {
 
 #[test]
 fn completes_no_delims_if_existing() {
+    // FIXME: We should not complete functions here
     check_empty(
         r#"
 struct Bar(u32);
@@ -409,7 +410,7 @@ fn foo() {
 }
 "#,
         expect![[r#"
-            fn foo()   fn()
+            fn foo     fn()
             st Bar
             bt u32
             kw crate::
@@ -427,7 +428,7 @@ fn foo() {
 }
 "#,
         expect![[r#"
-            fn foo()   fn()
+            fn foo     fn()
             st Foo
             bt u32
             kw crate::
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 3d702fe8dc2..cdd152ccf56 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -224,6 +224,7 @@ fn completion_item(
     max_relevance: u32,
     item: CompletionItem,
 ) {
+    let insert_replace_support = config.insert_replace_support().then(|| tdpp.position);
     let mut additional_text_edits = Vec::new();
 
     // LSP does not allow arbitrary edits in completion, so we have to do a
@@ -233,7 +234,6 @@ fn completion_item(
         let source_range = item.source_range();
         for indel in item.text_edit().iter() {
             if indel.delete.contains_range(source_range) {
-                let insert_replace_support = config.insert_replace_support().then(|| tdpp.position);
                 text_edit = Some(if indel.delete == source_range {
                     self::completion_text_edit(line_index, insert_replace_support, indel.clone())
                 } else {
@@ -254,6 +254,14 @@ fn completion_item(
         text_edit.unwrap()
     };
 
+    let insert_text_format = item.is_snippet().then(|| lsp_types::InsertTextFormat::SNIPPET);
+    let tags = item.deprecated().then(|| vec![lsp_types::CompletionItemTag::DEPRECATED]);
+    let command = if item.trigger_call_info() && config.client_commands().trigger_parameter_hints {
+        Some(command::trigger_parameter_hints())
+    } else {
+        None
+    };
+
     let mut lsp_item = lsp_types::CompletionItem {
         label: item.label().to_string(),
         detail: item.detail().map(|it| it.to_string()),
@@ -263,22 +271,14 @@ fn completion_item(
         additional_text_edits: Some(additional_text_edits),
         documentation: item.documentation().map(documentation),
         deprecated: Some(item.deprecated()),
+        tags,
+        command,
+        insert_text_format,
         ..Default::default()
     };
 
     set_score(&mut lsp_item, max_relevance, item.relevance());
 
-    if item.deprecated() {
-        lsp_item.tags = Some(vec![lsp_types::CompletionItemTag::DEPRECATED])
-    }
-
-    if item.trigger_call_info() && config.client_commands().trigger_parameter_hints {
-        lsp_item.command = Some(command::trigger_parameter_hints());
-    }
-
-    if item.is_snippet() {
-        lsp_item.insert_text_format = Some(lsp_types::InsertTextFormat::SNIPPET);
-    }
     if config.completion().enable_imports_on_the_fly {
         if let imports @ [_, ..] = item.imports_to_add() {
             let imports: Vec<_> = imports
@@ -299,18 +299,17 @@ fn completion_item(
         }
     }
 
-    if let Some((mutability, relevance)) = item.ref_match() {
+    if let Some((mutability, offset, relevance)) = item.ref_match() {
         let mut lsp_item_with_ref = lsp_item.clone();
         set_score(&mut lsp_item_with_ref, max_relevance, relevance);
         lsp_item_with_ref.label =
             format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label);
-        if let Some(it) = &mut lsp_item_with_ref.text_edit {
-            let new_text = match it {
-                lsp_types::CompletionTextEdit::Edit(it) => &mut it.new_text,
-                lsp_types::CompletionTextEdit::InsertAndReplace(it) => &mut it.new_text,
-            };
-            *new_text = format!("&{}{}", mutability.as_keyword_for_ref(), new_text);
-        }
+        lsp_item_with_ref.additional_text_edits.get_or_insert_with(Default::default).push(
+            self::text_edit(
+                line_index,
+                Indel::insert(offset, format!("&{}", mutability.as_keyword_for_ref())),
+            ),
+        );
 
         acc.push(lsp_item_with_ref);
     };