From 9999c31859210654dd572d54dfa42b67c00b33b0 Mon Sep 17 00:00:00 2001
From: Kirill Bulatov <kirill@zed.dev>
Date: Tue, 26 Nov 2024 14:29:54 +0200
Subject: [PATCH] Avoid endless loop of the diagnostic updates (#21209)

Follow-up of https://github.com/zed-industries/zed/pull/21173

Rust-analyzer with `checkOnSave` enabled will push diagnostics for a
file after each diagnostics refresh (e.g. save, file open, file close).

If there's a file that is not open in any pane and has only warnings,
and the diagnostics editor has warnings toggled off, then

0. rust-analyzer will push the corresponding warnings after initial load
1. the diagnostics editor code registers
`project::Event::DiagnosticsUpdated` for the corresponding file path and
opens the corresponding buffer to read its associated diagnostics from
the snapshot
2. opening the buffer would send `textDocument/didOpen` which would
trigger rust-analyzer to push the same diagnostics
3. meanwhile, the diagnostics editor would filter out all diagnostics
for that buffer, dropping the open buffer and effectively closing it
4. closing the buffer will send `textDocument/didClose` which would
trigger rust-analyzer to push the same diagnostics again, as those are
`cargo check` ones, still present in the file
5. GOTO 1

Release Notes:

- Fixed diagnostics editor not scrolling properly under certain
conditions
---
 crates/diagnostics/src/diagnostics.rs | 43 ++++++++++++++++++---------
 crates/project/src/lsp_store.rs       | 15 ++++++++++
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs
index be8da5c1302ee36756945408677e5d76b9b854a2..6db831c1ffddad32d6334aa50b3bba26620b515c 100644
--- a/crates/diagnostics/src/diagnostics.rs
+++ b/crates/diagnostics/src/diagnostics.rs
@@ -134,16 +134,27 @@ impl ProjectDiagnosticsEditor {
                     language_server_id,
                     path,
                 } => {
-                    this.paths_to_update
-                        .insert((path.clone(), Some(*language_server_id)));
-                    this.summary = project.read(cx).diagnostic_summary(false, cx);
-                    cx.emit(EditorEvent::TitleChanged);
-
-                    if this.editor.focus_handle(cx).contains_focused(cx) || this.focus_handle.contains_focused(cx) {
-                        log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. recording change");
+                    let max_severity = this.max_severity();
+                    let has_diagnostics_to_display = project.read(cx).lsp_store().read(cx).diagnostics_for_buffer(path)
+                        .into_iter().flatten()
+                        .filter(|(server_id, _)| language_server_id == server_id)
+                        .flat_map(|(_, diagnostics)| diagnostics)
+                        .any(|diagnostic| diagnostic.diagnostic.severity <= max_severity);
+
+                    if has_diagnostics_to_display {
+                        this.paths_to_update
+                            .insert((path.clone(), Some(*language_server_id)));
+                        this.summary = project.read(cx).diagnostic_summary(false, cx);
+                        cx.emit(EditorEvent::TitleChanged);
+
+                        if this.editor.focus_handle(cx).contains_focused(cx) || this.focus_handle.contains_focused(cx) {
+                            log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. recording change");
+                        } else {
+                            log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. updating excerpts");
+                            this.update_stale_excerpts(cx);
+                        }
                     } else {
-                        log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. updating excerpts");
-                        this.update_stale_excerpts(cx);
+                        log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. no diagnostics to display");
                     }
                 }
                 _ => {}
@@ -329,16 +340,12 @@ impl ProjectDiagnosticsEditor {
             ExcerptId::min()
         };
 
+        let max_severity = self.max_severity();
         let path_state = &mut self.path_states[path_ix];
         let mut new_group_ixs = Vec::new();
         let mut blocks_to_add = Vec::new();
         let mut blocks_to_remove = HashSet::default();
         let mut first_excerpt_id = None;
-        let max_severity = if self.include_warnings {
-            DiagnosticSeverity::WARNING
-        } else {
-            DiagnosticSeverity::ERROR
-        };
         let excerpts_snapshot = self.excerpts.update(cx, |excerpts, cx| {
             let mut old_groups = mem::take(&mut path_state.diagnostic_groups)
                 .into_iter()
@@ -627,6 +634,14 @@ impl ProjectDiagnosticsEditor {
             prev_path = Some(path);
         }
     }
+
+    fn max_severity(&self) -> DiagnosticSeverity {
+        if self.include_warnings {
+            DiagnosticSeverity::WARNING
+        } else {
+            DiagnosticSeverity::ERROR
+        }
+    }
 }
 
 impl FocusableView for ProjectDiagnosticsEditor {
diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs
index cc326285cb84952e0d0863ad959f9a010cdc1c1e..29a0afcfe59438c8bfee3a1c9888c6a456f62486 100644
--- a/crates/project/src/lsp_store.rs
+++ b/crates/project/src/lsp_store.rs
@@ -2919,6 +2919,21 @@ impl LspStore {
             })
     }
 
+    pub fn diagnostics_for_buffer(
+        &self,
+        path: &ProjectPath,
+    ) -> Option<
+        &[(
+            LanguageServerId,
+            Vec<DiagnosticEntry<Unclipped<PointUtf16>>>,
+        )],
+    > {
+        self.diagnostics
+            .get(&path.worktree_id)?
+            .get(&path.path)
+            .map(|diagnostics| diagnostics.as_slice())
+    }
+
     pub fn started_language_servers(&self) -> Vec<(WorktreeId, LanguageServerName)> {
         self.language_server_ids.keys().cloned().collect()
     }