Fix language servers improper restarts (#2702)

Fixes
https://linear.app/zed-industries/issue/Z-2595/language-servers-are-unnecessarily-restarted-when-unrelated-settings

Language servers mixed `initialization_options` from hardcodes and user
settings, fix that to ensure we restart servers on their settings
changes only.

Release Notes:

- N/A

crates/editor/src/editor_tests.rs

@@ -22,7 +22,10 @@ use language::{
     BracketPairConfig, FakeLspAdapter, LanguageConfig, LanguageRegistry, Point,
 };
 use parking_lot::Mutex;
+use project::project_settings::{LspSettings, ProjectSettings};
 use project::FakeFs;
+use std::sync::atomic;
+use std::sync::atomic::AtomicUsize;
 use std::{cell::RefCell, future::Future, rc::Rc, time::Instant};
 use unindent::Unindent;
 use util::{
@@ -1796,7 +1799,7 @@ async fn test_newline_comments(cx: &mut gpui::TestAppContext) {
     "});
     }
     // Ensure that comment continuations can be disabled.
-    update_test_settings(cx, |settings| {
+    update_test_language_settings(cx, |settings| {
         settings.defaults.extend_comment_on_newline = Some(false);
     });
     let mut cx = EditorTestContext::new(cx).await;
@@ -4546,7 +4549,7 @@ async fn test_document_format_during_save(cx: &mut gpui::TestAppContext) {
     assert!(!cx.read(|cx| editor.is_dirty(cx)));
 
     // Set rust language override and assert overridden tabsize is sent to language server
-    update_test_settings(cx, |settings| {
+    update_test_language_settings(cx, |settings| {
         settings.languages.insert(
             "Rust".into(),
             LanguageSettingsContent {
@@ -4660,7 +4663,7 @@ async fn test_range_format_during_save(cx: &mut gpui::TestAppContext) {
     assert!(!cx.read(|cx| editor.is_dirty(cx)));
 
     // Set rust language override and assert overridden tabsize is sent to language server
-    update_test_settings(cx, |settings| {
+    update_test_language_settings(cx, |settings| {
         settings.languages.insert(
             "Rust".into(),
             LanguageSettingsContent {
@@ -7084,6 +7087,142 @@ async fn test_on_type_formatting_not_triggered(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_language_server_restart_due_to_settings_change(cx: &mut gpui::TestAppContext) {
+    init_test(cx, |_| {});
+
+    let language_name: Arc<str> = "Rust".into();
+    let mut language = Language::new(
+        LanguageConfig {
+            name: Arc::clone(&language_name),
+            path_suffixes: vec!["rs".to_string()],
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::language()),
+    );
+
+    let server_restarts = Arc::new(AtomicUsize::new(0));
+    let closure_restarts = Arc::clone(&server_restarts);
+    let language_server_name = "test language server";
+    let mut fake_servers = language
+        .set_fake_lsp_adapter(Arc::new(FakeLspAdapter {
+            name: language_server_name,
+            initialization_options: Some(json!({
+                "testOptionValue": true
+            })),
+            initializer: Some(Box::new(move |fake_server| {
+                let task_restarts = Arc::clone(&closure_restarts);
+                fake_server.handle_request::<lsp::request::Shutdown, _, _>(move |_, _| {
+                    task_restarts.fetch_add(1, atomic::Ordering::Release);
+                    futures::future::ready(Ok(()))
+                });
+            })),
+            ..Default::default()
+        }))
+        .await;
+
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree(
+        "/a",
+        json!({
+            "main.rs": "fn main() { let a = 5; }",
+            "other.rs": "// Test file",
+        }),
+    )
+    .await;
+    let project = Project::test(fs, ["/a".as_ref()], cx).await;
+    project.update(cx, |project, _| project.languages().add(Arc::new(language)));
+    let (_, _workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+    let _buffer = project
+        .update(cx, |project, cx| {
+            project.open_local_buffer("/a/main.rs", cx)
+        })
+        .await
+        .unwrap();
+    let _fake_server = fake_servers.next().await.unwrap();
+    update_test_language_settings(cx, |language_settings| {
+        language_settings.languages.insert(
+            Arc::clone(&language_name),
+            LanguageSettingsContent {
+                tab_size: NonZeroU32::new(8),
+                ..Default::default()
+            },
+        );
+    });
+    cx.foreground().run_until_parked();
+    assert_eq!(
+        server_restarts.load(atomic::Ordering::Acquire),
+        0,
+        "Should not restart LSP server on an unrelated change"
+    );
+
+    update_test_project_settings(cx, |project_settings| {
+        project_settings.lsp.insert(
+            "Some other server name".into(),
+            LspSettings {
+                initialization_options: Some(json!({
+                    "some other init value": false
+                })),
+            },
+        );
+    });
+    cx.foreground().run_until_parked();
+    assert_eq!(
+        server_restarts.load(atomic::Ordering::Acquire),
+        0,
+        "Should not restart LSP server on an unrelated LSP settings change"
+    );
+
+    update_test_project_settings(cx, |project_settings| {
+        project_settings.lsp.insert(
+            language_server_name.into(),
+            LspSettings {
+                initialization_options: Some(json!({
+                    "anotherInitValue": false
+                })),
+            },
+        );
+    });
+    cx.foreground().run_until_parked();
+    assert_eq!(
+        server_restarts.load(atomic::Ordering::Acquire),
+        1,
+        "Should restart LSP server on a related LSP settings change"
+    );
+
+    update_test_project_settings(cx, |project_settings| {
+        project_settings.lsp.insert(
+            language_server_name.into(),
+            LspSettings {
+                initialization_options: Some(json!({
+                    "anotherInitValue": false
+                })),
+            },
+        );
+    });
+    cx.foreground().run_until_parked();
+    assert_eq!(
+        server_restarts.load(atomic::Ordering::Acquire),
+        1,
+        "Should not restart LSP server on a related LSP settings change that is the same"
+    );
+
+    update_test_project_settings(cx, |project_settings| {
+        project_settings.lsp.insert(
+            language_server_name.into(),
+            LspSettings {
+                initialization_options: None,
+            },
+        );
+    });
+    cx.foreground().run_until_parked();
+    assert_eq!(
+        server_restarts.load(atomic::Ordering::Acquire),
+        2,
+        "Should restart LSP server on another related LSP settings change"
+    );
+}
+
 fn empty_range(row: usize, column: usize) -> Range<DisplayPoint> {
     let point = DisplayPoint::new(row as u32, column as u32);
     point..point
@@ -7203,7 +7342,7 @@ fn handle_copilot_completion_request(
     });
 }
 
-pub(crate) fn update_test_settings(
+pub(crate) fn update_test_language_settings(
     cx: &mut TestAppContext,
     f: impl Fn(&mut AllLanguageSettingsContent),
 ) {
@@ -7214,6 +7353,17 @@ pub(crate) fn update_test_settings(
     });
 }
 
+pub(crate) fn update_test_project_settings(
+    cx: &mut TestAppContext,
+    f: impl Fn(&mut ProjectSettings),
+) {
+    cx.update(|cx| {
+        cx.update_global::<SettingsStore, _, _>(|store, cx| {
+            store.update_user_settings::<ProjectSettings>(cx, f);
+        });
+    });
+}
+
 pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsContent)) {
     cx.foreground().forbid_parking();
 
@@ -7227,5 +7377,5 @@ pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsC
         crate::init(cx);
     });
 
-    update_test_settings(cx, f);
+    update_test_language_settings(cx, f);
 }

crates/editor/src/element.rs

@@ -2916,7 +2916,7 @@ mod tests {
     use super::*;
     use crate::{
         display_map::{BlockDisposition, BlockProperties},
-        editor_tests::{init_test, update_test_settings},
+        editor_tests::{init_test, update_test_language_settings},
         Editor, MultiBuffer,
     };
     use gpui::TestAppContext;
@@ -3113,7 +3113,7 @@ mod tests {
         let resize_step = 10.0;
         let mut editor_width = 200.0;
         while editor_width <= 1000.0 {
-            update_test_settings(cx, |s| {
+            update_test_language_settings(cx, |s| {
                 s.defaults.tab_size = NonZeroU32::new(tab_size);
                 s.defaults.show_whitespaces = Some(ShowWhitespaceSetting::All);
                 s.defaults.preferred_line_length = Some(editor_width as u32);

crates/editor/src/inlay_hint_cache.rs

@@ -847,7 +847,7 @@ mod tests {
     use text::Point;
     use workspace::Workspace;
 
-    use crate::editor_tests::update_test_settings;
+    use crate::editor_tests::update_test_language_settings;
 
     use super::*;
 
@@ -1476,7 +1476,7 @@ mod tests {
             ),
         ] {
             edits_made += 1;
-            update_test_settings(cx, |settings| {
+            update_test_language_settings(cx, |settings| {
                 settings.defaults.inlay_hints = Some(InlayHintSettings {
                     enabled: true,
                     show_type_hints: new_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
@@ -1520,7 +1520,7 @@ mod tests {
 
         edits_made += 1;
         let another_allowed_hint_kinds = HashSet::from_iter([Some(InlayHintKind::Type)]);
-        update_test_settings(cx, |settings| {
+        update_test_language_settings(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: false,
                 show_type_hints: another_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
@@ -1577,7 +1577,7 @@ mod tests {
 
         let final_allowed_hint_kinds = HashSet::from_iter([Some(InlayHintKind::Parameter)]);
         edits_made += 1;
-        update_test_settings(cx, |settings| {
+        update_test_language_settings(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: true,
                 show_type_hints: final_allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
@@ -2269,7 +2269,7 @@ unedited (2nd) buffer should have the same hint");
             crate::init(cx);
         });
 
-        update_test_settings(cx, f);
+        update_test_language_settings(cx, f);
     }
 
     async fn prepare_test_objects(

crates/language/src/language.rs

@@ -90,7 +90,8 @@ pub struct LanguageServerName(pub Arc<str>);
 /// once at startup, and caches the results.
 pub struct CachedLspAdapter {
     pub name: LanguageServerName,
-    pub initialization_options: Option<Value>,
+    initialization_options: Option<Value>,
+    initialization_overrides: Mutex<Option<Value>>,
     pub disk_based_diagnostic_sources: Vec<String>,
     pub disk_based_diagnostics_progress_token: Option<String>,
     pub language_ids: HashMap<String, String>,
@@ -109,6 +110,7 @@ impl CachedLspAdapter {
         Arc::new(CachedLspAdapter {
             name,
             initialization_options,
+            initialization_overrides: Mutex::new(None),
             disk_based_diagnostic_sources,
             disk_based_diagnostics_progress_token,
             language_ids,
@@ -208,6 +210,30 @@ impl CachedLspAdapter {
     ) -> Option<CodeLabel> {
         self.adapter.label_for_symbol(name, kind, language).await
     }
+
+    pub fn update_initialization_overrides(&self, new: Option<&Value>) -> bool {
+        let mut current = self.initialization_overrides.lock();
+        if current.as_ref() != new {
+            *current = new.cloned();
+            true
+        } else {
+            false
+        }
+    }
+
+    pub fn initialization_options(&self) -> Option<Value> {
+        let initialization_options = self.initialization_options.as_ref();
+        let override_options = self.initialization_overrides.lock().clone();
+        match (initialization_options, override_options) {
+            (None, override_options) => override_options,
+            (initialization_options, None) => initialization_options.cloned(),
+            (Some(initialization_options), Some(override_options)) => {
+                let mut initialization_options = initialization_options.clone();
+                merge_json_value_into(override_options, &mut initialization_options);
+                Some(initialization_options)
+            }
+        }
+    }
 }
 
 pub trait LspAdapterDelegate: Send + Sync {
@@ -427,6 +453,7 @@ fn deserialize_regex<'de, D: Deserializer<'de>>(d: D) -> Result<Option<Regex>, D
 #[cfg(any(test, feature = "test-support"))]
 pub struct FakeLspAdapter {
     pub name: &'static str,
+    pub initialization_options: Option<Value>,
     pub capabilities: lsp::ServerCapabilities,
     pub initializer: Option<Box<dyn 'static + Send + Sync + Fn(&mut lsp::FakeLanguageServer)>>,
     pub disk_based_diagnostics_progress_token: Option<String>,
@@ -1637,6 +1664,7 @@ impl Default for FakeLspAdapter {
             capabilities: lsp::LanguageServer::full_capabilities(),
             initializer: None,
             disk_based_diagnostics_progress_token: None,
+            initialization_options: None,
             disk_based_diagnostics_sources: Vec::new(),
         }
     }
@@ -1686,6 +1714,10 @@ impl LspAdapter for Arc<FakeLspAdapter> {
     async fn disk_based_diagnostics_progress_token(&self) -> Option<String> {
         self.disk_based_diagnostics_progress_token.clone()
     }
+
+    async fn initialization_options(&self) -> Option<Value> {
+        self.initialization_options.clone()
+    }
 }
 
 fn get_capture_indices(query: &Query, captures: &mut [(&str, &mut Option<u32>)]) {

crates/project/src/project.rs

@@ -78,8 +78,8 @@ use std::{
 use terminals::Terminals;
 use text::Anchor;
 use util::{
-    debug_panic, defer, http::HttpClient, merge_json_value_into,
-    paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, TryFutureExt as _,
+    debug_panic, defer, http::HttpClient, paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt,
+    TryFutureExt as _,
 };
 
 pub use fs::*;
@@ -800,7 +800,7 @@ impl Project {
                         .lsp
                         .get(&adapter.name.0)
                         .and_then(|s| s.initialization_options.as_ref());
-                    if adapter.initialization_options.as_ref() != new_lsp_settings {
+                    if adapter.update_initialization_overrides(new_lsp_settings) {
                         language_servers_to_restart.push((worktree, Arc::clone(language)));
                     }
                 }
@@ -2545,20 +2545,13 @@ impl Project {
         let project_settings = settings::get::<ProjectSettings>(cx);
         let lsp = project_settings.lsp.get(&adapter.name.0);
         let override_options = lsp.map(|s| s.initialization_options.clone()).flatten();
-
-        let mut initialization_options = adapter.initialization_options.clone();
-        match (&mut initialization_options, override_options) {
-            (Some(initialization_options), Some(override_options)) => {
-                merge_json_value_into(override_options, initialization_options);
-            }
-            (None, override_options) => initialization_options = override_options,
-            _ => {}
-        }
+        adapter.update_initialization_overrides(override_options.as_ref());
 
         let server_id = pending_server.server_id;
         let container_dir = pending_server.container_dir.clone();
         let state = LanguageServerState::Starting({
             let adapter = adapter.clone();
+            let initialization_options = adapter.initialization_options();
             let server_name = adapter.name.0.clone();
             let languages = self.languages.clone();
             let language = language.clone();