Restart LSP server on corresponding `initialization_options` change (#2690)

Kirill Bulatov created 2 years ago

Inlay hints depend on LSP server settings, but servers do not update the
initialization options and query hints with old settings.

Generally, we cannot know whether a certain option can be changed
without server restart, which the name of the options implies too, so be
on the safe side and restart the server.
Hints will update automatically after the server either sends a /refresh
request or reports its work progress end after startup.

Release Notes:

- Fixed LSP server not restarting after `initialization_options`
settings changes

Change summary

crates/collab/src/tests/integration_tests.rs | 55 +--------------------
crates/editor/src/inlay_hint_cache.rs        | 51 +++++---------------
crates/project/src/project.rs                | 27 ++++++++--
crates/workspace/src/workspace.rs            |  1 
4 files changed, 40 insertions(+), 94 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs

@@ -18,7 +18,7 @@ use gpui::{
 };
 use indoc::indoc;
 use language::{
-    language_settings::{AllLanguageSettings, Formatter, InlayHintKind, InlayHintSettings},
+    language_settings::{AllLanguageSettings, Formatter, InlayHintSettings},
     tree_sitter_rust, Anchor, Diagnostic, DiagnosticEntry, FakeLspAdapter, Language,
     LanguageConfig, OffsetRangeExt, Point, Rope,
 };
@@ -7843,7 +7843,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             });
         });
     });
-    let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
 
     let mut language = Language::new(
         LanguageConfig {
@@ -7955,10 +7954,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Host should get its first hints when opens an editor"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Cache should use editor settings to get the allowed hint kinds"
-        );
         assert_eq!(
             inlay_cache.version, edits_made,
             "Host editor update the cache version after every cache/view change",
@@ -7982,10 +7977,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Client should get its first hints when opens an editor"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Cache should use editor settings to get the allowed hint kinds"
-        );
         assert_eq!(
             inlay_cache.version, edits_made,
             "Guest editor update the cache version after every cache/view change"
@@ -8007,10 +7998,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Host should get hints from the 1st edit and 1st LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(inlay_cache.version, edits_made);
     });
     editor_b.update(cx_b, |editor, _| {
@@ -8025,10 +8012,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Guest should get hints the 1st edit and 2nd LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(inlay_cache.version, edits_made);
     });
 
@@ -8054,10 +8037,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
 4th query was made by guest (but not applied) due to cache invalidation logic"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(inlay_cache.version, edits_made);
     });
     editor_b.update(cx_b, |editor, _| {
@@ -8074,10 +8053,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Guest should get hints from 3rd edit, 6th LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(inlay_cache.version, edits_made);
     });
 
@@ -8103,10 +8078,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Host should react to /refresh LSP request and get new hints from 7th LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(
             inlay_cache.version, edits_made,
             "Host should accepted all edits and bump its cache version every time"
@@ -8128,10 +8099,6 @@ async fn test_mutual_editor_inlay_hint_cache_update(
             "Guest should get a /refresh LSP request propagated by host and get new hints from 8th LSP query"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(
             inlay_cache.version,
             edits_made,
@@ -8164,9 +8131,9 @@ async fn test_inlay_hint_refresh_is_forwarded(
             store.update_user_settings::<AllLanguageSettings>(cx, |settings| {
                 settings.defaults.inlay_hints = Some(InlayHintSettings {
                     enabled: false,
-                    show_type_hints: true,
+                    show_type_hints: false,
                     show_parameter_hints: false,
-                    show_other_hints: true,
+                    show_other_hints: false,
                 })
             });
         });
@@ -8177,13 +8144,12 @@ async fn test_inlay_hint_refresh_is_forwarded(
                 settings.defaults.inlay_hints = Some(InlayHintSettings {
                     enabled: true,
                     show_type_hints: true,
-                    show_parameter_hints: false,
+                    show_parameter_hints: true,
                     show_other_hints: true,
                 })
             });
         });
     });
