Merge #8410

8410: Use CompletionTextEdit::InsertAndReplace if supported by the client r=Veykril a=Veykril

Fixes #8404, Fixes #3130

Co-authored-by: Lukas Wirth <[email protected]>

5 files changed, 50 insertions, 20 deletions

diff --git a/crates/ide_completion/src/item.rs b/crates/ide_completion/src/item.rs
index cc4ac9ea2..16991b688 100644
--- a/crates/ide_completion/src/item.rs
+++ b/crates/ide_completion/src/item.rs
@@ -29,7 +29,7 @@ pub struct CompletionItem {
     /// Range of identifier that is being completed.
     ///
     /// It should be used primarily for UI, but we also use this to convert
-    /// genetic TextEdit into LSP's completion edit (see conv.rs).
+    /// generic TextEdit into LSP's completion edit (see conv.rs).
     ///
     /// `source_range` must contain the completion offset. `insert_text` should
     /// start with what `source_range` points to, or VSCode will filter out the
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index e012b4452..f809667e9 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -656,6 +656,19 @@ impl Config {
     pub fn code_lens_refresh(&self) -> bool {
         try_or!(self.caps.workspace.as_ref()?.code_lens.as_ref()?.refresh_support?, false)
     }
+    pub fn insert_replace_support(&self) -> bool {
+        try_or!(
+            self.caps
+                .text_document
+                .as_ref()?
+                .completion
+                .as_ref()?
+                .completion_item
+                .as_ref()?
+                .insert_replace_support?,
+            false
+        )
+    }
 }
 
 #[derive(Deserialize, Debug, Clone)]
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 31d8c487b..edfa42eb5 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -664,10 +664,13 @@ pub(crate) fn handle_completion(
     };
     let line_index = snap.file_line_index(position.file_id)?;
 
+    let insert_replace_support =
+        snap.config.insert_replace_support().then(|| text_document_position.position);
     let items: Vec<CompletionItem> = items
         .into_iter()
         .flat_map(|item| {
-            let mut new_completion_items = to_proto::completion_item(&line_index, item.clone());
+            let mut new_completion_items =
+                to_proto::completion_item(insert_replace_support, &line_index, item.clone());
 
             if completion_config.enable_imports_on_the_fly {
                 for new_item in &mut new_completion_items {
diff --git a/crates/rust-analyzer/src/lsp_utils.rs b/crates/rust-analyzer/src/lsp_utils.rs
index 2ac487632..73c4193e8 100644
--- a/crates/rust-analyzer/src/lsp_utils.rs
+++ b/crates/rust-analyzer/src/lsp_utils.rs
@@ -150,8 +150,16 @@ pub(crate) fn all_edits_are_disjoint(
             edit_ranges.push(edit.range);
         }
         Some(lsp_types::CompletionTextEdit::InsertAndReplace(edit)) => {
-            edit_ranges.push(edit.insert);
-            edit_ranges.push(edit.replace);
+            let replace = edit.replace;
+            let insert = edit.insert;
+            if replace.start != insert.start
+                || insert.start > insert.end
+                || insert.end > replace.end
+            {
+                // insert has to be a prefix of replace but it is not
+                return false;
+            }
+            edit_ranges.push(replace);
         }
         None => {}
     }
@@ -314,18 +322,6 @@ mod tests {
             Some(CompletionTextEdit::InsertAndReplace(InsertReplaceEdit {
                 new_text: "new_text".to_string(),
                 insert: disjoint_edit.range,
-                replace: joint_edit.range,
-            }));
-        completion_with_joint_edits.additional_text_edits = None;
-        assert!(
-            !all_edits_are_disjoint(&completion_with_joint_edits, &[]),
-            "Completion with disjoint edits fails the validation even with empty extra edits"
-        );
-
-        completion_with_joint_edits.text_edit =
-            Some(CompletionTextEdit::InsertAndReplace(InsertReplaceEdit {
-                new_text: "new_text".to_string(),
-                insert: disjoint_edit.range,
                 replace: disjoint_edit_2.range,
             }));
         completion_with_joint_edits.additional_text_edits = Some(vec![joint_edit]);
diff --git a/crates/rust-analyzer/src/to_proto.rs b/crates/rust-analyzer/src/to_proto.rs
index 2ac31d981..9fac562ff 100644
--- a/crates/rust-analyzer/src/to_proto.rs
+++ b/crates/rust-analyzer/src/to_proto.rs
@@ -145,6 +145,23 @@ pub(crate) fn text_edit(line_index: &LineIndex, indel: Indel) -> lsp_types::Text
     lsp_types::TextEdit { range, new_text }
 }
 
+pub(crate) fn completion_text_edit(
+    line_index: &LineIndex,
+    insert_replace_support: Option<lsp_types::Position>,
+    indel: Indel,
+) -> lsp_types::CompletionTextEdit {
+    let text_edit = text_edit(line_index, indel);
+    match insert_replace_support {
+        Some(cursor_pos) => lsp_types::InsertReplaceEdit {
+            new_text: text_edit.new_text,
+            insert: lsp_types::Range { start: text_edit.range.start, end: cursor_pos },
+            replace: text_edit.range,
+        }
+        .into(),
+        None => text_edit.into(),
+    }
+}
+
 pub(crate) fn snippet_text_edit(
     line_index: &LineIndex,
     is_snippet: bool,
@@ -179,6 +196,7 @@ pub(crate) fn snippet_text_edit_vec(
 }
 
 pub(crate) fn completion_item(
+    insert_replace_support: Option<lsp_types::Position>,
     line_index: &LineIndex,
     item: CompletionItem,
 ) -> Vec<lsp_types::CompletionItem> {
@@ -190,7 +208,7 @@ pub(crate) fn completion_item(
     for indel in item.text_edit().iter() {
         if indel.delete.contains_range(source_range) {
             text_edit = Some(if indel.delete == source_range {
-                self::text_edit(line_index, indel.clone())
+                self::completion_text_edit(line_index, insert_replace_support, indel.clone())
             } else {
                 assert!(source_range.end() == indel.delete.end());
                 let range1 = TextRange::new(indel.delete.start(), source_range.start());
@@ -198,7 +216,7 @@ pub(crate) fn completion_item(
                 let indel1 = Indel::replace(range1, String::new());
                 let indel2 = Indel::replace(range2, indel.insert.clone());
                 additional_text_edits.push(self::text_edit(line_index, indel1));
-                self::text_edit(line_index, indel2)
+                self::completion_text_edit(line_index, insert_replace_support, indel2)
             })
         } else {
             assert!(source_range.intersect(indel.delete).is_none());
@@ -213,7 +231,7 @@ pub(crate) fn completion_item(
         detail: item.detail().map(|it| it.to_string()),
         filter_text: Some(item.lookup().to_string()),
         kind: item.kind().map(completion_item_kind),
-        text_edit: Some(text_edit.into()),
+        text_edit: Some(text_edit),
         additional_text_edits: Some(additional_text_edits),
         documentation: item.documentation().map(documentation),
         deprecated: Some(item.deprecated()),
@@ -1136,7 +1154,7 @@ mod tests {
             .unwrap()
             .into_iter()
             .filter(|c| c.label().ends_with("arg"))
-            .map(|c| completion_item(&line_index, c))
+            .map(|c| completion_item(None, &line_index, c))
             .flat_map(|comps| comps.into_iter().map(|c| (c.label, c.sort_text)))
             .collect();
         expect_test::expect![[r#"