From fa54fa80d00bbd27f7ffada9945ecc9014c38b17 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 9 Jun 2025 20:05:33 +0300 Subject: [PATCH] Store pulled diagnostics' result_ids more persistently (#32403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up of https://github.com/zed-industries/zed/pull/19230 `BufferId` can change between file reopens: e.g. open the buffer, close it, go back in history to reopen it — the 2nd one will have a different `BufferId`, but the same `result_ids` semantically. Release Notes: - N/A --- crates/editor/src/editor_tests.rs | 2 +- crates/project/src/lsp_command.rs | 2 +- crates/project/src/lsp_store.rs | 45 +++++++++++++++++++------------ 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 4c4b6809e9..fd4bc37ccf 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -21941,7 +21941,7 @@ async fn test_pulling_diagnostics(cx: &mut TestAppContext) { .expect("created a singleton buffer") .read(cx) .remote_id(); - let buffer_result_id = project.lsp_store().read(cx).result_id(buffer_id); + let buffer_result_id = project.lsp_store().read(cx).result_id(buffer_id, cx); assert_eq!(expected, buffer_result_id); }); }; diff --git a/crates/project/src/lsp_command.rs b/crates/project/src/lsp_command.rs index 5217034ee9..484338cb84 100644 --- a/crates/project/src/lsp_command.rs +++ b/crates/project/src/lsp_command.rs @@ -4050,7 +4050,7 @@ impl LspCommand for GetDocumentDiagnostics { let buffer_id = buffer.update(&mut cx, |buffer, _| buffer.remote_id())?; Ok(Self { previous_result_id: lsp_store - .update(&mut cx, |lsp_store, _| lsp_store.result_id(buffer_id))?, + .update(&mut cx, |lsp_store, cx| lsp_store.result_id(buffer_id, cx))?, }) } diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index a537cc39ef..49663f09f2 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -166,7 +166,7 @@ pub struct LocalLspStore { _subscription: gpui::Subscription, lsp_tree: Entity, registered_buffers: HashMap, - buffer_pull_diagnostics_result_ids: HashMap>, + buffer_pull_diagnostics_result_ids: HashMap>, } impl LocalLspStore { @@ -2295,8 +2295,11 @@ impl LocalLspStore { let set = DiagnosticSet::new(sanitized_diagnostics, &snapshot); buffer.update(cx, |buffer, cx| { - self.buffer_pull_diagnostics_result_ids - .insert(buffer.remote_id(), result_id); + if let Some(abs_path) = File::from_dyn(buffer.file()).map(|f| f.abs_path(cx)) { + self.buffer_pull_diagnostics_result_ids + .insert(abs_path, result_id); + } + buffer.update_diagnostics(server_id, set, cx) }); @@ -3792,8 +3795,16 @@ impl LspStore { } } BufferStoreEvent::BufferDropped(buffer_id) => { + let abs_path = self + .buffer_store + .read(cx) + .get(*buffer_id) + .and_then(|b| File::from_dyn(b.read(cx).file())) + .map(|f| f.abs_path(cx)); if let Some(local) = self.as_local_mut() { - local.buffer_pull_diagnostics_result_ids.remove(buffer_id); + if let Some(abs_path) = abs_path { + local.buffer_pull_diagnostics_result_ids.remove(&abs_path); + } } } _ => {} @@ -5745,7 +5756,7 @@ impl LspStore { ) -> Task>> { let buffer = buffer_handle.read(cx); let buffer_id = buffer.remote_id(); - let result_id = self.result_id(buffer_id); + let result_id = self.result_id(buffer_id, cx); if let Some((client, upstream_project_id)) = self.upstream_client() { let request_task = client.request(proto::MultiLspQuery { @@ -9704,22 +9715,28 @@ impl LspStore { } } - pub fn result_id(&self, buffer_id: BufferId) -> Option { + pub fn result_id(&self, buffer_id: BufferId, cx: &App) -> Option { + let abs_path = self + .buffer_store + .read(cx) + .get(buffer_id) + .and_then(|b| File::from_dyn(b.read(cx).file())) + .map(|f| f.abs_path(cx))?; self.as_local()? .buffer_pull_diagnostics_result_ids - .get(&buffer_id) + .get(&abs_path) .cloned() .flatten() } - pub fn all_result_ids(&self) -> HashMap { + pub fn all_result_ids(&self) -> HashMap { let Some(local) = self.as_local() else { return HashMap::default(); }; local .buffer_pull_diagnostics_result_ids .iter() - .filter_map(|(buffer_id, result_id)| Some((*buffer_id, result_id.clone()?))) + .filter_map(|(file_path, result_id)| Some((file_path.clone(), result_id.clone()?))) .collect() } @@ -9802,17 +9819,11 @@ fn lsp_workspace_diagnostics_refresh( .await; attempts += 1; - let Ok(previous_result_ids) = lsp_store.update(cx, |lsp_store, cx| { + let Ok(previous_result_ids) = lsp_store.update(cx, |lsp_store, _| { lsp_store .all_result_ids() .into_iter() - .filter_map(|(buffer_id, result_id)| { - let buffer = lsp_store - .buffer_store() - .read(cx) - .get_existing(buffer_id) - .ok()?; - let abs_path = buffer.read(cx).file()?.as_local()?.abs_path(cx); + .filter_map(|(abs_path, result_id)| { let uri = file_path_to_lsp_url(&abs_path).ok()?; Some(lsp::PreviousResultId { uri,