Avoid language servers fighting over diagnostics summaries

Previously each server would stomp all over the existing results

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

crates/collab/src/tests/integration_tests.rs

@@ -3477,6 +3477,7 @@ async fn test_collaborating_with_diagnostics(
                     worktree_id,
                     path: Arc::from(Path::new("a.rs")),
                 },
+                0,
                 DiagnosticSummary {
                     error_count: 1,
                     warning_count: 0,
@@ -3512,6 +3513,7 @@ async fn test_collaborating_with_diagnostics(
                 worktree_id,
                 path: Arc::from(Path::new("a.rs")),
             },
+            0,
             DiagnosticSummary {
                 error_count: 1,
                 warning_count: 0,
@@ -3552,10 +3554,10 @@ async fn test_collaborating_with_diagnostics(
                     worktree_id,
                     path: Arc::from(Path::new("a.rs")),
                 },
+                0,
                 DiagnosticSummary {
                     error_count: 1,
                     warning_count: 1,
-                    ..Default::default()
                 },
             )]
         );
@@ -3568,10 +3570,10 @@ async fn test_collaborating_with_diagnostics(
                     worktree_id,
                     path: Arc::from(Path::new("a.rs")),
                 },
+                0,
                 DiagnosticSummary {
                     error_count: 1,
                     warning_count: 1,
-                    ..Default::default()
                 },
             )]
         );

crates/diagnostics/src/diagnostics.rs