-    let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
 
     let mut language = Language::new(
         LanguageConfig {
@@ -8299,10 +8265,6 @@ async fn test_inlay_hint_refresh_is_forwarded(
             "Host should get no hints due to them turned off"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Host should have allowed hint kinds set despite hints are off"
-        );
         assert_eq!(
             inlay_cache.version, 0,
             "Host should not increment its cache version due to no changes",
@@ -8318,10 +8280,6 @@ async fn test_inlay_hint_refresh_is_forwarded(
             "Client should get its first hints when opens an editor"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Cache should use editor settings to get the allowed hint kinds"
-        );
         assert_eq!(
             inlay_cache.version, edits_made,
             "Guest editor update the cache version after every cache/view change"
@@ -8339,7 +8297,6 @@ async fn test_inlay_hint_refresh_is_forwarded(
             "Host should get nop hints due to them turned off, even after the /refresh"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
         assert_eq!(
             inlay_cache.version, 0,
             "Host should not increment its cache version due to no changes",
@@ -8355,10 +8312,6 @@ async fn test_inlay_hint_refresh_is_forwarded(
             "Guest should get a /refresh LSP request propagated by host despite host hints are off"
         );
         let inlay_cache = editor.inlay_hint_cache();
-        assert_eq!(
-            inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-            "Inlay kinds settings never change during the test"
-        );
         assert_eq!(
             inlay_cache.version, edits_made,
             "Guest should accepted all edits and bump its cache version every time"

crates/editor/src/inlay_hint_cache.rs

@@ -833,7 +833,7 @@ mod tests {
     use crate::{
         scroll::{autoscroll::Autoscroll, scroll_amount::ScrollAmount},
         serde_json::json,
-        ExcerptRange, InlayHintSettings,
+        ExcerptRange,
     };
     use futures::StreamExt;
     use gpui::{executor::Deterministic, TestAppContext, ViewHandle};
@@ -1087,13 +1087,12 @@ mod tests {
 
     #[gpui::test]
     async fn test_no_hint_updates_for_unrelated_language_files(cx: &mut gpui::TestAppContext) {
-        let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
         init_test(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: true,
-                show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
-                show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
-                show_other_hints: allowed_hint_kinds.contains(&None),
+                show_type_hints: true,
+                show_parameter_hints: true,
+                show_other_hints: true,
             })
         });
 
@@ -1196,10 +1195,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(
-                inlay_cache.allowed_hint_kinds, allowed_hint_kinds,
-                "Cache should use editor settings to get the allowed hint kinds"
-            );
             assert_eq!(
                 inlay_cache.version, 1,
                 "Rust editor update the cache version after every cache/view change"
@@ -1258,7 +1253,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 1);
         });
 
@@ -1276,7 +1270,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, 2,
                 "Every time hint cache changes, cache version should be incremented"
@@ -1291,7 +1284,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 1);
         });
 
@@ -1309,7 +1301,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 2);
         });
         rs_editor.update(cx, |editor, cx| {
@@ -1321,7 +1312,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 2);
         });
     }
@@ -1444,7 +1434,6 @@ mod tests {
                 visible_hint_labels(editor, cx)
             );
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, edits_made,
                 "Should not update cache version due to new loaded hints being the same"
@@ -1580,7 +1569,6 @@ mod tests {
             assert!(cached_hint_labels(editor).is_empty());
             assert!(visible_hint_labels(editor, cx).is_empty());
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, another_allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, edits_made,
                 "The editor should not update the cache version after /refresh query without updates"
@@ -1654,20 +1642,18 @@ mod tests {
                 visible_hint_labels(editor, cx),
             );
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, final_allowed_hint_kinds);
             assert_eq!(inlay_cache.version, edits_made);
         });
     }
 
     #[gpui::test]
     async fn test_hint_request_cancellation(cx: &mut gpui::TestAppContext) {
-        let allowed_hint_kinds = HashSet::from_iter([None]);
         init_test(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: true,
-                show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
-                show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
-                show_other_hints: allowed_hint_kinds.contains(&None),
+                show_type_hints: true,
+                show_parameter_hints: true,
+                show_other_hints: true,
             })
         });
 
@@ -1735,7 +1721,6 @@ mod tests {
             );
             assert_eq!(expected_hints, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, 1,
                 "Only one update should be registered in the cache after all cancellations"
@@ -1782,7 +1767,6 @@ mod tests {
             );
             assert_eq!(expected_hints, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, 2,
                 "Should update the cache version once more, for the new change"
@@ -1792,13 +1776,12 @@ mod tests {
 
     #[gpui::test]
     async fn test_large_buffer_inlay_requests_split(cx: &mut gpui::TestAppContext) {
-        let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
         init_test(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: true,
-                show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
-                show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
-                show_other_hints: allowed_hint_kinds.contains(&None),
+                show_type_hints: true,
+                show_parameter_hints: true,
+                show_other_hints: true,
             })
         });
 
@@ -1904,7 +1887,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(
                 inlay_cache.version, 2,
                 "Both LSP queries should've bumped the cache version"
@@ -1937,7 +1919,6 @@ mod tests {
                 "Should have hints from the new LSP response after edit");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 5, "Should update the cache for every LSP response with hints added");
         });
     }
