Regenerate completion labels on resolve (#21521)

Kirill Bulatov created

Closes https://github.com/zed-industries/zed/issues/21516

Technically, this is an LSP violation from `vtsls`, but seems that it's
not going to be fixed adequately on that side, see
https://github.com/yioneko/vtsls/issues/213 for more context.
So, we have to accommodate at least for now.

Release Notes:

- Fixed completion item labels not being updated after the resolve for
non-LSP compliant servers

Change summary

crates/editor/src/editor.rs       |  4 
crates/editor/src/editor_tests.rs | 91 +++++++++++++++++++++++++++++++++
crates/project/src/lsp_store.rs   | 35 +++++++++++-
3 files changed, 124 insertions(+), 6 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -1181,9 +1181,9 @@ impl CompletionsMenu {
         let delay = Duration::from_millis(delay_ms);
 
         completion_resolve.lock().fire_new(delay, cx, |_, cx| {
-            cx.spawn(move |this, mut cx| async move {
+            cx.spawn(move |editor, mut cx| async move {
                 if let Some(true) = resolve_task.await.log_err() {
-                    this.update(&mut cx, |_, cx| cx.notify()).ok();
+                    editor.update(&mut cx, |_, cx| cx.notify()).ok();
                 }
             })
         });

crates/editor/src/editor_tests.rs 🔗

@@ -10625,6 +10625,97 @@ async fn test_completions_with_additional_edits(cx: &mut gpui::TestAppContext) {
     cx.assert_editor_state(indoc! {"fn main() { let a = Some(2)ˇ; }"});
 }
 
+#[gpui::test]
+async fn test_completions_resolve_updates_labels(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorLspTestContext::new_rust(
+        lsp::ServerCapabilities {
+            completion_provider: Some(lsp::CompletionOptions {
+                trigger_characters: Some(vec![".".to_string()]),
+                resolve_provider: Some(true),
+                ..Default::default()
+            }),
+            ..Default::default()
+        },
+        cx,
+    )
+    .await;
+
+    cx.set_state(indoc! {"fn main() { let a = 2ˇ; }"});
+    cx.simulate_keystroke(".");
+
+    let completion_item = lsp::CompletionItem {
+        label: "unresolved".to_string(),
+        detail: None,
+        documentation: None,
+        text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+            range: lsp::Range::new(lsp::Position::new(0, 22), lsp::Position::new(0, 22)),
+            new_text: ".unresolved".to_string(),
+        })),
+        ..lsp::CompletionItem::default()
+    };
+
+    cx.handle_request::<lsp::request::Completion, _, _>(move |_, _, _| {
+        let item = completion_item.clone();
+        async move { Ok(Some(lsp::CompletionResponse::Array(vec![item]))) }
+    })
+    .next()
+    .await;
+
+    cx.condition(|editor, _| editor.context_menu_visible())
+        .await;
+    cx.update_editor(|editor, _| {
+        let context_menu = editor.context_menu.read();
+        let context_menu = context_menu
+            .as_ref()
+            .expect("Should have the context menu deployed");
+        match context_menu {
+            ContextMenu::Completions(completions_menu) => {
+                let completions = completions_menu.completions.read();
+                assert_eq!(completions.len(), 1, "Should have one completion");
+                assert_eq!(completions.get(0).unwrap().label.text, "unresolved");
+            }
+            ContextMenu::CodeActions(_) => panic!("Should show the completions menu"),
+        }
+    });
+
+    cx.handle_request::<lsp::request::ResolveCompletionItem, _, _>(move |_, _, _| async move {
+        Ok(lsp::CompletionItem {
+            label: "resolved".to_string(),
+            detail: Some("Now resolved!".to_string()),
+            documentation: Some(lsp::Documentation::String("Docs".to_string())),
+            text_edit: Some(lsp::CompletionTextEdit::Edit(lsp::TextEdit {
+                range: lsp::Range::new(lsp::Position::new(0, 22), lsp::Position::new(0, 22)),
+                new_text: ".resolved".to_string(),
+            })),
+            ..lsp::CompletionItem::default()
+        })
+    })
+    .next()
+    .await;
+    cx.run_until_parked();
+
+    cx.update_editor(|editor, _| {
+        let context_menu = editor.context_menu.read();
+        let context_menu = context_menu
+            .as_ref()
+            .expect("Should have the context menu deployed");
+        match context_menu {
+            ContextMenu::Completions(completions_menu) => {
+                let completions = completions_menu.completions.read();
+                assert_eq!(completions.len(), 1, "Should have one completion");
+                assert_eq!(
+                    completions.get(0).unwrap().label.text,
+                    "resolved",
+                    "Should update the completion label after resolving"
+                );
+            }
+            ContextMenu::CodeActions(_) => panic!("Should show the completions menu"),
+        }
+    });
+}
+
 #[gpui::test]
 async fn test_completions_default_resolve_data_handling(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});

crates/project/src/lsp_store.rs 🔗

@@ -2241,17 +2241,23 @@ impl LspStore {
                         (server_id, completion)
                     };
 
-                    let server = this
-                        .read_with(&cx, |this, _| this.language_server_for_id(server_id))
+                    let server_and_adapter = this
+                        .read_with(&cx, |lsp_store, _| {
+                            let server = lsp_store.language_server_for_id(server_id)?;
+                            let adapter =
+                                lsp_store.language_server_adapter_for_id(server.server_id())?;
+                            Some((server, adapter))
+                        })
                         .ok()
                         .flatten();
-                    let Some(server) = server else {
+                    let Some((server, adapter)) = server_and_adapter else {
                         continue;
                     };
 
                     did_resolve = true;
                     Self::resolve_completion_local(
                         server,
+                        adapter,
                         &buffer_snapshot,
                         completions.clone(),
                         completion_index,
@@ -2268,6 +2274,7 @@ impl LspStore {
 
     async fn resolve_completion_local(
         server: Arc<lsp::LanguageServer>,
+        adapter: Arc<CachedLspAdapter>,
         snapshot: &BufferSnapshot,
         completions: Arc<RwLock<Box<[Completion]>>>,
         completion_index: usize,
@@ -2293,7 +2300,7 @@ impl LspStore {
             let documentation = language::prepare_completion_documentation(
                 lsp_documentation,
                 &language_registry,
-                None, // TODO: Try to reasonably work out which language the completion is for
+                snapshot.language().cloned(),
             )
             .await;
 
@@ -2332,9 +2339,29 @@ impl LspStore {
             }
         }
 
+        // NB: Zed does not have `details` inside the completion resolve capabilities, but certain language servers violate the spec and do not return `details` immediately, e.g. https://github.com/yioneko/vtsls/issues/213
+        // So we have to update the label here anyway...
+        let new_label = match snapshot.language() {
+            Some(language) => adapter
+                .labels_for_completions(&[completion_item.clone()], language)
+                .await
+                .log_err()
+                .unwrap_or_default(),
+            None => Vec::new(),
+        }
+        .pop()
+        .flatten()
+        .unwrap_or_else(|| {
+            CodeLabel::plain(
+                completion_item.label.clone(),
+                completion_item.filter_text.as_deref(),
+            )
+        });
+
         let mut completions = completions.write();
         let completion = &mut completions[completion_index];
         completion.lsp_completion = completion_item;
+        completion.label = new_label;
     }
 
     #[allow(clippy::too_many_arguments)]