@@ -168,7 +168,7 @@ impl ProjectDiagnosticsEditor {
         let project = project_handle.read(cx);
         let paths_to_update = project
             .diagnostic_summaries(cx)
-            .map(|e| (e.0, e.1.language_server_id))
+            .map(|(path, server_id, _)| (path, server_id))
             .collect();
         let summary = project.diagnostic_summary(cx);
         let mut this = Self {

crates/project/src/project.rs

@@ -243,7 +243,6 @@ 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,
 }
@@ -314,12 +313,8 @@ pub struct Hover {
 pub struct ProjectTransaction(pub HashMap<ModelHandle<Buffer>, language::Transaction>);
 
 impl DiagnosticSummary {
-    fn new<'a, T: 'a>(
-        language_server_id: usize,
-        diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>,
-    ) -> Self {
+    fn new<'a, T: 'a>(diagnostics: impl IntoIterator<Item = &'a DiagnosticEntry<T>>) -> Self {
         let mut this = Self {
-            language_server_id,
             error_count: 0,
             warning_count: 0,
         };
@@ -341,10 +336,10 @@ impl DiagnosticSummary {
         self.error_count == 0 && self.warning_count == 0
     }
 
-    pub fn to_proto(&self, path: &Path) -> proto::DiagnosticSummary {
+    pub fn to_proto(&self, language_server_id: usize, path: &Path) -> proto::DiagnosticSummary {
         proto::DiagnosticSummary {
             path: path.to_string_lossy().to_string(),
-            language_server_id: self.language_server_id as u64,
+            language_server_id: language_server_id as u64,
             error_count: self.error_count as u32,
             warning_count: self.warning_count as u32,
         }
@@ -4731,7 +4726,7 @@ impl Project {
 
     pub fn diagnostic_summary(&self, cx: &AppContext) -> DiagnosticSummary {
         let mut summary = DiagnosticSummary::default();
-        for (_, path_summary) in self.diagnostic_summaries(cx) {
+        for (_, _, path_summary) in self.diagnostic_summaries(cx) {
             summary.error_count += path_summary.error_count;
             summary.warning_count += path_summary.warning_count;
         }
@@ -4741,13 +4736,15 @@ impl Project {
     pub fn diagnostic_summaries<'a>(
         &'a self,
         cx: &'a AppContext,
-    ) -> impl Iterator<Item = (ProjectPath, DiagnosticSummary)> + 'a {
+    ) -> impl Iterator<Item = (ProjectPath, usize, DiagnosticSummary)> + 'a {
         self.visible_worktrees(cx).flat_map(move |worktree| {
             let worktree = worktree.read(cx);
             let worktree_id = worktree.id();
             worktree
                 .diagnostic_summaries()
-                .map(move |(path, summary)| (ProjectPath { worktree_id, path }, summary))
+                .map(move |(path, server_id, summary)| {
+                    (ProjectPath { worktree_id, path }, server_id, summary)
+                })
         })
     }
 

crates/project/src/project_tests.rs

@@ -1449,6 +1449,64 @@ async fn test_empty_diagnostic_ranges(cx: &mut gpui::TestAppContext) {
     });
 }
 
+#[gpui::test]
+async fn test_diagnostics_from_multiple_language_servers(cx: &mut gpui::TestAppContext) {
+    println!("hello from stdout");
+    eprintln!("hello from stderr");
+    cx.foreground().forbid_parking();
+
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree("/dir", json!({ "a.rs": "one two three" }))
+        .await;
+
+    let project = Project::test(fs, ["/dir".as_ref()], cx).await;
+
+    project.update(cx, |project, cx| {
+        project
+            .update_diagnostic_entries(
+                0,
+                Path::new("/dir/a.rs").to_owned(),
+                None,
+                vec![DiagnosticEntry {
+                    range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 3)),
+                    diagnostic: Diagnostic {
+                        severity: DiagnosticSeverity::ERROR,
+                        is_primary: true,
+                        message: "syntax error a1".to_string(),
+                        ..Default::default()
+                    },
+                }],
+                cx,
+            )
+            .unwrap();
+        project
+            .update_diagnostic_entries(
+                1,
+                Path::new("/dir/a.rs").to_owned(),
+                None,
+                vec![DiagnosticEntry {
+                    range: Unclipped(PointUtf16::new(0, 0))..Unclipped(PointUtf16::new(0, 3)),
+                    diagnostic: Diagnostic {
+                        severity: DiagnosticSeverity::ERROR,
+                        is_primary: true,
+                        message: "syntax error b1".to_string(),
+                        ..Default::default()
+                    },
+                }],
+                cx,
+            )
+            .unwrap();
+
+        assert_eq!(
+            project.diagnostic_summary(cx),
+            DiagnosticSummary {
+                error_count: 2,
+                warning_count: 0,
+            }
+        );
+    });
+}
+
 #[gpui::test]
 async fn test_edits_from_lsp_with_past_version(cx: &mut gpui::TestAppContext) {
     cx.foreground().forbid_parking();

crates/project/src/worktree.rs

@@ -50,7 +50,7 @@ use std::{
     },
     time::{Duration, SystemTime},
 };
-use sum_tree::{Bias, Edit, SeekTarget, SumTree, TreeMap, TreeSet};
+use sum_tree::{Bias, Edit, SeekTarget, SumTree, TreeSet};
 use util::{paths::HOME, ResultExt, TryFutureExt};
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)]
@@ -68,7 +68,7 @@ pub struct LocalWorktree {
     _background_scanner_task: Task<()>,
     share: Option<ShareState>,
     diagnostics: HashMap<Arc<Path>, Vec<(usize, Vec<DiagnosticEntry<Unclipped<PointUtf16>>>)>>,
-    diagnostic_summaries: TreeMap<PathKey, DiagnosticSummary>,
+    diagnostic_summaries: HashMap<Arc<Path>, HashMap<usize, DiagnosticSummary>>,
     client: Arc<Client>,
     fs: Arc<dyn Fs>,
     visible: bool,
@@ -82,7 +82,7 @@ pub struct RemoteWorktree {
     updates_tx: Option<UnboundedSender<proto::UpdateWorktree>>,
     snapshot_subscriptions: VecDeque<(usize, oneshot::Sender<()>)>,
     replica_id: ReplicaId,
-    diagnostic_summaries: TreeMap<PathKey, DiagnosticSummary>,
+    diagnostic_summaries: HashMap<Arc<Path>, HashMap<usize, DiagnosticSummary>>,
     visible: bool,
     disconnected: bool,
 }
@@ -463,13 +463,17 @@ impl Worktree {
 
     pub fn diagnostic_summaries(
         &self,
-    ) -> impl Iterator<Item = (Arc<Path>, DiagnosticSummary)> + '_ {
+    ) -> impl Iterator<Item = (Arc<Path>, usize, DiagnosticSummary)> + '_ {
         match self {
             Worktree::Local(worktree) => &worktree.diagnostic_summaries,
             Worktree::Remote(worktree) => &worktree.diagnostic_summaries,
         }
         .iter()
-        .map(|(path, summary)| (path.0.clone(), *summary))
+        .flat_map(|(path, summaries)| {
+            summaries
+                .iter()
+                .map(move |(&server_id, &summary)| (path.clone(), server_id, summary))
+        })
     }
 
     pub fn abs_path(&self) -> Arc<Path> {
@@ -525,30 +529,40 @@ impl LocalWorktree {
         diagnostics: Vec<DiagnosticEntry<Unclipped<PointUtf16>>>,
         _: &mut ModelContext<Worktree>,
     ) -> Result<bool> {
-        self.diagnostics.remove(&worktree_path);
-        let old_summary = self
+        let summaries_by_server_id = self
             .diagnostic_summaries
-            .remove(&PathKey(worktree_path.clone()))
+            .entry(worktree_path.clone())
+            .or_default();
+
+        let old_summary = summaries_by_server_id
+            .remove(&server_id)
             .unwrap_or_default();
-        let new_summary = DiagnosticSummary::new(server_id, &diagnostics);
-        if !new_summary.is_empty() {
-            self.diagnostic_summaries
-                .insert(PathKey(worktree_path.clone()), new_summary);
+
+        let new_summary = DiagnosticSummary::new(&diagnostics);
+        if new_summary.is_empty() {
+            if let Some(diagnostics_by_server_id) = self.diagnostics.get_mut(&worktree_path) {
+                if let Ok(ix) = diagnostics_by_server_id.binary_search_by_key(&server_id, |e| e.0) {
+                    diagnostics_by_server_id.remove(ix);
+                }
+                if diagnostics_by_server_id.is_empty() {
+                    self.diagnostics.remove(&worktree_path);
+                }
+            }
+        } else {
+            summaries_by_server_id.insert(server_id, new_summary);
             let diagnostics_by_server_id =
                 self.diagnostics.entry(worktree_path.clone()).or_default();
             match diagnostics_by_server_id.binary_search_by_key(&server_id, |e| e.0) {
                 Ok(ix) => {
                     diagnostics_by_server_id[ix] = (server_id, diagnostics);
                 }
-
                 Err(ix) => {
                     diagnostics_by_server_id.insert(ix, (server_id, diagnostics));
                 }
             }
         }
 
-        let updated = !old_summary.is_empty() || !new_summary.is_empty();
-        if updated {
+        if !old_summary.is_empty() || !new_summary.is_empty() {
             if let Some(share) = self.share.as_ref() {
                 self.client
                     .send(proto::UpdateDiagnosticSummary {
@@ -565,7 +579,7 @@ impl LocalWorktree {
             }
         }
 
-        Ok(updated)
+        Ok(!old_summary.is_empty() || !new_summary.is_empty())
     }
 
     fn set_snapshot(&mut self, new_snapshot: LocalSnapshot, cx: &mut ModelContext<Worktree>) {
@@ -955,13 +969,15 @@ impl LocalWorktree {
             let (resume_updates_tx, mut resume_updates_rx) = watch::channel();
             let worktree_id = cx.model_id() as u64;
 
-            for (path, summary) in self.diagnostic_summaries.iter() {
-                if let Err(e) = self.client.send(proto::UpdateDiagnosticSummary {
-                    project_id,
-                    worktree_id,
-                    summary: Some(summary.to_proto(&path.0)),
-                }) {
-                    return Task::ready(Err(e));
+            for (path, summaries) in &self.diagnostic_summaries {
+                for (&server_id, summary) in summaries {
+                    if let Err(e) = self.client.send(proto::UpdateDiagnosticSummary {
+                        project_id,
+                        worktree_id,
+                        summary: Some(summary.to_proto(server_id, &path)),
+                    }) {
+                        return Task::ready(Err(e));
+                    }
                 }
             }
 
@@ -1119,15 +1135,24 @@ impl RemoteWorktree {
         path: Arc<Path>,
         summary: &proto::DiagnosticSummary,
     ) {
+        let server_id = summary.language_server_id as usize;
         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,
         };
+
         if summary.is_empty() {
-            self.diagnostic_summaries.remove(&PathKey(path));
+            if let Some(summaries) = self.diagnostic_summaries.get_mut(&path) {
+                summaries.remove(&server_id);
+                if summaries.is_empty() {
+                    self.diagnostic_summaries.remove(&path);
+                }
+            }
         } else {
-            self.diagnostic_summaries.insert(PathKey(path), summary);
+            self.diagnostic_summaries
+                .entry(path)
+                .or_default()
+                .insert(server_id, summary);
         }
     }
 

crates/zed/src/languages.rs

@@ -107,7 +107,7 @@ pub fn init(
             tree_sitter_typescript::language_tsx(),
             vec![
                 adapter_arc(typescript::TypeScriptLspAdapter::new(node_runtime.clone())),
-                adapter_arc(typescript::EsLintLspAdapter::new(node_runtime.clone())),
+                // adapter_arc(typescript::EsLintLspAdapter::new(node_runtime.clone())),
             ],
         ),
         (