lsp_button: Do not surface language servers from different windows in current workspace (#42733)

Piotr Osiewicz , HactarCE , and Kirill Bulatov created

This led to a problem where we'd have a zombie entries in LSP dropdown
because they were treated as if they originated from an unknown
worktree.

Closes #42077

Release Notes:

- Fixed LSP status list containing zombie entries for LSPs in other
windows

---------

Co-authored-by: HactarCE <6060305+HactarCE@users.noreply.github.com>
Co-authored-by: Kirill Bulatov <kirill@zed.dev>

Change summary

crates/language_tools/src/lsp_button.rs | 83 ++++++++------------------
1 file changed, 26 insertions(+), 57 deletions(-)

Detailed changes

crates/language_tools/src/lsp_button.rs 🔗

@@ -94,7 +94,7 @@ struct LanguageServerBinaryStatus {
 #[derive(Debug)]
 struct ServerInfo {
     name: LanguageServerName,
-    id: Option<LanguageServerId>,
+    id: LanguageServerId,
     health: Option<ServerHealth>,
     binary_status: Option<LanguageServerBinaryStatus>,
     message: Option<SharedString>,
@@ -102,9 +102,7 @@ struct ServerInfo {
 
 impl ServerInfo {
     fn server_selector(&self) -> LanguageServerSelector {
-        self.id
-            .map(LanguageServerSelector::Id)
-            .unwrap_or_else(|| LanguageServerSelector::Name(self.name.clone()))
+        LanguageServerSelector::Id(self.id)
     }
 }
 
@@ -214,7 +212,6 @@ impl LanguageServerState {
             let Some(server_info) = item.server_info() else {
                 continue;
             };
-
             let server_selector = server_info.server_selector();
             let is_remote = self
                 .lsp_store
@@ -430,7 +427,7 @@ enum ServerData<'a> {
         binary_status: Option<&'a LanguageServerBinaryStatus>,
     },
     WithBinaryStatus {
-        server_id: Option<LanguageServerId>,
+        server_id: LanguageServerId,
         server_name: &'a LanguageServerName,
         binary_status: &'a LanguageServerBinaryStatus,
     },
@@ -444,7 +441,7 @@ enum LspMenuItem {
         binary_status: Option<LanguageServerBinaryStatus>,
     },
     WithBinaryStatus {
-        server_id: Option<LanguageServerId>,
+        server_id: LanguageServerId,
         server_name: LanguageServerName,
         binary_status: LanguageServerBinaryStatus,
     },
@@ -469,7 +466,7 @@ impl LspMenuItem {
                 ..
             } => Some(ServerInfo {
                 name: health.name.clone(),
-                id: Some(*server_id),
+                id: *server_id,
                 health: health.health(),
                 binary_status: binary_status.clone(),
                 message: health.message(),
@@ -591,6 +588,8 @@ impl LspButton {
         };
         let mut updated = false;
 
+        // TODO `LspStore` is global and reports status from all language servers, even from the other windows.
+        // Also, we do not get "LSP removed" events so LSPs are never removed.
         match e {
             LspStoreEvent::LanguageServerUpdate {
                 language_server_id,
@@ -758,7 +757,6 @@ impl LspButton {
                 .ok();
 
             let mut servers_per_worktree = BTreeMap::<SharedString, Vec<ServerData>>::new();
-            let mut servers_without_worktree = Vec::<ServerData>::new();
             let mut servers_with_health_checks = HashSet::default();
 
             for (server_id, health) in &state.language_servers.health_statuses {
@@ -780,12 +778,11 @@ impl LspButton {
                     health,
                     binary_status,
                 };
-                match worktree_name {
-                    Some(worktree_name) => servers_per_worktree
+                if let Some(worktree_name) = worktree_name {
+                    servers_per_worktree
                         .entry(worktree_name.clone())
                         .or_default()
-                        .push(server_data),
-                    None => servers_without_worktree.push(server_data),
+                        .push(server_data);
                 }
             }
 
@@ -822,42 +819,25 @@ impl LspButton {
                     BinaryStatus::Failed { .. } => {}
                 }
 
-                match server_names_to_worktrees.get(server_name) {
-                    Some(worktrees_for_name) => {
-                        match worktrees_for_name
-                            .iter()
-                            .find(|(worktree, _)| active_worktrees.contains(worktree))
-                            .or_else(|| worktrees_for_name.iter().next())
-                        {
-                            Some((worktree, server_id)) => {
-                                let worktree_name =
-                                    SharedString::new(worktree.read(cx).root_name_str());
-                                servers_per_worktree
-                                    .entry(worktree_name.clone())
-                                    .or_default()
-                                    .push(ServerData::WithBinaryStatus {
-                                        server_name,
-                                        binary_status,
-                                        server_id: Some(*server_id),
-                                    });
-                            }
-                            None => servers_without_worktree.push(ServerData::WithBinaryStatus {
-                                server_name,
-                                binary_status,
-                                server_id: None,
-                            }),
-                        }
-                    }
-                    None => servers_without_worktree.push(ServerData::WithBinaryStatus {
-                        server_name,
-                        binary_status,
-                        server_id: None,
-                    }),
+                if let Some(worktrees_for_name) = server_names_to_worktrees.get(server_name)
+                    && let Some((worktree, server_id)) = worktrees_for_name
+                        .iter()
+                        .find(|(worktree, _)| active_worktrees.contains(worktree))
+                        .or_else(|| worktrees_for_name.iter().next())
+                {
+                    let worktree_name = SharedString::new(worktree.read(cx).root_name_str());
+                    servers_per_worktree
+                        .entry(worktree_name.clone())
+                        .or_default()
+                        .push(ServerData::WithBinaryStatus {
+                            server_name,
+                            binary_status,
+                            server_id: *server_id,
+                        });
                 }
             }
 
-            let mut new_lsp_items =
-                Vec::with_capacity(servers_per_worktree.len() + servers_without_worktree.len() + 2);
+            let mut new_lsp_items = Vec::with_capacity(servers_per_worktree.len() + 1);
             for (worktree_name, worktree_servers) in servers_per_worktree {
                 if worktree_servers.is_empty() {
                     continue;
@@ -868,17 +848,6 @@ impl LspButton {
                 });
                 new_lsp_items.extend(worktree_servers.into_iter().map(ServerData::into_lsp_item));
             }
-            if !servers_without_worktree.is_empty() {
-                new_lsp_items.push(LspMenuItem::Header {
-                    header: Some(SharedString::from("Unknown worktree")),
-                    separator: false,
-                });
-                new_lsp_items.extend(
-                    servers_without_worktree
-                        .into_iter()
-                        .map(ServerData::into_lsp_item),
-                );
-            }
             if !new_lsp_items.is_empty() {
                 if can_stop_all {
                     new_lsp_items.push(LspMenuItem::ToggleServersButton { restart: true });