diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index df71b2e788..ac59e3eeb1 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -10037,6 +10037,17 @@ impl Editor { if !is_fresh_language { self.registered_buffers.remove(&buffer_id); } + // No event exists for a buffer detaching from a language server: a switch + // to a language with no server emits neither LanguageServerRemoved nor + // LanguageServerBufferRegistered, so the language change itself is the only + // signal that this buffer's server set changed. Run the same LSP data sweep + // as those events do, or hints and colors from the old server stay visible. + if self.project.is_some() { + self.register_buffer(*buffer_id, cx); + self.invalidate_semantic_tokens(Some(*buffer_id)); + self.update_lsp_data(Some(*buffer_id), window, cx); + self.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx); + } jsx_tag_auto_close::refresh_enabled_in_any_buffer(self, multibuffer, cx); cx.emit(EditorEvent::Reparsed(*buffer_id)); self.update_edit_prediction_settings(cx); diff --git a/crates/editor/src/inlays/inlay_hints.rs b/crates/editor/src/inlays/inlay_hints.rs index 05c52d429a..ee42e68e6c 100644 --- a/crates/editor/src/inlays/inlay_hints.rs +++ b/crates/editor/src/inlays/inlay_hints.rs @@ -5034,6 +5034,81 @@ let c = 3;"# .unwrap(); } + #[gpui::test] + async fn test_hints_cleared_when_language_changes_and_no_server_attaches( + cx: &mut gpui::TestAppContext, + ) { + init_test(cx, &|settings| { + settings.defaults.inlay_hints = Some(InlayHintSettingsContent { + show_value_hints: Some(true), + enabled: Some(true), + edit_debounce_ms: Some(0), + scroll_debounce_ms: Some(0), + show_type_hints: Some(true), + show_parameter_hints: Some(true), + show_other_hints: Some(true), + show_background: Some(false), + toggle_on_modifiers_press: None, + }) + }); + + let (_, editor, _fake_server) = prepare_test_objects(cx, |fake_server, file_with_hints| { + fake_server.set_request_handler::( + move |params, _| async move { + assert_eq!( + params.text_document.uri, + lsp::Uri::from_file_path(file_with_hints).unwrap(), + ); + Ok(Some(vec![lsp::InlayHint { + position: lsp::Position::new(0, 1), + label: lsp::InlayHintLabel::String("stale hint".to_string()), + kind: None, + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }])) + }, + ); + }) + .await; + + cx.executor().run_until_parked(); + editor + .update(cx, |editor, _, cx| { + let expected_hints = vec!["stale hint".to_string()]; + assert_eq!(expected_hints, cached_hint_labels(editor, cx)); + assert_eq!(expected_hints, visible_hint_labels(editor, cx)); + }) + .unwrap(); + + let plain_text = Arc::new(Language::new( + LanguageConfig { + name: "Plain Text".into(), + ..LanguageConfig::default() + }, + None, + )); + editor + .update(cx, |editor, _, cx| { + let project = editor.project().unwrap().clone(); + let buffer = editor.buffer().read(cx).as_singleton().unwrap(); + project.update(cx, |project, cx| { + project.set_language_for_buffer(&buffer, plain_text, cx); + }); + }) + .unwrap(); + cx.executor().run_until_parked(); + + editor + .update(cx, |editor, _, cx| { + assert_eq!(Vec::::new(), cached_hint_labels(editor, cx)); + assert_eq!(Vec::::new(), visible_hint_labels(editor, cx)); + }) + .unwrap(); + } + pub(crate) fn init_test(cx: &mut TestAppContext, f: &dyn Fn(&mut AllLanguageSettingsContent)) { cx.update(|cx| { let settings_store = SettingsStore::test(cx); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 99bc93bd15..ff4f393933 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -1418,6 +1418,16 @@ impl LocalLspStore { } } + fn server_pulls_workspace_diagnostics(&self, server_id: LanguageServerId) -> bool { + match self.language_servers.get(&server_id) { + Some(LanguageServerState::Running { + workspace_diagnostics_refresh_tasks, + .. + }) => !workspace_diagnostics_refresh_tasks.is_empty(), + _ => false, + } + } + fn language_servers_for_buffer<'a>( &'a self, buffer: &'a Buffer, @@ -3223,16 +3233,16 @@ impl LocalLspStore { buffer: &Entity, old_file: &File, cx: &mut App, - ) { + ) -> Vec { let old_path = match old_file.as_local() { Some(local) => local.abs_path(cx), - None => return, + None => return Vec::new(), }; let Ok(file_url) = lsp::Uri::from_file_path(old_path.as_path()) else { - return; + return Vec::new(); }; - self.unregister_buffer_from_language_servers(buffer, &file_url, cx); + self.unregister_buffer_from_language_servers(buffer, &file_url, cx) } pub(crate) fn unregister_buffer_from_language_servers( @@ -3240,19 +3250,42 @@ impl LocalLspStore { buffer: &Entity, file_url: &lsp::Uri, cx: &mut App, - ) { + ) -> Vec { + let abs_path = file_url.to_file_path().ok(); buffer.update(cx, |buffer, cx| { - let mut snapshots = self.buffer_snapshots.remove(&buffer.remote_id()); + let buffer_id = buffer.remote_id(); + let mut snapshots = self.buffer_snapshots.remove(&buffer_id); + let mut detached_servers = Vec::new(); for (_, language_server) in self.language_servers_for_buffer(buffer, cx) { + let server_id = language_server.server_id(); if snapshots .as_mut() - .is_some_and(|map| map.remove(&language_server.server_id()).is_some()) + .is_some_and(|map| map.remove(&server_id).is_some()) { language_server.unregister_buffer(file_url.clone()); + detached_servers.push(server_id); } } - }); + + for &server_id in &detached_servers { + buffer.set_completion_triggers(server_id, Default::default(), cx); + if let Some(opened_in_servers) = self.buffers_opened_in_servers.get_mut(&buffer_id) + { + opened_in_servers.remove(&server_id); + } + if let Some(abs_path) = &abs_path + && let Some(result_ids) = + self.buffer_pull_diagnostics_result_ids.get_mut(&server_id) + { + for result_ids in result_ids.values_mut() { + result_ids.remove(abs_path); + } + } + } + + detached_servers + }) } fn buffer_snapshot_for_lsp_version( @@ -5131,46 +5164,27 @@ impl LspStore { ); } - let diagnostic_updates = local + // Closing the buffer does not change which servers are relevant + // to its path, so pushed diagnostics stay: a running server may + // still describe the file. Pulled document diagnostics need an + // open document to be refreshed, so they are dropped, except for + // servers whose workspace pull still owns the path's verdict. + let servers_to_clear = local .language_servers - .iter() - .filter_map(|(server_id, state)| { - let supports_workspace_diagnostics = match state { - LanguageServerState::Running { - workspace_diagnostics_refresh_tasks, - .. - } => !workspace_diagnostics_refresh_tasks.is_empty(), - _ => false, - }; - if supports_workspace_diagnostics { - None - } else { - Some(*server_id) - } - }) - .map(|server_id| DocumentDiagnosticsUpdate { - diagnostics: DocumentDiagnostics { - document_abs_path: buffer_abs_path.clone(), - version: None, - diagnostics: Vec::new(), - }, - result_id: None, - registration_id: None, - server_id, - disk_based_sources: Cow::Borrowed(&[]), + .keys() + .copied() + .filter(|server_id| { + !local.server_pulls_workspace_diagnostics(*server_id) }) .collect::>(); - - lsp_store - .merge_diagnostic_entries( - diagnostic_updates, - |_, diagnostic, _| { - diagnostic.source_kind != DiagnosticSourceKind::Pulled - }, - cx, - ) - .context("Clearing diagnostics for the closed buffer") - .log_err(); + lsp_store.clear_path_diagnostics_for_servers( + buffer_abs_path, + servers_to_clear, + |_, diagnostic, _| { + diagnostic.source_kind != DiagnosticSourceKind::Pulled + }, + cx, + ); } } }) @@ -5232,23 +5246,25 @@ impl LspStore { for buffer in buffer_store.buffers() { if let Some(f) = File::from_dyn(buffer.read(cx).file()).cloned() { + // Detach before the language is unassigned: the + // detachment resolves the buffer's servers through + // its current language, and with the language gone + // it skips didClose and leaks the per-server state + // while still dropping the snapshots. + if let Some(local) = this.as_local_mut() + && local + .registered_buffers + .contains_key(&buffer.read(cx).remote_id()) + && let Some(file_url) = + file_path_to_lsp_url(&f.abs_path(cx)).log_err() + { + local.unregister_buffer_from_language_servers( + &buffer, &file_url, cx, + ); + } buffer.update(cx, |buffer, cx| { buffer.set_language_async(None, cx) }); - if let Some(local) = this.as_local_mut() { - local.reset_buffer(&buffer, &f, cx); - - if local - .registered_buffers - .contains_key(&buffer.read(cx).remote_id()) - && let Some(file_url) = - file_path_to_lsp_url(&f.abs_path(cx)).log_err() - { - local.unregister_buffer_from_language_servers( - &buffer, &file_url, cx, - ); - } - } } } }); @@ -5428,13 +5444,17 @@ impl LspStore { let buffer = buffer_entity.read(cx); let buffer_file = buffer.file().cloned(); let buffer_id = buffer.remote_id(); + let mut detached_servers = Vec::new(); + let mut detached_abs_path = None; if let Some(local_store) = self.as_local_mut() && local_store.registered_buffers.contains_key(&buffer_id) && let Some(abs_path) = File::from_dyn(buffer_file.as_ref()).map(|file| file.abs_path(cx)) && let Some(file_url) = file_path_to_lsp_url(&abs_path).log_err() { - local_store.unregister_buffer_from_language_servers(buffer_entity, &file_url, cx); + detached_servers = + local_store.unregister_buffer_from_language_servers(buffer_entity, &file_url, cx); + detached_abs_path = Some(abs_path); } buffer_entity.update(cx, |buffer, cx| { if buffer @@ -5465,6 +5485,41 @@ impl LspStore { None }; + // A detached server keeps running and may legitimately re-publish for this path, + // since publishes are applied by path with no regard for registration. So only + // drop what it already said: clear the (path, server) diagnostics everywhere, + // exempting servers whose workspace-diagnostics pull owns the verdict, mirroring + // the buffer-close path. Anything a live server still asserts comes back on its + // next publish; a verdict from a server that stopped tracking the file has no + // other way to ever go away. + if let Some(abs_path) = detached_abs_path + && !detached_servers.is_empty() + { + let servers_to_clear = if let Some(local) = self.as_local() { + let reattached_servers = buffer_entity.update(cx, |buffer, cx| { + local.language_server_ids_for_buffer(buffer, cx) + }); + detached_servers + .into_iter() + .filter(|server_id| !reattached_servers.contains(server_id)) + .filter(|server_id| !local.server_pulls_workspace_diagnostics(*server_id)) + .collect::>() + } else { + Vec::new() + }; + if let Some(lsp_data) = self.lsp_data.get_mut(&buffer_id) { + for server_id in &servers_to_clear { + lsp_data.remove_server_data(*server_id); + } + } + self.clear_path_diagnostics_for_servers( + abs_path, + servers_to_clear, + |_, _, _| false, + cx, + ); + } + if settings.prettier.allowed && let Some(prettier_plugins) = prettier_store::prettier_plugins_for_language(&settings) { @@ -9212,6 +9267,20 @@ impl LspStore { changes: &UpdatedEntriesSet, cx: &mut Context, ) { + // A removed path takes its stored diagnostics with it, or reopening a file + // recreated at the same path replays them from the worktree store via + // initialize_buffer, showing verdicts about content that no longer exists. + if let Some(local) = self.as_local_mut() + && let Some(diagnostics_for_tree) = local.diagnostics.get_mut(&worktree_id) + { + for (path, _, _) in changes + .iter() + .filter(|(_, _, change)| *change == PathChange::Removed) + { + diagnostics_for_tree.remove(path); + } + } + let Some(summaries_for_tree) = self.diagnostic_summaries.get_mut(&worktree_id) else { return; }; @@ -9429,6 +9498,38 @@ impl LspStore { Ok(()) } + // A path's diagnostics are dropped per server, through the same merge machinery that + // applies them: buffer sets, worktree diagnostics, summaries, downstream propagation + // and the DiagnosticsUpdated event all follow from one call. + fn clear_path_diagnostics_for_servers( + &mut self, + abs_path: PathBuf, + server_ids: Vec, + retain_diagnostic: impl Fn(&lsp::Uri, &Diagnostic, &App) -> bool + Clone, + cx: &mut Context, + ) { + if server_ids.is_empty() { + return; + } + let diagnostic_updates = server_ids + .into_iter() + .map(|server_id| DocumentDiagnosticsUpdate { + diagnostics: DocumentDiagnostics { + document_abs_path: abs_path.clone(), + version: None, + diagnostics: Vec::new(), + }, + result_id: None, + registration_id: None, + server_id, + disk_based_sources: Cow::Borrowed(&[]), + }) + .collect::>(); + self.merge_diagnostic_entries(diagnostic_updates, retain_diagnostic, cx) + .context("clearing path diagnostics") + .log_err(); + } + pub fn merge_diagnostic_entries<'a>( &mut self, diagnostic_updates: Vec>, diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index ad3a787fe1..06d6c5da20 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -3217,6 +3217,220 @@ async fn test_restarted_server_reporting_invalid_buffer_version(cx: &mut gpui::T assert_eq!(notification.version, 0); } +#[gpui::test] +async fn test_selecting_a_language_clears_the_old_servers_diagnostics( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "const A: i32 = 1;" })) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + language_registry.add(rust_lang()); + language_registry.add(js_lang()); + let mut fake_servers = language_registry.register_fake_lsp("Rust", FakeLspAdapter::default()); + + let (buffer, _handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + + let fake_server = fake_servers.next().await.unwrap(); + fake_server.notify::(lsp::PublishDiagnosticsParams { + uri: lsp::Uri::from_file_path(path!("/dir/a.rs")).unwrap(), + version: None, + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 7)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: lsp::DiagnosticMessage::String("unused constant".to_string()), + ..Default::default() + }], + }); + cx.executor().run_until_parked(); + + let diagnostic_messages = |buffer: &Buffer| { + buffer + .snapshot() + .diagnostics_in_range::<_, usize>(0..buffer.len(), false) + .map(|entry| entry.diagnostic.message.to_string()) + .collect::>() + }; + + buffer.update(cx, |buffer, _| { + assert_eq!(diagnostic_messages(buffer), vec!["unused constant"]); + }); + project.update(cx, |project, cx| { + assert_eq!(project.diagnostic_summary(false, cx).error_count, 1); + }); + + project.update(cx, |project, cx| { + project.set_language_for_buffer(&buffer, js_lang(), cx); + }); + cx.executor().run_until_parked(); + + buffer.update(cx, |buffer, _| { + assert_eq!(diagnostic_messages(buffer), Vec::::new()); + }); + project.update(cx, |project, cx| { + assert_eq!(project.diagnostic_summary(false, cx).error_count, 0); + }); +} + +#[gpui::test] +async fn test_selecting_a_language_clears_diagnostics_when_the_server_keeps_other_buffers( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/dir"), + json!({ "a.rs": "const A: i32 = 1;", "b.rs": "const B: i32 = 2;" }), + ) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + language_registry.add(rust_lang()); + language_registry.add(js_lang()); + let mut fake_servers = language_registry.register_fake_lsp("Rust", FakeLspAdapter::default()); + + let (buffer_a, _handle_a) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + let (buffer_b, _handle_b) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/b.rs"), cx) + }) + .await + .unwrap(); + + let fake_server = fake_servers.next().await.unwrap(); + let mut shutdown_requests = fake_server + .set_request_handler::(|_, _| future::ready(Ok(()))); + for (path, message) in [ + (path!("/dir/a.rs"), "unused constant A"), + (path!("/dir/b.rs"), "unused constant B"), + ] { + fake_server.notify::( + lsp::PublishDiagnosticsParams { + uri: lsp::Uri::from_file_path(path).unwrap(), + version: None, + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(0, 6), lsp::Position::new(0, 7)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: lsp::DiagnosticMessage::String(message.to_string()), + ..Default::default() + }], + }, + ); + } + cx.executor().run_until_parked(); + + let diagnostic_messages = |buffer: &Buffer| { + buffer + .snapshot() + .diagnostics_in_range::<_, usize>(0..buffer.len(), false) + .map(|entry| entry.diagnostic.message.to_string()) + .collect::>() + }; + + buffer_a.update(cx, |buffer, _| { + assert_eq!(diagnostic_messages(buffer), vec!["unused constant A"]); + }); + project.update(cx, |project, cx| { + assert_eq!(project.diagnostic_summary(false, cx).error_count, 2); + }); + + project.update(cx, |project, cx| { + project.set_language_for_buffer(&buffer_a, js_lang(), cx); + }); + cx.executor().run_until_parked(); + + buffer_a.update(cx, |buffer, _| { + assert_eq!(diagnostic_messages(buffer), Vec::::new()); + }); + buffer_b.update(cx, |buffer, _| { + assert_eq!(diagnostic_messages(buffer), vec!["unused constant B"]); + }); + project.update(cx, |project, cx| { + assert_eq!(project.diagnostic_summary(false, cx).error_count, 1); + }); + assert!( + shutdown_requests.next().now_or_never().flatten().is_none(), + "the server still serves b.rs and must not be stopped" + ); +} + +#[gpui::test] +async fn test_registry_reload_detaches_buffers_from_language_servers( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "const A: i32 = 1;" })) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + language_registry.register_test_language(LanguageConfig { + name: "Rust".into(), + matcher: Arc::new(LanguageMatcher { + path_suffixes: vec!["rs".to_string()], + ..LanguageMatcher::default() + }), + ..LanguageConfig::default() + }); + let mut fake_servers = language_registry.register_fake_lsp("Rust", FakeLspAdapter::default()); + + let (_buffer, _handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + cx.executor().run_until_parked(); + + let mut fake_server = fake_servers.next().await.unwrap(); + let open_notification = fake_server + .receive_notification::() + .await; + assert_eq!( + open_notification.text_document.uri, + lsp::Uri::from_file_path(path!("/dir/a.rs")).unwrap() + ); + + language_registry.reload(); + cx.executor().run_until_parked(); + + let close_notification = fake_server + .receive_notification::() + .await; + assert_eq!( + close_notification.text_document.uri, + lsp::Uri::from_file_path(path!("/dir/a.rs")).unwrap() + ); + let reopen_notification = fake_server + .receive_notification::() + .await; + assert_eq!( + reopen_notification.text_document.uri, + lsp::Uri::from_file_path(path!("/dir/a.rs")).unwrap() + ); +} + #[gpui::test] async fn test_cancel_language_server_work(cx: &mut gpui::TestAppContext) { init_test(cx); @@ -4384,6 +4598,66 @@ async fn test_diagnostic_summaries_cleared_on_worktree_entry_removal( }); } +#[gpui::test] +async fn test_stored_diagnostics_not_replayed_after_entry_removal(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "one" })) + .await; + + let project = Project::test(fs.clone(), [Path::new(path!("/dir"))], cx).await; + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); + + lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .update_diagnostic_entries( + LanguageServerId(0), + Path::new(path!("/dir/a.rs")).to_owned(), + None, + None, + vec![DiagnosticEntry::new( + Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 3)), + Diagnostic { + severity: DiagnosticSeverity::ERROR, + is_primary: true, + message: "error in a".into(), + source_kind: DiagnosticSourceKind::Pushed, + ..Diagnostic::default() + }, + )], + cx, + ) + .unwrap(); + }); + + fs.remove_file(path!("/dir/a.rs").as_ref(), Default::default()) + .await + .unwrap(); + cx.executor().run_until_parked(); + + fs.insert_file(path!("/dir/a.rs"), "one".as_bytes().to_vec()) + .await; + cx.executor().run_until_parked(); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + buffer.update(cx, |buffer, _| { + assert_eq!( + buffer + .snapshot() + .diagnostics_in_range::<_, usize>(0..buffer.len(), false) + .map(|entry| entry.diagnostic.message.to_string()) + .collect::>(), + Vec::::new(), + ); + }); +} + #[gpui::test] async fn test_diagnostic_summaries_cleared_on_server_restart(cx: &mut gpui::TestAppContext) { init_test(cx);