lsp: Use correct LSP adapter for completion labels in remote development (#50697)

Xin Zhao created 1 month ago

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.

Change summary

crates/project/src/lsp_store.rs | 25 +++++++++++--------------
1 file changed, 11 insertions(+), 14 deletions(-)

Detailed changes

crates/project/src/lsp_store.rs

@@ -4904,7 +4904,7 @@ impl LspStore {
         buffer: &Entity<Buffer>,
         mut check: F,
         cx: &App,
-    ) -> Vec<lsp::LanguageServerId>
+    ) -> 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| {