Do not repeat proposed LSP completions in the word completions (#26682)

Kirill Bulatov created 9 months ago

Follow-up of https://github.com/zed-industries/zed/pull/26410

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs       | 37 +++++++++--------
crates/editor/src/editor_tests.rs | 67 ++++++++++++++++++++++++++++++++
2 files changed, 85 insertions(+), 19 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -4104,23 +4104,26 @@ impl Editor {
 
                 match completion_settings.words {
                     WordsCompletionMode::Enabled => {
-                        completions.extend(
-                            words
-                                .await
-                                .into_iter()
-                                .filter(|(word, _)| word_to_exclude.as_ref() != Some(word))
-                                .map(|(word, word_range)| Completion {
-                                    old_range: old_range.clone(),
-                                    new_text: word.clone(),
-                                    label: CodeLabel::plain(word, None),
-                                    documentation: None,
-                                    source: CompletionSource::BufferWord {
-                                        word_range,
-                                        resolved: false,
-                                    },
-                                    confirm: None,
-                                }),
-                        );
+                        let mut words = words.await;
+                        if let Some(word_to_exclude) = &word_to_exclude {
+                            words.remove(word_to_exclude);
+                        }
+                        for lsp_completion in &completions {
+                            words.remove(&lsp_completion.new_text);
+                        }
+                        completions.extend(words.into_iter().map(|(word, word_range)| {
+                            Completion {
+                                old_range: old_range.clone(),
+                                new_text: word.clone(),
+                                label: CodeLabel::plain(word, None),
+                                documentation: None,
+                                source: CompletionSource::BufferWord {
+                                    word_range,
+                                    resolved: false,
+                                },
+                                confirm: None,
+                            }
+                        }));
                     }
                     WordsCompletionMode::Fallback => {
                         if completions.is_empty() {

crates/editor/src/editor_tests.rs

@@ -9236,11 +9236,11 @@ async fn test_words_completion(cx: &mut TestAppContext) {
                     Ok(Some(lsp::CompletionResponse::Array(vec![
                         lsp::CompletionItem {
                             label: "first".into(),
-                            ..Default::default()
+                            ..lsp::CompletionItem::default()
                         },
                         lsp::CompletionItem {
                             label: "last".into(),
-                            ..Default::default()
+                            ..lsp::CompletionItem::default()
                         },
                     ])))
                 }
@@ -9290,6 +9290,69 @@ async fn test_words_completion(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_word_completions_do_not_duplicate_lsp_ones(cx: &mut TestAppContext) {
+    init_test(cx, |language_settings| {
+        language_settings.defaults.completions = Some(CompletionSettings {
+            words: WordsCompletionMode::Enabled,
+            lsp: true,
+            lsp_fetch_timeout_ms: 0,
+        });
+    });
+
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            completion_provider: Some(lsp::CompletionOptions {
+                trigger_characters: Some(vec![".".to_string(), ":".to_string()]),
+                ..lsp::CompletionOptions::default()
+            }),
+            signature_help_provider: Some(lsp::SignatureHelpOptions::default()),
+            ..lsp::ServerCapabilities::default()
+        },
+        cx,
+    )
+    .await;
+
+    let _completion_requests_handler =
+        cx.lsp
+            .server
+            .on_request::<lsp::request::Completion, _, _>(move |_, _| async move {
+                Ok(Some(lsp::CompletionResponse::Array(vec![
+                    lsp::CompletionItem {
+                        label: "first".into(),
+                        ..lsp::CompletionItem::default()
+                    },
+                    lsp::CompletionItem {
+                        label: "last".into(),
+                        ..lsp::CompletionItem::default()
+                    },
+                ])))
+            });
+
+    cx.set_state(indoc! {"ˇ
+        first
+        last
+        second
+    "});
+    cx.simulate_keystroke(".");
+    cx.executor().run_until_parked();
+    cx.condition(|editor, _| editor.context_menu_visible())
+        .await;
+    cx.update_editor(|editor, window, cx| {
+        if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref()
+        {
+            assert_eq!(
+                completion_menu_entries(&menu),
+                &["first", "last", "second"],
+                "Word completions that has the same edit as the any of the LSP ones, should not be proposed"
+            );
+        } else {
+            panic!("expected completion menu to be open");
+        }
+        editor.cancel(&Cancel, window, cx);
+    });
+}
+
 #[gpui::test]
 async fn test_multiline_completion(cx: &mut TestAppContext) {
     init_test(cx, |_| {});