Fix LSP shenanigans when opening and/or editing settings files (#4212)

Fixes
* LSP servers never being shut down on worktree release
* worktrees (and LSP servers) being re-created for settings files on
every opening

Also,
* removes `async` from `workspace_configuration` to simplify the code:
we only return static configurations now

Release Notes:

- Fixed excessive LSP server creation for Zed settings files

crates/language/src/language.rs

@@ -23,7 +23,7 @@ use async_trait::async_trait;
 use collections::{HashMap, HashSet};
 use futures::{
     channel::{mpsc, oneshot},
-    future::{BoxFuture, Shared},
+    future::Shared,
     FutureExt, TryFutureExt as _,
 };
 use gpui::{AppContext, AsyncAppContext, BackgroundExecutor, Task};
@@ -216,11 +216,7 @@ impl CachedLspAdapter {
         self.adapter.code_action_kinds()
     }
 
-    pub fn workspace_configuration(
-        &self,
-        workspace_root: &Path,
-        cx: &mut AppContext,
-    ) -> BoxFuture<'static, Value> {
+    pub fn workspace_configuration(&self, workspace_root: &Path, cx: &mut AppContext) -> Value {
         self.adapter.workspace_configuration(workspace_root, cx)
     }
 
@@ -345,8 +341,8 @@ pub trait LspAdapter: 'static + Send + Sync {
         None
     }
 
-    fn workspace_configuration(&self, _: &Path, _: &mut AppContext) -> BoxFuture<'static, Value> {
-        futures::future::ready(serde_json::json!({})).boxed()
+    fn workspace_configuration(&self, _: &Path, _: &mut AppContext) -> Value {
+        serde_json::json!({})
     }
 
     /// Returns a list of code actions supported by a given LspAdapter

crates/project/src/prettier_support.rs

@@ -16,7 +16,7 @@ use language::{
     language_settings::{Formatter, LanguageSettings},
     Buffer, Language, LanguageServerName, LocalFile,
 };
-use lsp::LanguageServerId;
+use lsp::{LanguageServer, LanguageServerId};
 use node_runtime::NodeRuntime;
 use prettier::Prettier;
 use util::{paths::DEFAULT_PRETTIER_DIR, ResultExt, TryFutureExt};
@@ -212,6 +212,10 @@ impl PrettierInstance {
             },
         })
     }
