From f5c924c583e037aa74cf19e0d530c08b4a8f4053 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Fri, 9 Jan 2026 18:21:48 -0500 Subject: [PATCH] Revert "lsp: Do not reuse disk-based diagnostics (#46437)" (#46487) This reverts commit dd43891521484dc25d520e3c1551d9d211a1f2fa. This commit was causing issue with diagnostics: https://zed-industries.slack.com/archives/C07NUKHLVUZ/p1767999287421729 Release Notes: - Reverted #46437 --- crates/project/src/lsp_store.rs | 20 +------ crates/project/src/project_tests.rs | 91 ----------------------------- 2 files changed, 1 insertion(+), 110 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index d007bd7b21881a47637f5bfbfa8ac510e3ee1eb3..00d26652c2b0d9ef5aaab1e70f6c519365588537 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -249,21 +249,6 @@ pub struct DocumentDiagnostics { version: Option, } -#[cfg(test)] -impl DocumentDiagnostics { - pub fn new( - diagnostics: Vec>>, - document_abs_path: PathBuf, - version: Option, - ) -> Self { - Self { - diagnostics, - document_abs_path, - version, - } - } -} - #[derive(Default, Debug)] struct DynamicRegistrations { did_change_watched_files: HashMap>, @@ -2475,10 +2460,7 @@ impl LocalLspStore { for (new_diagnostic, entry) in diagnostics { let start; let end; - if entry.diagnostic.is_disk_based { - if !new_diagnostic { - continue; - } + if new_diagnostic && entry.diagnostic.is_disk_based { // Some diagnostics are based on files on disk instead of buffers' // current contents. Adjust these diagnostics' ranges to reflect // any unsaved edits. diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index a7a1b32317a0787fab22a9507c364e85d07387f0..af459c29f1dde1e711ea2e18873a62497f065784 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -3,7 +3,6 @@ use crate::{ Event, git_store::{GitStoreEvent, RepositoryEvent, StatusEntry, pending_op}, - lsp_store::{DocumentDiagnostics, DocumentDiagnosticsUpdate}, task_inventory::TaskContexts, task_store::TaskSettingsLocation, *, @@ -2879,96 +2878,6 @@ async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppC }); } -#[gpui::test] -async fn test_disk_based_diagnostics_not_reused(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree(path!("/dir"), json!({ "a.rs": "fn a() { A }" })) - .await; - - let project = Project::test(fs, [Path::new(path!("/dir"))], cx).await; - let lsp_store = project.read_with(cx, |project, _| project.lsp_store.clone()); - let buffer = project - .update(cx, |project, cx| { - project.open_local_buffer(path!("/dir/a.rs"), cx) - }) - .await - .unwrap(); - - // Setup an initial disk-based diagnostic, which will later become old, so - // we can later assert that it is ignored when merging diagnostic entries. - let diagnostic = DiagnosticEntry { - range: Unclipped(PointUtf16::new(0, 9))..Unclipped(PointUtf16::new(0, 10)), - diagnostic: Diagnostic { - is_disk_based: true, - ..Diagnostic::default() - }, - }; - let diagnostic_update = DocumentDiagnosticsUpdate { - diagnostics: DocumentDiagnostics::new( - vec![diagnostic], - PathBuf::from(path!("/dir/a.rs")), - None, - ), - result_id: None, - server_id: LanguageServerId(0), - disk_based_sources: Cow::Borrowed(&[]), - registration_id: None, - }; - lsp_store.update(cx, |lsp_store, cx| { - lsp_store - .merge_diagnostic_entries(vec![diagnostic_update], |_, _, _| false, cx) - .unwrap(); - }); - - // Quick sanity check, ensure that the initial diagnostic is part of the - // buffer's diagnostics. - buffer.update(cx, |buffer, _| { - let snapshot = buffer.snapshot(); - let diagnostics: Vec<_> = snapshot - .diagnostics_in_range::<_, Point>(0..buffer.len(), false) - .collect(); - - assert_eq!(diagnostics.len(), 1); - }); - - // We'll now merge the diagnostic entries with a new diagnostic update, with - // no diagnostics. This time around, the `merge` closure will return `true` - // to ensure that old diagnostics are retained, ensuring that the first - // diagnostic does get added to the full list of diagnostics, even though - // it'll later be ignored. - let diagnostic_update = lsp_store::DocumentDiagnosticsUpdate { - diagnostics: lsp_store::DocumentDiagnostics::new( - vec![], - PathBuf::from(path!("/dir/a.rs")), - None, - ), - result_id: None, - server_id: LanguageServerId(0), - disk_based_sources: Cow::Borrowed(&[]), - registration_id: None, - }; - lsp_store.update(cx, |lsp_store, cx| { - lsp_store - .merge_diagnostic_entries(vec![diagnostic_update], |_, _, _| true, cx) - .unwrap(); - }); - - // We can now assert that the initial, disk-based diagnostic has been - // removed from the buffer's diagnostics, even if the `merge` closure - // returned `true`, informing that the old diagnostic could be reused. - // The old disk-based diagnostic should be gone, NOT retained - buffer.update(cx, |buffer, _| { - let snapshot = buffer.snapshot(); - let diagnostics: Vec<_> = snapshot - .diagnostics_in_range::<_, Point>(0..buffer.len(), false) - .collect(); - - assert_eq!(diagnostics.len(), 0); - }); -} - #[gpui::test] async fn test_edits_from_lsp2_with_past_version(cx: &mut gpui::TestAppContext) { init_test(cx);