From 45ca651f43f5d295790ee3bc241bdd0d87567e1e Mon Sep 17 00:00:00 2001 From: Arthur Jean <128491831+ArthurDEV44@users.noreply.github.com> Date: Mon, 23 Mar 2026 14:45:42 +0100 Subject: [PATCH] Fix stale diagnostic error markers in file tree (#49333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #48289 Closes https://github.com/zed-industries/zed/issues/52021 Diagnostic error markers (red/yellow dots) in the project panel file tree persisted after errors were resolved. Three root causes: - **WorktreeUpdatedEntries ignored** — when files changed on disk (e.g. `yarn install`), stale diagnostic summaries were never cleared. Added `invalidate_diagnostic_summaries_for_updated_entries()` to clear summaries for `Removed`/`Updated`/`AddedOrUpdated` paths. - **Missing DiagnosticsUpdated emission on server stop** — `stop_local_language_server()` cleared summaries and sent proto messages but never emitted `LspStoreEvent::DiagnosticsUpdated`, so the project panel never refreshed. - **Buffer reload not handled** — reloading a buffer from disk did not clear stale summaries. Added `BufferEvent::Reloaded` handler. All three paths also send zeroed `UpdateDiagnosticSummary` proto messages to downstream collab clients. ## Test plan - [x] `./script/clippy` passes - [x] `cargo test -p project -p project_panel -p worktree` passes (319 tests, 0 failures) - [x] 4 new tests added: - `test_diagnostic_summaries_cleared_on_worktree_entry_removal` - `test_diagnostic_summaries_cleared_on_worktree_entry_update` - `test_diagnostic_summaries_cleared_on_server_restart` - `test_diagnostic_summaries_cleared_on_buffer_reload` - [x] Manual testing: error markers clear when files change on disk - [x] Manual testing: error markers clear on LSP restart Release Notes: - Fixed stale diagnostic data persisting after file reloads, server restarts and FS entry removals --- crates/project/src/lsp_store.rs | 83 +++++- .../tests/integration/project_tests.rs | 262 +++++++++++++++++- 2 files changed, 343 insertions(+), 2 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 41190a32f2d8ff3281240d16e96e35452e390561..da09681e221d9e8d25365218f2e064237ddd92b1 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -3918,6 +3918,7 @@ pub struct LspStore { pub lsp_server_capabilities: HashMap, semantic_token_config: SemanticTokenConfig, lsp_data: HashMap, + buffer_reload_tasks: HashMap>>, next_hint_id: Arc, } @@ -4245,6 +4246,7 @@ impl LspStore { lsp_server_capabilities: HashMap::default(), semantic_token_config: SemanticTokenConfig::new(cx), lsp_data: HashMap::default(), + buffer_reload_tasks: HashMap::default(), next_hint_id: Arc::default(), active_entry: None, _maintain_workspace_config, @@ -4307,6 +4309,7 @@ impl LspStore { semantic_token_config: SemanticTokenConfig::new(cx), next_hint_id: Arc::default(), lsp_data: HashMap::default(), + buffer_reload_tasks: HashMap::default(), active_entry: None, _maintain_workspace_config, @@ -4372,9 +4375,11 @@ impl LspStore { WorktreeStoreEvent::WorktreeUpdateSent(worktree) => { worktree.update(cx, |worktree, _cx| self.send_diagnostic_summaries(worktree)); } + WorktreeStoreEvent::WorktreeUpdatedEntries(worktree_id, changes) => { + self.invalidate_diagnostic_summaries_for_removed_entries(*worktree_id, changes, cx); + } WorktreeStoreEvent::WorktreeReleased(..) | WorktreeStoreEvent::WorktreeOrderChanged - | WorktreeStoreEvent::WorktreeUpdatedEntries(..) | WorktreeStoreEvent::WorktreeUpdatedGitRepositories(..) | WorktreeStoreEvent::WorktreeDeletedEntry(..) => {} } @@ -4439,6 +4444,10 @@ impl LspStore { self.on_buffer_saved(buffer, cx); } + language::BufferEvent::Reloaded => { + self.on_buffer_reloaded(buffer, cx); + } + _ => {} } } @@ -4545,6 +4554,7 @@ impl LspStore { }; if refcount == 0 { lsp_store.lsp_data.remove(&buffer_id); + lsp_store.buffer_reload_tasks.remove(&buffer_id); let local = lsp_store.as_local_mut().unwrap(); local.registered_buffers.remove(&buffer_id); @@ -7954,6 +7964,12 @@ impl LspStore { None } + fn on_buffer_reloaded(&mut self, buffer: Entity, cx: &mut Context) { + let buffer_id = buffer.read(cx).remote_id(); + let task = self.pull_diagnostics_for_buffer(buffer, cx); + self.buffer_reload_tasks.insert(buffer_id, task); + } + async fn refresh_workspace_configurations(lsp_store: &WeakEntity, cx: &mut AsyncApp) { maybe!(async move { let mut refreshed_servers = HashSet::default(); @@ -8122,6 +8138,60 @@ impl LspStore { } } + fn invalidate_diagnostic_summaries_for_removed_entries( + &mut self, + worktree_id: WorktreeId, + changes: &UpdatedEntriesSet, + cx: &mut Context, + ) { + let Some(summaries_for_tree) = self.diagnostic_summaries.get_mut(&worktree_id) else { + return; + }; + + let mut cleared_paths: Vec = Vec::new(); + let mut cleared_server_ids: HashSet = HashSet::default(); + let downstream = self.downstream_client.clone(); + + for (path, _, _) in changes + .iter() + .filter(|(_, _, change)| *change == PathChange::Removed) + { + if let Some(summaries_by_server_id) = summaries_for_tree.remove(path) { + for (server_id, _) in &summaries_by_server_id { + cleared_server_ids.insert(*server_id); + if let Some((client, project_id)) = &downstream { + client + .send(proto::UpdateDiagnosticSummary { + project_id: *project_id, + worktree_id: worktree_id.to_proto(), + summary: Some(proto::DiagnosticSummary { + path: path.as_ref().to_proto(), + language_server_id: server_id.0 as u64, + error_count: 0, + warning_count: 0, + }), + more_summaries: Vec::new(), + }) + .ok(); + } + } + cleared_paths.push(ProjectPath { + worktree_id, + path: path.clone(), + }); + } + } + + if !cleared_paths.is_empty() { + for server_id in cleared_server_ids { + cx.emit(LspStoreEvent::DiagnosticsUpdated { + server_id, + paths: cleared_paths.clone(), + }); + } + } + } + pub fn shared( &mut self, project_id: u64, @@ -10783,6 +10853,7 @@ impl LspStore { } }); + let mut cleared_paths: Vec = Vec::new(); for (worktree_id, summaries) in self.diagnostic_summaries.iter_mut() { summaries.retain(|path, summaries_by_server_id| { if summaries_by_server_id.remove(&server_id).is_some() { @@ -10801,12 +10872,22 @@ impl LspStore { }) .log_err(); } + cleared_paths.push(ProjectPath { + worktree_id: *worktree_id, + path: path.clone(), + }); !summaries_by_server_id.is_empty() } else { true } }); } + if !cleared_paths.is_empty() { + cx.emit(LspStoreEvent::DiagnosticsUpdated { + server_id, + paths: cleared_paths, + }); + } let local = self.as_local_mut().unwrap(); for diagnostics in local.diagnostics.values_mut() { diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index e8cf950dd34af09fa432a6c96553db389ba2ff1c..d218e015c3454d4eb769512e12cd7fbba5d8ffc5 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -76,7 +76,7 @@ use std::{ path::{Path, PathBuf}, rc::Rc, str::FromStr, - sync::{Arc, OnceLock}, + sync::{Arc, OnceLock, atomic}, task::Poll, time::Duration, }; @@ -3758,6 +3758,266 @@ async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppC }); } +#[gpui::test] +async fn test_diagnostic_summaries_cleared_on_worktree_entry_removal( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "one", "b.rs": "two" })) + .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 { + range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 3)), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::ERROR, + is_primary: true, + message: "error in a".to_string(), + source_kind: DiagnosticSourceKind::Pushed, + ..Diagnostic::default() + }, + }], + cx, + ) + .unwrap(); + lsp_store + .update_diagnostic_entries( + LanguageServerId(0), + Path::new(path!("/dir/b.rs")).to_owned(), + None, + None, + vec![DiagnosticEntry { + range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 3)), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::WARNING, + is_primary: true, + message: "warning in b".to_string(), + source_kind: DiagnosticSourceKind::Pushed, + ..Diagnostic::default() + }, + }], + cx, + ) + .unwrap(); + + assert_eq!( + lsp_store.diagnostic_summary(false, cx), + DiagnosticSummary { + error_count: 1, + warning_count: 1, + } + ); + }); + + fs.remove_file(path!("/dir/a.rs").as_ref(), Default::default()) + .await + .unwrap(); + cx.executor().run_until_parked(); + + lsp_store.update(cx, |lsp_store, cx| { + assert_eq!( + lsp_store.diagnostic_summary(false, cx), + DiagnosticSummary { + error_count: 0, + warning_count: 1, + }, + ); + }); +} + +#[gpui::test] +async fn test_diagnostic_summaries_cleared_on_server_restart(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "x" })).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()); + 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: Uri::from_file_path(path!("/dir/a.rs")).unwrap(), + version: None, + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 1)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: "error before restart".to_string(), + ..Default::default() + }], + }); + cx.executor().run_until_parked(); + + project.update(cx, |project, cx| { + assert_eq!( + project.diagnostic_summary(false, cx), + DiagnosticSummary { + error_count: 1, + warning_count: 0, + } + ); + }); + + let mut events = cx.events(&project); + + project.update(cx, |project, cx| { + project.restart_language_servers_for_buffers(vec![buffer.clone()], HashSet::default(), cx); + }); + cx.executor().run_until_parked(); + + let mut received_diagnostics_updated = false; + while let Some(Some(event)) = + futures::FutureExt::now_or_never(futures::StreamExt::next(&mut events)) + { + if matches!(event, Event::DiagnosticsUpdated { .. }) { + received_diagnostics_updated = true; + } + } + assert!( + received_diagnostics_updated, + "DiagnosticsUpdated event should be emitted when a language server is stopped" + ); + + project.update(cx, |project, cx| { + assert_eq!( + project.diagnostic_summary(false, cx), + DiagnosticSummary { + error_count: 0, + warning_count: 0, + } + ); + }); +} + +#[gpui::test] +async fn test_diagnostic_summaries_cleared_on_buffer_reload(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": "one two three" })) + .await; + + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + let pull_count = Arc::new(atomic::AtomicUsize::new(0)); + let closure_pull_count = pull_count.clone(); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + diagnostic_provider: Some(lsp::DiagnosticServerCapabilities::Options( + lsp::DiagnosticOptions { + identifier: Some("test-reload".to_string()), + inter_file_dependencies: true, + workspace_diagnostics: false, + work_done_progress_options: Default::default(), + }, + )), + ..lsp::ServerCapabilities::default() + }, + initializer: Some(Box::new(move |fake_server| { + let pull_count = closure_pull_count.clone(); + fake_server.set_request_handler::( + move |_, _| { + let pull_count = pull_count.clone(); + async move { + pull_count.fetch_add(1, atomic::Ordering::SeqCst); + Ok(lsp::DocumentDiagnosticReportResult::Report( + lsp::DocumentDiagnosticReport::Full( + lsp::RelatedFullDocumentDiagnosticReport { + related_documents: None, + full_document_diagnostic_report: + lsp::FullDocumentDiagnosticReport { + result_id: None, + items: Vec::new(), + }, + }, + ), + )) + } + }, + ); + })), + ..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(); + cx.executor().run_until_parked(); + + // Publish initial diagnostics via the fake server. + fake_server.notify::(lsp::PublishDiagnosticsParams { + uri: Uri::from_file_path(path!("/dir/a.rs")).unwrap(), + version: None, + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 3)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: "error in a".to_string(), + ..Default::default() + }], + }); + cx.executor().run_until_parked(); + + project.update(cx, |project, cx| { + assert_eq!( + project.diagnostic_summary(false, cx), + DiagnosticSummary { + error_count: 1, + warning_count: 0, + } + ); + }); + + let pulls_before = pull_count.load(atomic::Ordering::SeqCst); + + // Change the file on disk. The FS event triggers buffer reload, + // which in turn triggers pull_diagnostics_for_buffer. + fs.save( + path!("/dir/a.rs").as_ref(), + &"fixed content".into(), + LineEnding::Unix, + ) + .await + .unwrap(); + cx.executor().run_until_parked(); + + let pulls_after = pull_count.load(atomic::Ordering::SeqCst); + assert!( + pulls_after > pulls_before, + "Expected document diagnostic pull after buffer reload (before={pulls_before}, after={pulls_after})" + ); +} + #[gpui::test] async fn test_edits_from_lsp2_with_past_version(cx: &mut gpui::TestAppContext) { init_test(cx);