+
+    pub async fn server(&self) -> Option<Arc<LanguageServer>> {
+        self.prettier.clone()?.await.ok()?.server().cloned()
+    }
 }
 
 fn start_default_prettier(

crates/project/src/project.rs

@@ -632,7 +632,7 @@ impl Project {
             let copilot_lsp_subscription =
                 Copilot::global(cx).map(|copilot| subscribe_for_copilot_events(&copilot, cx));
             Self {
-                worktrees: Default::default(),
+                worktrees: Vec::new(),
                 buffer_ordered_messages_tx: tx,
                 collaborators: Default::default(),
                 next_buffer_id: 0,
@@ -664,7 +664,7 @@ impl Project {
                 next_diagnostic_group_id: Default::default(),
                 supplementary_language_servers: HashMap::default(),
                 language_servers: Default::default(),
-                language_server_ids: Default::default(),
+                language_server_ids: HashMap::default(),
                 language_server_statuses: Default::default(),
                 last_workspace_edits_by_language_server: Default::default(),
                 buffers_being_formatted: Default::default(),
@@ -752,7 +752,7 @@ impl Project {
                 },
                 supplementary_language_servers: HashMap::default(),
                 language_servers: Default::default(),
-                language_server_ids: Default::default(),
+                language_server_ids: HashMap::default(),
                 language_server_statuses: response
                     .payload
                     .language_servers
@@ -2700,7 +2700,7 @@ impl Project {
         });
 
         cx.spawn(move |this, mut cx| async move {
-            while let Some(_) = settings_changed_rx.next().await {
+            while let Some(()) = settings_changed_rx.next().await {
                 let servers: Vec<_> = this.update(&mut cx, |this, _| {
                     this.language_servers
                         .values()
@@ -2714,9 +2714,8 @@ impl Project {
                 })?;
 
                 for (adapter, server) in servers {
-                    let workspace_config = cx
-                        .update(|cx| adapter.workspace_configuration(server.root_path(), cx))?
-                        .await;
+                    let workspace_config =
+                        cx.update(|cx| adapter.workspace_configuration(server.root_path(), cx))?;
                     server
                         .notify::<lsp::notification::DidChangeConfiguration>(
                             lsp::DidChangeConfigurationParams {
@@ -3020,9 +3019,8 @@ impl Project {
         server_id: LanguageServerId,
         cx: &mut AsyncAppContext,
     ) -> Result<Arc<LanguageServer>> {
-        let workspace_config = cx
-            .update(|cx| adapter.workspace_configuration(worktree_path, cx))?
-            .await;
+        let workspace_config =
+            cx.update(|cx| adapter.workspace_configuration(worktree_path, cx))?;
         let language_server = pending_server.task.await?;
 
         language_server
@@ -3056,9 +3054,8 @@ impl Project {
                     let adapter = adapter.clone();
                     let worktree_path = worktree_path.clone();
                     async move {
-                        let workspace_config = cx
-                            .update(|cx| adapter.workspace_configuration(&worktree_path, cx))?
-                            .await;
+                        let workspace_config =
+                            cx.update(|cx| adapter.workspace_configuration(&worktree_path, cx))?;
                         Ok(params
                             .items
                             .into_iter()
@@ -6370,6 +6367,55 @@ impl Project {
     }
 
     pub fn remove_worktree(&mut self, id_to_remove: WorktreeId, cx: &mut ModelContext<Self>) {
+        let mut servers_to_remove = HashMap::default();
+        let mut servers_to_preserve = HashSet::default();
+        for ((worktree_id, server_name), &server_id) in &self.language_server_ids {
+            if worktree_id == &id_to_remove {
+                servers_to_remove.insert(server_id, server_name.clone());
+            } else {
+                servers_to_preserve.insert(server_id);
+            }
+        }
+        servers_to_remove.retain(|server_id, _| !servers_to_preserve.contains(server_id));
+        for (server_id_to_remove, server_name) in servers_to_remove {
+            self.language_server_ids
+                .remove(&(id_to_remove, server_name));
+            self.language_server_statuses.remove(&server_id_to_remove);
+            self.last_workspace_edits_by_language_server
+                .remove(&server_id_to_remove);
+            self.language_servers.remove(&server_id_to_remove);
+            cx.emit(Event::LanguageServerRemoved(server_id_to_remove));
+        }
+
+        let mut prettier_instances_to_clean = FuturesUnordered::new();
+        if let Some(prettier_paths) = self.prettiers_per_worktree.remove(&id_to_remove) {
+            for path in prettier_paths.iter().flatten() {
+                if let Some(prettier_instance) = self.prettier_instances.remove(path) {
+                    prettier_instances_to_clean.push(async move {
+                        prettier_instance
+                            .server()
+                            .await
+                            .map(|server| server.server_id())
+                    });
+                }
+            }
+        }
+        cx.spawn(|project, mut cx| async move {
+            while let Some(prettier_server_id) = prettier_instances_to_clean.next().await {
+                if let Some(prettier_server_id) = prettier_server_id {
+                    project
+                        .update(&mut cx, |project, cx| {
+                            project
+                                .supplementary_language_servers
+                                .remove(&prettier_server_id);
+                            cx.emit(Event::LanguageServerRemoved(prettier_server_id));
+                        })
+                        .ok();
+                }
+            }
+        })
+        .detach();
+
         self.worktrees.retain(|worktree| {
             if let Some(worktree) = worktree.upgrade() {
                 let id = worktree.read(cx).id();

crates/zed/src/languages/json.rs

@@ -2,7 +2,7 @@ use anyhow::{anyhow, Result};
 use async_trait::async_trait;
 use collections::HashMap;
 use feature_flags::FeatureFlagAppExt;
-use futures::{future::BoxFuture, FutureExt, StreamExt};
+use futures::StreamExt;
 use gpui::AppContext;
 use language::{LanguageRegistry, LanguageServerName, LspAdapter, LspAdapterDelegate};
 use lsp::LanguageServerBinary;
@@ -13,7 +13,6 @@ use smol::fs;
 use std::{
     any::Any,
     ffi::OsString,
-    future,
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -107,7 +106,7 @@ impl LspAdapter for JsonLspAdapter {
         &self,
         _workspace_root: &Path,
         cx: &mut AppContext,
-    ) -> BoxFuture<'static, serde_json::Value> {
+    ) -> serde_json::Value {
         let action_names = cx.all_action_names();
         let staff_mode = cx.is_staff();
         let language_names = &self.languages.language_names();
@@ -119,7 +118,7 @@ impl LspAdapter for JsonLspAdapter {
             cx,
         );
 
-        future::ready(serde_json::json!({
+        serde_json::json!({
             "json": {
                 "format": {
                     "enable": true,
@@ -138,8 +137,7 @@ impl LspAdapter for JsonLspAdapter {
                     }
                 ]
             }
-        }))
-        .boxed()
+        })
     }
 
     async fn language_ids(&self) -> HashMap<String, String> {

crates/zed/src/languages/tailwind.rs

@@ -1,10 +1,7 @@
 use anyhow::{anyhow, Result};
 use async_trait::async_trait;
 use collections::HashMap;
-use futures::{
-    future::{self, BoxFuture},
-    FutureExt, StreamExt,
-};
+use futures::StreamExt;
 use gpui::AppContext;
 use language::{LanguageServerName, LspAdapter, LspAdapterDelegate};
 use lsp::LanguageServerBinary;
@@ -107,17 +104,12 @@ impl LspAdapter for TailwindLspAdapter {
         }))
     }
 
-    fn workspace_configuration(
-        &self,
-        _workspace_root: &Path,
-        _: &mut AppContext,
-    ) -> BoxFuture<'static, Value> {
-        future::ready(json!({
+    fn workspace_configuration(&self, _workspace_root: &Path, _: &mut AppContext) -> Value {
+        json!({
             "tailwindCSS": {
                 "emmetCompletions": true,
             }
-        }))
-        .boxed()
+        })
     }
 
     async fn language_ids(&self) -> HashMap<String, String> {

crates/zed/src/languages/typescript.rs

@@ -3,7 +3,6 @@ use async_compression::futures::bufread::GzipDecoder;
 use async_tar::Archive;
 use async_trait::async_trait;
 use collections::HashMap;
-use futures::{future::BoxFuture, FutureExt};
 use gpui::AppContext;
 use language::{LanguageServerName, LspAdapter, LspAdapterDelegate};
 use lsp::{CodeActionKind, LanguageServerBinary};
@@ -13,7 +12,6 @@ use smol::{fs, io::BufReader, stream::StreamExt};
 use std::{
     any::Any,
     ffi::OsString,
-    future,
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -212,12 +210,8 @@ impl EsLintLspAdapter {
 
 #[async_trait]
 impl LspAdapter for EsLintLspAdapter {
-    fn workspace_configuration(
-        &self,
-        workspace_root: &Path,
-        _: &mut AppContext,
-    ) -> BoxFuture<'static, Value> {
-        future::ready(json!({
+    fn workspace_configuration(&self, workspace_root: &Path, _: &mut AppContext) -> Value {
+        json!({
             "": {
                 "validate": "on",
                 "rulesCustomizations": [],
@@ -230,8 +224,7 @@ impl LspAdapter for EsLintLspAdapter {
                         .unwrap_or_else(|| workspace_root.as_os_str()),
                 },
             }
-        }))
-        .boxed()
+        })
     }
 
     async fn name(&self) -> LanguageServerName {

crates/zed/src/languages/yaml.rs

@@ -1,6 +1,6 @@
 use anyhow::{anyhow, Result};
 use async_trait::async_trait;
-use futures::{future::BoxFuture, FutureExt, StreamExt};
+use futures::StreamExt;
 use gpui::AppContext;
 use language::{
     language_settings::all_language_settings, LanguageServerName, LspAdapter, LspAdapterDelegate,
@@ -12,7 +12,6 @@ use smol::fs;
 use std::{
     any::Any,
     ffi::OsString,
-    future,
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -93,24 +92,17 @@ impl LspAdapter for YamlLspAdapter {
     ) -> Option<LanguageServerBinary> {
         get_cached_server_binary(container_dir, &*self.node).await
     }
-    fn workspace_configuration(
-        &self,
-        _workspace_root: &Path,
-        cx: &mut AppContext,
-    ) -> BoxFuture<'static, Value> {
-        let tab_size = all_language_settings(None, cx)
-            .language(Some("YAML"))
-            .tab_size;
-
-        future::ready(serde_json::json!({
+    fn workspace_configuration(&self, _workspace_root: &Path, cx: &mut AppContext) -> Value {
+        serde_json::json!({
             "yaml": {
                 "keyOrdering": false
             },
             "[yaml]": {
-                "editor.tabSize": tab_size,
+                "editor.tabSize": all_language_settings(None, cx)
+                    .language(Some("YAML"))
+                    .tab_size,
             }
-        }))
-        .boxed()
+        })
     }
 }
 

crates/zed/src/zed.rs

@@ -20,9 +20,10 @@ use assets::Assets;
 use futures::{channel::mpsc, select_biased, StreamExt};
 use project_panel::ProjectPanel;
 use quick_action_bar::QuickActionBar;
+use rope::Rope;
 use search::project_search::ProjectSearchBar;
 use settings::{initial_local_settings_content, KeymapFile, Settings, SettingsStore};
-use std::{borrow::Cow, ops::Deref, sync::Arc};
+use std::{borrow::Cow, ops::Deref, path::Path, sync::Arc};
 use terminal_view::terminal_panel::{self, TerminalPanel};
 use util::{
     asset_str,
@@ -256,16 +257,16 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
             )
             .register_action(
                 move |_: &mut Workspace, _: &OpenKeymap, cx: &mut ViewContext<Workspace>| {
-                    create_and_open_local_file(&paths::KEYMAP, cx, Default::default)
-                        .detach_and_log_err(cx);
+                    open_settings_file(&paths::KEYMAP, Rope::default, cx);
                 },
             )
             .register_action(
                 move |_: &mut Workspace, _: &OpenSettings, cx: &mut ViewContext<Workspace>| {
-                    create_and_open_local_file(&paths::SETTINGS, cx, || {
-                        settings::initial_user_settings_content().as_ref().into()
-                    })
-                    .detach_and_log_err(cx);
+                    open_settings_file(
+                        &paths::SETTINGS,
+                        || settings::initial_user_settings_content().as_ref().into(),
+                        cx,
+                    );
                 },
             )
             .register_action(open_local_settings_file)
@@ -723,6 +724,30 @@ fn open_bundled_file(
     .detach_and_log_err(cx);
 }
 
+fn open_settings_file(
+    abs_path: &'static Path,
+    default_content: impl FnOnce() -> Rope + Send + 'static,
+    cx: &mut ViewContext<Workspace>,
+) {
+    cx.spawn(|workspace, mut cx| async move {
+        let (worktree_creation_task, settings_open_task) =
+            workspace.update(&mut cx, |workspace, cx| {
+                let worktree_creation_task = workspace.project().update(cx, |project, cx| {
+                    // Set up a dedicated worktree for settings, since otherwise we're dropping and re-starting LSP servers for each file inside on every settings file close/open
+                    // TODO: Do note that all other external files (e.g. drag and drop from OS) still have their worktrees released on file close, causing LSP servers' restarts.
+                    project.find_or_create_local_worktree(paths::CONFIG_DIR.as_path(), false, cx)
+                });
+                let settings_open_task = create_and_open_local_file(&abs_path, cx, default_content);
+                (worktree_creation_task, settings_open_task)
+            })?;
+
+        let _ = worktree_creation_task.await?;
+        let _ = settings_open_task.await?;
+        anyhow::Ok(())
+    })
+    .detach_and_log_err(cx);
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;