Merge pull request #860 from zed-industries/fix-more-project-diagnostics-cycles

Antonio Scandurra created 3 years ago

Don't emit event when LSP reports consecutive empty diagnostics

Change summary

crates/project/src/project.rs  | 33 ++++++++++++++++++++++---
crates/project/src/worktree.rs | 46 +++++++++++++++++++----------------
2 files changed, 54 insertions(+), 25 deletions(-)

Detailed changes

crates/project/src/project.rs

@@ -154,7 +154,7 @@ pub struct ProjectPath {
     pub path: Arc<Path>,
 }
 
-#[derive(Clone, Debug, Default, PartialEq, Serialize)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Serialize)]
 pub struct DiagnosticSummary {
     pub error_count: usize,
     pub warning_count: usize,
@@ -1961,13 +1961,15 @@ impl Project {
             self.update_buffer_diagnostics(&buffer, diagnostics.clone(), version, cx)?;
         }
 
-        worktree.update(cx, |worktree, cx| {
+        let updated = worktree.update(cx, |worktree, cx| {
             worktree
                 .as_local_mut()
                 .ok_or_else(|| anyhow!("not a local worktree"))?
                 .update_diagnostics(project_path.path.clone(), diagnostics, cx)
         })?;
-        cx.emit(Event::DiagnosticsUpdated(project_path));
+        if updated {
+            cx.emit(Event::DiagnosticsUpdated(project_path));
+        }
         Ok(())
     }
 
@@ -4879,7 +4881,7 @@ mod tests {
     };
     use lsp::Url;
     use serde_json::json;
-    use std::{cell::RefCell, os::unix, path::PathBuf, rc::Rc};
+    use std::{cell::RefCell, os::unix, path::PathBuf, rc::Rc, task::Poll};
     use unindent::Unindent as _;
     use util::{assert_set_eq, test::temp_tree};
     use worktree::WorktreeHandle as _;
@@ -5564,6 +5566,29 @@ mod tests {
                 }]
             )
         });
+
+        // Ensure publishing empty diagnostics twice only results in one update event.
+        fake_server.notify::<lsp::notification::PublishDiagnostics>(
+            lsp::PublishDiagnosticsParams {
+                uri: Url::from_file_path("/dir/a.rs").unwrap(),
+                version: None,
+                diagnostics: Default::default(),
+            },
+        );
+        assert_eq!(
+            events.next().await.unwrap(),
+            Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into())
+        );
+
+        fake_server.notify::<lsp::notification::PublishDiagnostics>(
+            lsp::PublishDiagnosticsParams {
+                uri: Url::from_file_path("/dir/a.rs").unwrap(),
+                version: None,
+                diagnostics: Default::default(),
+            },
+        );
+        cx.foreground().run_until_parked();
+        assert_eq!(futures::poll!(events.next()), Poll::Pending);
     }
 
     #[gpui::test]

crates/project/src/worktree.rs

@@ -564,33 +564,37 @@ impl LocalWorktree {
         worktree_path: Arc<Path>,
         diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
         _: &mut ModelContext<Worktree>,
-    ) -> Result<()> {
-        let summary = DiagnosticSummary::new(&diagnostics);
-        if summary.is_empty() {
+    ) -> Result<bool> {
+        self.diagnostics.remove(&worktree_path);
+        let old_summary = self
+            .diagnostic_summaries
+            .remove(&PathKey(worktree_path.clone()))
+            .unwrap_or_default();
+        let new_summary = DiagnosticSummary::new(&diagnostics);
+        if !new_summary.is_empty() {
             self.diagnostic_summaries
-                .remove(&PathKey(worktree_path.clone()));
-            self.diagnostics.remove(&worktree_path);
-        } else {
-            self.diagnostic_summaries
-                .insert(PathKey(worktree_path.clone()), summary.clone());
+                .insert(PathKey(worktree_path.clone()), new_summary);
             self.diagnostics.insert(worktree_path.clone(), diagnostics);
         }
 
-        if let Some(share) = self.share.as_ref() {
-            self.client
-                .send(proto::UpdateDiagnosticSummary {
-                    project_id: share.project_id,
-                    worktree_id: self.id().to_proto(),
-                    summary: Some(proto::DiagnosticSummary {
-                        path: worktree_path.to_string_lossy().to_string(),
-                        error_count: summary.error_count as u32,
-                        warning_count: summary.warning_count as u32,
-                    }),
-                })
-                .log_err();
+        let updated = !old_summary.is_empty() || !new_summary.is_empty();
+        if updated {
+            if let Some(share) = self.share.as_ref() {
+                self.client
+                    .send(proto::UpdateDiagnosticSummary {
+                        project_id: share.project_id,
+                        worktree_id: self.id().to_proto(),
+                        summary: Some(proto::DiagnosticSummary {
+                            path: worktree_path.to_string_lossy().to_string(),
+                            error_count: new_summary.error_count as u32,
+                            warning_count: new_summary.warning_count as u32,
+                        }),
+                    })
+                    .log_err();
+            }
         }
 
-        Ok(())
+        Ok(updated)
     }
 
     pub fn scan_complete(&self) -> impl Future<Output = ()> {