Merge pull request #2246 from zed-industries/fix-lsp-derive-error

Mikayla Maki created

Make diagnostic processing order independent

Change summary

Cargo.lock                          |  1 
crates/project/Cargo.toml           |  1 
crates/project/src/project.rs       |  6 ++
crates/project/src/project_tests.rs | 56 ++++++++++++++++--------------
4 files changed, 36 insertions(+), 28 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4592,6 +4592,7 @@ dependencies = [
  "lsp",
  "parking_lot 0.11.2",
  "postage",
+ "pretty_assertions",
  "pulldown-cmark",
  "rand 0.8.5",
  "regex",

crates/project/Cargo.toml 🔗

@@ -57,6 +57,7 @@ thiserror = "1.0.29"
 toml = "0.5"
 
 [dev-dependencies]
+pretty_assertions = "1.3.0"
 client = { path = "../client", features = ["test-support"] }
 collections = { path = "../collections", features = ["test-support"] }
 db = { path = "../db", features = ["test-support"] }

crates/project/src/project.rs 🔗

@@ -2538,7 +2538,7 @@ impl Project {
     pub fn update_diagnostics(
         &mut self,
         language_server_id: usize,
-        params: lsp::PublishDiagnosticsParams,
+        mut params: lsp::PublishDiagnosticsParams,
         disk_based_sources: &[String],
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
@@ -2550,6 +2550,10 @@ impl Project {
         let mut primary_diagnostic_group_ids = HashMap::default();
         let mut sources_by_group_id = HashMap::default();
         let mut supporting_diagnostics = HashMap::default();
+
+        // Ensure that primary diagnostics are always the most severe
+        params.diagnostics.sort_by_key(|item| item.severity);
+
         for diagnostic in &params.diagnostics {
             let source = diagnostic.source.as_ref();
             let code = diagnostic.code.as_ref().map(|code| match code {

crates/project/src/project_tests.rs 🔗

@@ -8,6 +8,7 @@ use language::{
     OffsetRangeExt, Point, ToPoint,
 };
 use lsp::Url;
+use pretty_assertions::assert_eq;
 use serde_json::json;
 use std::{cell::RefCell, os::unix, rc::Rc, task::Poll};
 use unindent::Unindent as _;
@@ -2846,7 +2847,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::WARNING,
                     message: "error 1".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -2856,7 +2857,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 1 hint 1".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -2866,7 +2867,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 1".to_string(),
-                    group_id: 1,
+                    group_id: 0,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -2876,7 +2877,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 2".to_string(),
-                    group_id: 1,
+                    group_id: 0,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -2886,7 +2887,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::ERROR,
                     message: "error 2".to_string(),
-                    group_id: 1,
+                    group_id: 0,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -2898,60 +2899,61 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
         buffer.diagnostic_group::<Point>(0).collect::<Vec<_>>(),
         &[
             DiagnosticEntry {
-                range: Point::new(1, 8)..Point::new(1, 9),
+                range: Point::new(1, 13)..Point::new(1, 15),
                 diagnostic: Diagnostic {
-                    severity: DiagnosticSeverity::WARNING,
-                    message: "error 1".to_string(),
+                    severity: DiagnosticSeverity::HINT,
+                    message: "error 2 hint 1".to_string(),
                     group_id: 0,
-                    is_primary: true,
+                    is_primary: false,
                     ..Default::default()
                 }
             },
             DiagnosticEntry {
-                range: Point::new(1, 8)..Point::new(1, 9),
+                range: Point::new(1, 13)..Point::new(1, 15),
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
-                    message: "error 1 hint 1".to_string(),
+                    message: "error 2 hint 2".to_string(),
                     group_id: 0,
                     is_primary: false,
                     ..Default::default()
                 }
             },
+            DiagnosticEntry {
+                range: Point::new(2, 8)..Point::new(2, 17),
+                diagnostic: Diagnostic {
+                    severity: DiagnosticSeverity::ERROR,
+                    message: "error 2".to_string(),
+                    group_id: 0,
+                    is_primary: true,
+                    ..Default::default()
+                }
+            }
         ]
     );
+
     assert_eq!(
         buffer.diagnostic_group::<Point>(1).collect::<Vec<_>>(),
         &[
             DiagnosticEntry {
-                range: Point::new(1, 13)..Point::new(1, 15),
+                range: Point::new(1, 8)..Point::new(1, 9),
                 diagnostic: Diagnostic {
-                    severity: DiagnosticSeverity::HINT,
-                    message: "error 2 hint 1".to_string(),
+                    severity: DiagnosticSeverity::WARNING,
+                    message: "error 1".to_string(),
                     group_id: 1,
-                    is_primary: false,
+                    is_primary: true,
                     ..Default::default()
                 }
             },
             DiagnosticEntry {
-                range: Point::new(1, 13)..Point::new(1, 15),
+                range: Point::new(1, 8)..Point::new(1, 9),
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
-                    message: "error 2 hint 2".to_string(),
+                    message: "error 1 hint 1".to_string(),
                     group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
             },
-            DiagnosticEntry {
-                range: Point::new(2, 8)..Point::new(2, 17),
-                diagnostic: Diagnostic {
-                    severity: DiagnosticSeverity::ERROR,
-                    message: "error 2".to_string(),
-                    group_id: 1,
-                    is_primary: true,
-                    ..Default::default()
-                }
-            }
         ]
     );
 }