Avoid refreshing diagnostics for language servers that didn't update

Antonio Scandurra and Max Brunsfeld created

Co-Authored-By: Max Brunsfeld <max@zed.dev>

Change summary

crates/diagnostics/src/diagnostics.rs |  51 +++++++---
crates/diagnostics/src/items.rs       |   2 
crates/project/src/project.rs         | 142 ++++++++++++++++++----------
crates/project/src/worktree.rs        |   6 +
crates/rpc/proto/zed.proto            |   5 
crates/rpc/src/rpc.rs                 |   2 
6 files changed, 135 insertions(+), 73 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -1,7 +1,7 @@
 pub mod items;
 
 use anyhow::Result;
-use collections::{BTreeSet, HashSet};
+use collections::{BTreeMap, HashSet};
 use editor::{
     diagnostic_block_renderer,
     display_map::{BlockDisposition, BlockId, BlockProperties, RenderBlock},
@@ -23,7 +23,6 @@ use smallvec::SmallVec;
 use std::{
     any::{Any, TypeId},
     cmp::Ordering,
-    mem,
     ops::Range,
     path::PathBuf,
     sync::Arc,
@@ -52,7 +51,7 @@ struct ProjectDiagnosticsEditor {
     summary: DiagnosticSummary,
     excerpts: ModelHandle<MultiBuffer>,
     path_states: Vec<PathState>,
-    paths_to_update: BTreeSet<ProjectPath>,
+    paths_to_update: BTreeMap<ProjectPath, usize>,
 }
 
 struct PathState {
@@ -114,8 +113,8 @@ impl View for ProjectDiagnosticsEditor {
                 "summary": project.diagnostic_summary(cx),
             }),
             "summary": self.summary,
-            "paths_to_update": self.paths_to_update.iter().map(|path|
-                path.path.to_string_lossy()
+            "paths_to_update": self.paths_to_update.iter().map(|(path, server_id)|
+                (path.path.to_string_lossy(), server_id)
             ).collect::<Vec<_>>(),
             "paths_states": self.path_states.iter().map(|state|
                 json!({
@@ -139,12 +138,16 @@ impl ProjectDiagnosticsEditor {
         cx: &mut ViewContext<Self>,
     ) -> Self {
         cx.subscribe(&project_handle, |this, _, event, cx| match event {
-            project::Event::DiskBasedDiagnosticsFinished => {
-                this.update_excerpts(cx);
+            project::Event::DiskBasedDiagnosticsFinished { language_server_id } => {
+                this.update_excerpts(Some(*language_server_id), cx);
                 this.update_title(cx);
             }
-            project::Event::DiagnosticsUpdated(path) => {
-                this.paths_to_update.insert(path.clone());
+            project::Event::DiagnosticsUpdated {
+                language_server_id,
+                path,
+            } => {
+                this.paths_to_update
+                    .insert(path.clone(), *language_server_id);
             }
             _ => {}
         })
@@ -161,7 +164,10 @@ impl ProjectDiagnosticsEditor {
             .detach();
 
         let project = project_handle.read(cx);
-        let paths_to_update = project.diagnostic_summaries(cx).map(|e| e.0).collect();
+        let paths_to_update = project
+            .diagnostic_summaries(cx)
+            .map(|e| (e.0, e.1.language_server_id))
+            .collect();
         let summary = project.diagnostic_summary(cx);
         let mut this = Self {
             project: project_handle,
@@ -172,7 +178,7 @@ impl ProjectDiagnosticsEditor {
             path_states: Default::default(),
             paths_to_update,
         };
-        this.update_excerpts(cx);
+        this.update_excerpts(None, cx);
         this
     }
 
@@ -212,8 +218,18 @@ impl ProjectDiagnosticsEditor {
         .detach()
     }
 
-    fn update_excerpts(&mut self, cx: &mut ViewContext<Self>) {
-        let paths = mem::take(&mut self.paths_to_update);
+    fn update_excerpts(&mut self, language_server_id: Option<usize>, cx: &mut ViewContext<Self>) {
+        let mut paths = Vec::new();
+        self.paths_to_update.retain(|path, server_id| {
+            if language_server_id
+                .map_or(true, |language_server_id| language_server_id == *server_id)
+            {
+                paths.push(path.clone());
+                false
+            } else {
+                true
+            }
+        });
         let project = self.project.clone();
         cx.spawn(|this, mut cx| {
             async move {
@@ -221,7 +237,7 @@ impl ProjectDiagnosticsEditor {
                     let buffer = project
                         .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))
                         .await?;
-                    this.update(&mut cx, |view, cx| view.populate_excerpts(path, buffer, cx))
+                    this.update(&mut cx, |this, cx| this.populate_excerpts(path, buffer, cx))
                 }
                 Result::<_, anyhow::Error>::Ok(())
             }
@@ -838,6 +854,7 @@ mod tests {
         project.update(cx, |project, cx| {
             project
                 .update_diagnostic_entries(
+                    0,
                     PathBuf::from("/test/main.rs"),
                     None,
                     vec![
@@ -989,6 +1006,7 @@ mod tests {
             project.disk_based_diagnostics_started(cx);
             project
                 .update_diagnostic_entries(
+                    0,
                     PathBuf::from("/test/consts.rs"),
                     None,
                     vec![DiagnosticEntry {
@@ -1005,7 +1023,7 @@ mod tests {
                     cx,
                 )
                 .unwrap();
-            project.disk_based_diagnostics_finished(cx);
+            project.disk_based_diagnostics_finished(0, cx);
         });
 
         view.next_notification(&cx).await;
@@ -1088,6 +1106,7 @@ mod tests {
             project.disk_based_diagnostics_started(cx);
             project
                 .update_diagnostic_entries(
+                    0,
                     PathBuf::from("/test/consts.rs"),
                     None,
                     vec![
@@ -1118,7 +1137,7 @@ mod tests {
                     cx,
                 )
                 .unwrap();
-            project.disk_based_diagnostics_finished(cx);
+            project.disk_based_diagnostics_finished(0, cx);
         });
 
         view.next_notification(&cx).await;

crates/diagnostics/src/items.rs 🔗

@@ -30,7 +30,7 @@ impl DiagnosticIndicator {
                 this.check_in_progress = true;
                 cx.notify();
             }
-            project::Event::DiskBasedDiagnosticsFinished => {
+            project::Event::DiskBasedDiagnosticsFinished { .. } => {
                 this.summary = project.read(cx).diagnostic_summary(cx);
                 this.check_in_progress = false;
                 cx.notify();

crates/project/src/project.rs 🔗

@@ -156,8 +156,13 @@ pub enum Event {
     WorktreeRemoved(WorktreeId),
     DiskBasedDiagnosticsStarted,
     DiskBasedDiagnosticsUpdated,
-    DiskBasedDiagnosticsFinished,
-    DiagnosticsUpdated(ProjectPath),
+    DiskBasedDiagnosticsFinished {
+        language_server_id: usize,
+    },
+    DiagnosticsUpdated {
+        path: ProjectPath,
+        language_server_id: usize,
+    },
     RemoteIdChanged(Option<u64>),
     CollaboratorLeft(PeerId),
     ContactRequestedJoin(Arc<User>),
@@ -187,6 +192,7 @@ pub struct ProjectPath {
 
 #[derive(Copy, Clone, Debug, Default, PartialEq, Serialize)]
 pub struct DiagnosticSummary {
+    pub language_server_id: usize,
     pub error_count: usize,
     pub warning_count: usize,
 }
@@ -220,8 +226,12 @@ pub struct Symbol {
 pub struct ProjectTransaction(pub HashMap<ModelHandle<Buffer>, language::Transaction>);
 
 impl DiagnosticSummary {
-    fn new<'a, T: 'a>(diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>) -> Self {
+    fn new<'a, T: 'a>(
+        language_server_id: usize,
+        diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>,
+    ) -> Self {
         let mut this = Self {
+            language_server_id,
             error_count: 0,
             warning_count: 0,
         };
@@ -246,6 +256,7 @@ impl DiagnosticSummary {
     pub fn to_proto(&self, path: &Path) -> proto::DiagnosticSummary {
         proto::DiagnosticSummary {
             path: path.to_string_lossy().to_string(),
+            language_server_id: self.language_server_id as u64,
             error_count: self.error_count as u32,
             warning_count: self.warning_count as u32,
         }
@@ -1742,6 +1753,24 @@ impl Project {
                         )
                         .log_err();
                 }
+
+                // After saving a buffer, simulate disk-based diagnostics being finished for languages
+                // that don't support a disk-based progress token.
+                let (lsp_adapter, language_server) =
+                    self.language_server_for_buffer(buffer.read(cx), cx)?;
+                if lsp_adapter
+                    .disk_based_diagnostics_progress_token()
+                    .is_none()
+                {
+                    let server_id = language_server.server_id();
+                    self.disk_based_diagnostics_finished(server_id, cx);
+                    self.broadcast_language_server_update(
+                        server_id,
+                        proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
+                            proto::LspDiskBasedDiagnosticsUpdated {},
+                        ),
+                    );
+                }
             }
             _ => {}
         }
@@ -1827,11 +1856,7 @@ impl Project {
                                 if let Some(this) = this.upgrade(&cx) {
                                     this.update(&mut cx, |this, cx| {
                                         this.on_lsp_diagnostics_published(
-                                            server_id,
-                                            params,
-                                            &adapter,
-                                            disk_based_diagnostics_progress_token,
-                                            cx,
+                                            server_id, params, &adapter, cx,
                                         );
                                     });
                                 }
@@ -2061,30 +2086,16 @@ impl Project {
         server_id: usize,
         mut params: lsp::PublishDiagnosticsParams,
         adapter: &Arc<dyn LspAdapter>,
-        disk_based_diagnostics_progress_token: Option<&str>,
         cx: &mut ModelContext<Self>,
     ) {
         adapter.process_diagnostics(&mut params);
-        if disk_based_diagnostics_progress_token.is_none() {
-            self.disk_based_diagnostics_started(cx);
-            self.broadcast_language_server_update(
-                server_id,
-                proto::update_language_server::Variant::DiskBasedDiagnosticsUpdating(
-                    proto::LspDiskBasedDiagnosticsUpdating {},
-                ),
-            );
-        }
-        self.update_diagnostics(params, adapter.disk_based_diagnostic_sources(), cx)
-            .log_err();
-        if disk_based_diagnostics_progress_token.is_none() {
-            self.disk_based_diagnostics_finished(cx);
-            self.broadcast_language_server_update(
-                server_id,
-                proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
-                    proto::LspDiskBasedDiagnosticsUpdated {},
-                ),
-            );
-        }
+        self.update_diagnostics(
+            server_id,
+            params,
+            adapter.disk_based_diagnostic_sources(),
+            cx,
+        )
+        .log_err();
     }
 
     fn on_lsp_progress(
@@ -2161,7 +2172,7 @@ impl Project {
                 if Some(token.as_str()) == disk_based_diagnostics_progress_token {
                     language_server_status.pending_diagnostic_updates -= 1;
                     if language_server_status.pending_diagnostic_updates == 0 {
-                        self.disk_based_diagnostics_finished(cx);
+                        self.disk_based_diagnostics_finished(server_id, cx);
                         self.broadcast_language_server_update(
                             server_id,
                             proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(
@@ -2297,6 +2308,7 @@ impl Project {
 
     pub fn update_diagnostics(
         &mut self,
+        language_server_id: usize,
         params: lsp::PublishDiagnosticsParams,
         disk_based_sources: &[&str],
         cx: &mut ModelContext<Self>,
@@ -2401,12 +2413,19 @@ impl Project {
             }
         }
 
-        self.update_diagnostic_entries(abs_path, params.version, diagnostics, cx)?;
+        self.update_diagnostic_entries(
+            language_server_id,
+            abs_path,
+            params.version,
+            diagnostics,
+            cx,
+        )?;
         Ok(())
     }
 
     pub fn update_diagnostic_entries(
         &mut self,
+        language_server_id: usize,
         abs_path: PathBuf,
         version: Option<i32>,
         diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
@@ -2431,10 +2450,18 @@ impl Project {
             worktree
                 .as_local_mut()
                 .ok_or_else(|| anyhow!("not a local worktree"))?
-                .update_diagnostics(project_path.path.clone(), diagnostics, cx)
+                .update_diagnostics(
+                    language_server_id,
+                    project_path.path.clone(),
+                    diagnostics,
+                    cx,
+                )
         })?;
         if updated {
-            cx.emit(Event::DiagnosticsUpdated(project_path));
+            cx.emit(Event::DiagnosticsUpdated {
+                language_server_id,
+                path: project_path,
+            });
         }
         Ok(())
     }
@@ -4010,17 +4037,13 @@ impl Project {
         }
     }
 
-    pub fn disk_based_diagnostics_finished(&mut self, cx: &mut ModelContext<Self>) {
+    pub fn disk_based_diagnostics_finished(
+        &mut self,
+        language_server_id: usize,
+        cx: &mut ModelContext<Self>,
+    ) {
         cx.emit(Event::DiskBasedDiagnosticsUpdated);
-        if self
-            .language_server_statuses
-            .values()
-            .map(|status| status.pending_diagnostic_updates)
-            .sum::<isize>()
-            == 0
-        {
-            cx.emit(Event::DiskBasedDiagnosticsFinished);
-        }
+        cx.emit(Event::DiskBasedDiagnosticsFinished { language_server_id });
     }
 
     pub fn active_entry(&self) -> Option<ProjectEntryId> {
@@ -4351,7 +4374,10 @@ impl Project {
                             .unwrap()
                             .update_diagnostic_summary(project_path.path.clone(), &summary);
                     });
-                    cx.emit(Event::DiagnosticsUpdated(project_path));
+                    cx.emit(Event::DiagnosticsUpdated {
+                        language_server_id: summary.language_server_id as usize,
+                        path: project_path,
+                    });
                 }
             }
             Ok(())
@@ -4424,7 +4450,9 @@ impl Project {
                 })
             }
             proto::update_language_server::Variant::DiskBasedDiagnosticsUpdated(_) => {
-                this.update(&mut cx, |this, cx| this.disk_based_diagnostics_finished(cx));
+                this.update(&mut cx, |this, cx| {
+                    this.disk_based_diagnostics_finished(language_server_id, cx)
+                });
             }
         }
 
@@ -6064,6 +6092,7 @@ mod tests {
         project.update(cx, |project, cx| {
             project
                 .update_diagnostics(
+                    0,
                     lsp::PublishDiagnosticsParams {
                         uri: Url::from_file_path("/dir/a.rs").unwrap(),
                         version: None,
@@ -6083,6 +6112,7 @@ mod tests {
                 .unwrap();
             project
                 .update_diagnostics(
+                    0,
                     lsp::PublishDiagnosticsParams {
                         uri: Url::from_file_path("/dir/b.rs").unwrap(),
                         version: None,
@@ -6199,7 +6229,10 @@ mod tests {
         );
         assert_eq!(
             events.next().await.unwrap(),
-            Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into())
+            Event::DiagnosticsUpdated {
+                language_server_id: 0,
+                path: (worktree_id, Path::new("a.rs")).into()
+            }
         );
 
         fake_server.end_progress(progress_token).await;
@@ -6210,7 +6243,9 @@ mod tests {
         );
         assert_eq!(
             events.next().await.unwrap(),
-            Event::DiskBasedDiagnosticsFinished
+            Event::DiskBasedDiagnosticsFinished {
+                language_server_id: 0
+            }
         );
 
         let buffer = project
@@ -6248,7 +6283,10 @@ mod tests {
         );
         assert_eq!(
             events.next().await.unwrap(),
-            Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into())
+            Event::DiagnosticsUpdated {
+                language_server_id: 0,
+                path: (worktree_id, Path::new("a.rs")).into()
+            }
         );
 
         fake_server.notify::<lsp::notification::PublishDiagnostics>(
@@ -6316,10 +6354,10 @@ mod tests {
             events.next().await.unwrap(),
             Event::DiskBasedDiagnosticsUpdated
         );
-        assert_eq!(
+        assert!(matches!(
             events.next().await.unwrap(),
-            Event::DiskBasedDiagnosticsFinished
-        );
+            Event::DiskBasedDiagnosticsFinished { .. }
+        ));
         project.read_with(cx, |project, _| {
             assert!(!project.is_running_disk_based_diagnostics());
         });
@@ -8002,7 +8040,7 @@ mod tests {
         };
 
         project
-            .update(cx, |p, cx| p.update_diagnostics(message, &[], cx))
+            .update(cx, |p, cx| p.update_diagnostics(0, message, &[], cx))
             .unwrap();
         let buffer = buffer.read_with(cx, |buffer, _| buffer.snapshot());
 

crates/project/src/worktree.rs 🔗

@@ -210,6 +210,7 @@ impl Worktree {
                         (
                             PathKey(PathBuf::from(summary.path).into()),
                             DiagnosticSummary {
+                                language_server_id: summary.language_server_id as usize,
                                 error_count: summary.error_count as usize,
                                 warning_count: summary.warning_count as usize,
                             },
@@ -528,6 +529,7 @@ impl LocalWorktree {
 
     pub fn update_diagnostics(
         &mut self,
+        language_server_id: usize,
         worktree_path: Arc<Path>,
         diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
         _: &mut ModelContext<Worktree>,
@@ -537,7 +539,7 @@ impl LocalWorktree {
             .diagnostic_summaries
             .remove(&PathKey(worktree_path.clone()))
             .unwrap_or_default();
-        let new_summary = DiagnosticSummary::new(&diagnostics);
+        let new_summary = DiagnosticSummary::new(language_server_id, &diagnostics);
         if !new_summary.is_empty() {
             self.diagnostic_summaries
                 .insert(PathKey(worktree_path.clone()), new_summary);
@@ -553,6 +555,7 @@ impl LocalWorktree {
                         worktree_id: self.id().to_proto(),
                         summary: Some(proto::DiagnosticSummary {
                             path: worktree_path.to_string_lossy().to_string(),
+                            language_server_id: language_server_id as u64,
                             error_count: new_summary.error_count as u32,
                             warning_count: new_summary.warning_count as u32,
                         }),
@@ -1065,6 +1068,7 @@ impl RemoteWorktree {
         summary: &proto::DiagnosticSummary,
     ) {
         let summary = DiagnosticSummary {
+            language_server_id: summary.language_server_id as usize,
             error_count: summary.error_count as usize,
             warning_count: summary.warning_count as usize,
         };

crates/rpc/proto/zed.proto 🔗

@@ -516,8 +516,9 @@ message UpdateDiagnosticSummary {
 
 message DiagnosticSummary {
     string path = 1;
-    uint32 error_count = 2;
-    uint32 warning_count = 3;
+    uint64 language_server_id = 2;
+    uint32 error_count = 3;
+    uint32 warning_count = 4;
 }
 
 message UpdateLanguageServer {

crates/rpc/src/rpc.rs 🔗

@@ -6,4 +6,4 @@ pub use conn::Connection;
 pub use peer::*;
 mod macros;
 
-pub const PROTOCOL_VERSION: u32 = 22;
+pub const PROTOCOL_VERSION: u32 = 23;