From 09b4140b80491eebcd5967297f885eb26f7fe019 Mon Sep 17 00:00:00 2001 From: Xin Zhao Date: Fri, 6 Mar 2026 22:11:16 +0800 Subject: [PATCH] lsp: Use correct LSP adapter for completion labels in remote development (#50697) Closes #47917 Currently, during remote development, Zed uses the first available local LSP adapter to handle all label computing. This isn't ideal and causes bugs because different adapters may expect different label formats. By leveraging the `all_capable_for_proto_request` method, we can now retrieve the specific language server name and use it as a key to find the correct LSP adapter for label population. Even if this lookup fails, we can still fall back to the first adapter, so this PR should provide more accurate label population than before. This addresses the root cause of #47917, which stems from two main issues. For example, in remote Python development, the `basedpyright` adapter might incorrectly handle labels even when the remote server is actually `ty`. The completion items returned by `ty` are slightly different from `basedpyright`: `ty` stores labels in `labelDetails.detail`, while basedpyright uses `labelDetails.description`. By matching the correct adapter, we ensure labels are populated properly for completion items. ```json // RPC message returned by `ty`, label is in `labelDetails.detail` { ... "labelDetails": { "detail": " (import pathlib)" }, ... } // RPC message returned by `basedpyright`, label is in `labelDetails.description` { ... "labelDetails": { "description": "pathlib" }, ... } ``` Additionally, adapters registered via `register_available_lsp_adapter` are lazy-loaded into the `LanguageRegistry` (which is the case for `ty` before). In remote scenarios, the adapter might be loaded on the remote server but not on the local host, making it hard to find in `lsp_adapters`. This is partially resolved in #50662, and combined with this PR, we can fully address #47917. There is still more to do, however. In some cases, we still can't find the correct local LSP adapter if the local host lacks the registry that the remote server has; this typically happens when the adapter is registered via `register_available_lsp_adapter`. I've opened a feature discussion #49178 to track this. If it's decided that this needs further refinement, I'm happy to continue working on it. Before you mark this PR as ready for review, make sure that you have: - [ ] Added a solid test coverage and/or screenshots from doing manual testing - [x] Done a self-review taking into account security and performance aspects - [x] Aligned any UI changes with the [UI checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) Release Notes: - Fixed missing labels for `ty` completion items in remote development. --- crates/project/src/lsp_store.rs | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 7573af1dc69f33586199c6f9e5e4d2a59f6d2d6f..97aa03cec730c61acfb129579c77f6a5b560ee32 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -4904,7 +4904,7 @@ impl LspStore { buffer: &Entity, mut check: F, cx: &App, - ) -> Vec + ) -> Vec<(lsp::LanguageServerId, lsp::LanguageServerName)> where F: FnMut(&lsp::LanguageServerName, &lsp::ServerCapabilities) -> bool, { @@ -4934,7 +4934,7 @@ impl LspStore { .map(|c| (server_id, server_name, c)) }) .filter(|(_, server_name, capabilities)| check(server_name, capabilities)) - .map(|(server_id, _, _)| *server_id) + .map(|(server_id, server_name, _)| (*server_id, server_name.clone())) .collect() } @@ -6132,23 +6132,13 @@ impl LspStore { let language = buffer.read(cx).language().cloned(); - // In the future, we should provide project guests with the names of LSP adapters, - // so that they can use the correct LSP adapter when computing labels. For now, - // guests just use the first LSP adapter associated with the buffer's language. - let lsp_adapter = language.as_ref().and_then(|language| { - language_registry - .lsp_adapters(&language.name()) - .first() - .cloned() - }); - let buffer = buffer.clone(); cx.spawn(async move |this, cx| { let requests = join_all( capable_lsps .into_iter() - .map(|id| { + .map(|(id, server_name)| { let request = GetCompletions { position, context: context.clone(), @@ -6156,7 +6146,14 @@ impl LspStore { }; let buffer = buffer.clone(); let language = language.clone(); - let lsp_adapter = lsp_adapter.clone(); + let lsp_adapter = language.as_ref().and_then(|language| { + let adapters = language_registry.lsp_adapters(&language.name()); + adapters + .iter() + .find(|adapter| adapter.name() == server_name) + .or_else(|| adapters.first()) + .cloned() + }); let upstream_client = upstream_client.clone(); let response = this .update(cx, |this, cx| {