@@ -1947,13 +1928,12 @@ mod tests {
         deterministic: Arc<Deterministic>,
         cx: &mut gpui::TestAppContext,
     ) {
-        let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]);
         init_test(cx, |settings| {
             settings.defaults.inlay_hints = Some(InlayHintSettings {
                 enabled: true,
-                show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)),
-                show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)),
-                show_other_hints: allowed_hint_kinds.contains(&None),
+                show_type_hints: true,
+                show_parameter_hints: true,
+                show_other_hints: true,
             })
         });
 
@@ -2159,7 +2139,6 @@ mod tests {
             );
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 4, "Every visible excerpt hints should bump the verison");
         });
 
@@ -2191,7 +2170,6 @@ mod tests {
                 "With more scrolls of the multibuffer, more hints should be added into the cache and nothing invalidated without edits");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 9);
         });
 
@@ -2220,7 +2198,6 @@ mod tests {
                 "After multibuffer was scrolled to the end, all hints for all excerpts should be fetched");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 12);
         });
 
@@ -2249,7 +2226,6 @@ mod tests {
                 "After multibuffer was scrolled to the end, further scrolls up should not bring more hints");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 12, "No updates should happen during scrolling already scolled buffer");
         });
 
@@ -2276,7 +2252,6 @@ mod tests {
 unedited (2nd) buffer should have the same hint");
             assert_eq!(expected_layers, visible_hint_labels(editor, cx));
             let inlay_cache = editor.inlay_hint_cache();
-            assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds);
             assert_eq!(inlay_cache.version, 16);
         });
     }

crates/project/src/project.rs

@@ -777,20 +777,32 @@ impl Project {
         }
 
         let mut language_servers_to_stop = Vec::new();
+        let mut language_servers_to_restart = Vec::new();
         let languages = self.languages.to_vec();
+        let project_settings = settings::get::<ProjectSettings>(cx).clone();
         for (worktree_id, started_lsp_name) in self.language_server_ids.keys() {
-            let language = languages.iter().find(|l| {
-                l.lsp_adapters()
+            let language = languages.iter().find_map(|l| {
+                let adapter = l
+                    .lsp_adapters()
                     .iter()
-                    .any(|adapter| &adapter.name == started_lsp_name)
+                    .find(|adapter| &adapter.name == started_lsp_name)?;
+                Some((l, adapter))
             });
-            if let Some(language) = language {
+            if let Some((language, adapter)) = language {
                 let worktree = self.worktree_for_id(*worktree_id, cx);
-                let file = worktree.and_then(|tree| {
+                let file = worktree.as_ref().and_then(|tree| {
                     tree.update(cx, |tree, cx| tree.root_file(cx).map(|f| f as _))
                 });
                 if !language_settings(Some(language), file.as_ref(), cx).enable_language_server {
                     language_servers_to_stop.push((*worktree_id, started_lsp_name.clone()));
+                } else if let Some(worktree) = worktree {
+                    let new_lsp_settings = project_settings
+                        .lsp
+                        .get(&adapter.name.0)
+                        .and_then(|s| s.initialization_options.as_ref());
+                    if adapter.initialization_options.as_ref() != new_lsp_settings {
+                        language_servers_to_restart.push((worktree, Arc::clone(language)));
+                    }
                 }
             }
         }
@@ -807,6 +819,11 @@ impl Project {
             self.start_language_servers(&worktree, worktree_path, language, cx);
         }
 
+        // Restart all language servers with changed initialization options.
+        for (worktree, language) in language_servers_to_restart {
+            self.restart_language_servers(worktree, language, cx);
+        }
+
         if !self.copilot_enabled && Copilot::global(cx).is_some() {
             self.copilot_enabled = true;
             for buffer in self.opened_buffers.values() {

crates/workspace/src/workspace.rs

@@ -4809,6 +4809,7 @@ mod tests {
             theme::init((), cx);
             language::init(cx);
             crate::init_settings(cx);
+            Project::init_settings(cx);
         });
     }
 }