Respect the `language_servers` setting's order when determining the primary language server (#15624)

Marshall Bowers created 1 year ago

This PR updates how we determine the "primary" language server for a
buffer to make it respect the order specified by the `language_servers`
setting.

Previously we were relying on the language servers to be registered in
the right order in order to select the primary one effectively.

However, in my testing I observed some cases where a native language
server (e.g., `tailwindcss-language-server`) could end up first in the
list of language servers despite not being first in the
`language_servers` setting.

While this wasn't a problem for the Tailwind or ESLint language servers
on account of them being defined natively with the designation of
"secondary" language servers, this could cause problems with
extension-based language servers.

To remedy this, every time we start up language servers we reorder the
list of language servers for a given language to reflect the order in
the `language_servers` setting. This ordering then allows us to treat
the first language server in the list as the "primary" one.

Related issues:

- https://github.com/zed-industries/zed/issues/15023
- https://github.com/zed-industries/zed/issues/15279

Release Notes:

- The ordering of language servers will now respect the order in the
`language_servers` setting.
- The first language server in this list will be used as the primary
language server.

Change summary

crates/language/src/language_registry.rs | 48 +++++++++++++++++++++++++
crates/project/src/project.rs            | 19 ++++++++--
2 files changed, 62 insertions(+), 5 deletions(-)

Detailed changes

crates/language/src/language_registry.rs

@@ -7,7 +7,7 @@ use crate::{
     LanguageServerName, LspAdapter, LspAdapterDelegate, PLAIN_TEXT,
 };
 use anyhow::{anyhow, Context as _, Result};
-use collections::{hash_map, HashMap};
+use collections::{hash_map, HashMap, HashSet};
 use futures::TryFutureExt;
 use futures::{
     channel::{mpsc, oneshot},
@@ -188,6 +188,22 @@ impl LanguageRegistry {
         self.state.write().reload();
     }
 
+    /// Reorders the list of language servers for the given language.
+    ///
+    /// Uses the provided list of ordered [`CachedLspAdapters`] as the desired order.
+    ///
+    /// Any existing language servers not present in `ordered_lsp_adapters` will be
+    /// appended to the end.
+    pub fn reorder_language_servers(
+        &self,
+        language: &Arc<Language>,
+        ordered_lsp_adapters: Vec<Arc<CachedLspAdapter>>,
+    ) {
+        self.state
+            .write()
+            .reorder_language_servers(language, ordered_lsp_adapters);
+    }
+
     /// Removes the specified languages and grammars from the registry.
     pub fn remove_languages(
         &self,
@@ -920,6 +936,36 @@ impl LanguageRegistryState {
         *self.subscription.0.borrow_mut() = ();
     }
 
+    /// Reorders the list of language servers for the given language.
+    ///
+    /// Uses the provided list of ordered [`CachedLspAdapters`] as the desired order.
+    ///
+    /// Any existing language servers not present in `ordered_lsp_adapters` will be
+    /// appended to the end.
+    fn reorder_language_servers(
+        &mut self,
+        language: &Arc<Language>,
+        ordered_lsp_adapters: Vec<Arc<CachedLspAdapter>>,
+    ) {
+        let Some(lsp_adapters) = self.lsp_adapters.get_mut(&language.config.name) else {
+            return;
+        };
+
+        let ordered_lsp_adapter_ids = ordered_lsp_adapters
+            .iter()
+            .map(|lsp_adapter| lsp_adapter.name.clone())
+            .collect::<HashSet<_>>();
+
+        let mut new_lsp_adapters = ordered_lsp_adapters;
+        for adapter in lsp_adapters.iter() {
+            if !ordered_lsp_adapter_ids.contains(&adapter.name) {
+                new_lsp_adapters.push(adapter.clone());
+            }
+        }
+
+        *lsp_adapters = new_lsp_adapters;
+    }
+
     fn remove_languages(
         &mut self,
         languages_to_remove: &[Arc<str>],

crates/project/src/project.rs

@@ -2999,9 +2999,18 @@ impl Project {
                 .join(", ")
         );
 
-        for adapter in enabled_lsp_adapters {
-            self.start_language_server(worktree, adapter, language.clone(), cx);
+        for adapter in &enabled_lsp_adapters {
+            self.start_language_server(worktree, adapter.clone(), language.clone(), cx);
         }
+
+        // After starting all the language servers, reorder them to reflect the desired order
+        // based on the settings.
+        //
+        // This is done, in part, to ensure that language servers loaded at different points
+        // (e.g., native vs extension) still end up in the right order at the end, rather than
+        // it being based on which language server happened to be loaded in first.
+        self.languages()
+            .reorder_language_servers(&language, enabled_lsp_adapters);
     }
 
     fn start_language_server(
@@ -10247,8 +10256,10 @@ impl Project {
         buffer: &Buffer,
         cx: &AppContext,
     ) -> Option<(&Arc<CachedLspAdapter>, &Arc<LanguageServer>)> {
-        self.language_servers_for_buffer(buffer, cx)
-            .find(|s| s.0.is_primary)
+        // The list of language servers is ordered based on the `language_servers` setting
+        // for each language, thus we can consider the first one in the list to be the
+        // primary one.
+        self.language_servers_for_buffer(buffer, cx).next()
     }
 
     pub fn language_server_for_buffer(