editor: Fix TypeScript auto-import breaking generic function calls (#30312)

Smit Barmase created 7 months ago

Closes #29982

When auto-importing TypeScript functions with generic type arguments
(like `useRef<HTMLDivElement>(null)`), the language server returns
snippets with placeholders (e.g., `useRef(${1:initialValue})$0`). While
useful for new function calls, this behavior breaks existing code when
renaming functions that already have parameters.

For example, completing `useR^<HTMLDivElement>(null)` incorrectly
results in `useRef(initialValue)^<HTMLDivElement>(null)`.

Related upstream issue:
https://github.com/microsoft/TypeScript/issues/51758
Similar workaround fix:
https://github.com/pmizio/typescript-tools.nvim/pull/147

Release Notes:

- Fixed TypeScript auto-import behavior where functions with generic
type arguments (like `useRef<HTMLDivElement>(null)`) would incorrectly
insert snippet placeholders, breaking the syntax.

Change summary

crates/editor/src/editor.rs   | 32 ++++++++++++++++++++++++++------
crates/project/src/project.rs | 18 ++++++++++++++----
2 files changed, 40 insertions(+), 10 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -4997,10 +4997,34 @@ impl Editor {
             .clone();
         cx.stop_propagation();
 
+        let snapshot = self.buffer.read(cx).snapshot(cx);
+        let newest_anchor = self.selections.newest_anchor();
+
         let snippet;
         let new_text;
         if completion.is_snippet() {
-            snippet = Some(Snippet::parse(&completion.new_text).log_err()?);
+            // lsp returns function definition with placeholders in "new_text"
+            // when configured from language server, even when renaming a function
+            //
+            // in such cases, we use the label instead
+            // https://github.com/zed-industries/zed/issues/29982
+            let snippet_source = completion
+                .label()
+                .filter(|label| {
+                    completion.kind() == Some(CompletionItemKind::FUNCTION)
+                        && label != &completion.new_text
+                })
+                .and_then(|label| {
+                    let cursor_offset = newest_anchor.head().to_offset(&snapshot);
+                    let next_char_is_not_whitespace = snapshot
+                        .chars_at(cursor_offset)
+                        .next()
+                        .map_or(true, |ch| !ch.is_whitespace());
+                    next_char_is_not_whitespace.then_some(label)
+                })
+                .unwrap_or(completion.new_text.clone());
+
+            snippet = Some(Snippet::parse(&snippet_source).log_err()?);
             new_text = snippet.as_ref().unwrap().text.clone();
         } else {
             snippet = None;
@@ -5009,11 +5033,8 @@ impl Editor {
 
         let replace_range = choose_completion_range(&completion, intent, &buffer_handle, cx);
         let buffer = buffer_handle.read(cx);
-        let snapshot = self.buffer.read(cx).snapshot(cx);
         let replace_range_multibuffer = {
-            let excerpt = snapshot
-                .excerpt_containing(self.selections.newest_anchor().range())
-                .unwrap();
+            let excerpt = snapshot.excerpt_containing(newest_anchor.range()).unwrap();
             let multibuffer_anchor = snapshot
                 .anchor_in_excerpt(excerpt.id(), buffer.anchor_before(replace_range.start))
                 .unwrap()
@@ -5023,7 +5044,6 @@ impl Editor {
             multibuffer_anchor.start.to_offset(&snapshot)
                 ..multibuffer_anchor.end.to_offset(&snapshot)
         };
-        let newest_anchor = self.selections.newest_anchor();
         if newest_anchor.head().buffer_id != Some(buffer.remote_id()) {
             return None;
         }

crates/project/src/project.rs

@@ -5187,15 +5187,25 @@ impl ProjectItem for Buffer {
 }
 
 impl Completion {
+    pub fn kind(&self) -> Option<CompletionItemKind> {
+        self.source
+            // `lsp::CompletionListItemDefaults` has no `kind` field
+            .lsp_completion(false)
+            .and_then(|lsp_completion| lsp_completion.kind)
+    }
+
+    pub fn label(&self) -> Option<String> {
+        self.source
+            .lsp_completion(false)
+            .map(|lsp_completion| lsp_completion.label.clone())
+    }
+
     /// A key that can be used to sort completions when displaying
     /// them to the user.
     pub fn sort_key(&self) -> (usize, &str) {
         const DEFAULT_KIND_KEY: usize = 3;
         let kind_key = self
-            .source
-            // `lsp::CompletionListItemDefaults` has no `kind` field
-            .lsp_completion(false)
-            .and_then(|lsp_completion| lsp_completion.kind)
+            .kind()
             .and_then(|lsp_completion_kind| match lsp_completion_kind {
                 lsp::CompletionItemKind::KEYWORD => Some(0),
                 lsp::CompletionItemKind::VARIABLE => Some(1),