Start diagnostic group_id at 1 to handle non LS diagnostics (#22694)

Michael Sloan created 1 year ago

In particular, `DiagnosticPopover` both:

* Supports moving the selection to a diagnostic when clicked, based on
`group_id`

* Provides Diagnostic values with `group_id: 0` providing informztion on
hover about invisible characters.

So, clicking such a popover would navigate to the very first error
produced by a language server. Really not a big deal of course, but
seems good to fix as it might result in surprising behavior in other
future circumstances

Release Notes:

- N/A

Change summary

crates/collab/src/tests/integration_tests.rs |  4 +-
crates/diagnostics/src/diagnostics_tests.rs  | 20 +++++-----
crates/language/src/buffer.rs                |  6 +-
crates/project/src/lsp_store.rs              |  2 
crates/project/src/project_tests.rs          | 38 +++++++++++-----------
5 files changed, 35 insertions(+), 35 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs

@@ -4065,7 +4065,7 @@ async fn test_collaborating_with_diagnostics(
                 DiagnosticEntry {
                     range: Point::new(0, 4)..Point::new(0, 7),
                     diagnostic: Diagnostic {
-                        group_id: 2,
+                        group_id: 3,
                         message: "message 1".to_string(),
                         severity: lsp::DiagnosticSeverity::ERROR,
                         is_primary: true,
@@ -4075,7 +4075,7 @@ async fn test_collaborating_with_diagnostics(
                 DiagnosticEntry {
                     range: Point::new(0, 10)..Point::new(0, 13),
                     diagnostic: Diagnostic {
-                        group_id: 3,
+                        group_id: 4,
                         severity: lsp::DiagnosticSeverity::WARNING,
                         message: "message 2".to_string(),
                         is_primary: true,

crates/diagnostics/src/diagnostics_tests.rs

@@ -82,7 +82,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::INFORMATION,
                             is_primary: false,
                             is_disk_based: true,
-                            group_id: 1,
+                            group_id: 2,
                             ..Default::default()
                         },
                     },
@@ -95,7 +95,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::INFORMATION,
                             is_primary: false,
                             is_disk_based: true,
-                            group_id: 0,
+                            group_id: 1,
                             ..Default::default()
                         },
                     },
@@ -106,7 +106,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::INFORMATION,
                             is_primary: false,
                             is_disk_based: true,
-                            group_id: 1,
+                            group_id: 2,
                             ..Default::default()
                         },
                     },
@@ -117,7 +117,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::INFORMATION,
                             is_primary: false,
                             is_disk_based: true,
-                            group_id: 0,
+                            group_id: 1,
                             ..Default::default()
                         },
                     },
@@ -128,7 +128,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::ERROR,
                             is_primary: true,
                             is_disk_based: true,
-                            group_id: 0,
+                            group_id: 1,
                             ..Default::default()
                         },
                     },
@@ -139,7 +139,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::ERROR,
                             is_primary: true,
                             is_disk_based: true,
-                            group_id: 1,
+                            group_id: 2,
                             ..Default::default()
                         },
                     },
@@ -241,7 +241,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                         severity: DiagnosticSeverity::ERROR,
                         is_primary: true,
                         is_disk_based: true,
-                        group_id: 0,
+                        group_id: 1,
                         ..Default::default()
                     },
                 }],
@@ -348,7 +348,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::ERROR,
                             is_primary: true,
                             is_disk_based: true,
-                            group_id: 0,
+                            group_id: 1,
                             ..Default::default()
                         },
                     },
@@ -359,7 +359,7 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
                             severity: DiagnosticSeverity::ERROR,
                             is_primary: true,
                             is_disk_based: true,
-                            group_id: 1,
+                            group_id: 2,
                             ..Default::default()
                         },
                     },
@@ -775,7 +775,7 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) {
         assert!(view.focus_handle.is_focused(cx));
     });
 
-    let mut next_group_id = 0;
+    let mut next_group_id = 1;
     let mut next_filename = 0;
     let mut language_server_ids = vec![LanguageServerId(0)];
     let mut updated_language_servers = HashSet::default();

crates/language/src/buffer.rs

@@ -208,10 +208,10 @@ pub struct Diagnostic {
     /// The human-readable message associated with this diagnostic.
     pub message: String,
     /// An id that identifies the group to which this diagnostic belongs.
+    /// 0 is used for diagnostics that do not come from a language server.
     ///
-    /// When a language server produces a diagnostic with
-    /// one or more associated diagnostics, those diagnostics are all
-    /// assigned a single group ID.
+    /// When a language server produces a diagnostic with one or more associated diagnostics, those
+    /// diagnostics are all assigned a single group ID.
     pub group_id: usize,
     /// Whether this diagnostic is the primary diagnostic for its group.
     ///

crates/project/src/lsp_store.rs

@@ -2960,7 +2960,7 @@ impl LspStore {
                 http_client,
                 fs,
                 yarn,
-                next_diagnostic_group_id: Default::default(),
+                next_diagnostic_group_id: 1,
                 diagnostics: Default::default(),
                 _subscription: cx.on_app_quit(|this, cx| {
                     this.as_local_mut().unwrap().shutdown_language_servers(cx)

crates/project/src/project_tests.rs

@@ -1312,7 +1312,7 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: lsp::DiagnosticSeverity::ERROR,
                     message: "undefined variable 'A'".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -1828,7 +1828,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::ERROR,
                         message: "undefined variable 'BB'".to_string(),
                         is_disk_based: true,
-                        group_id: 1,
+                        group_id: 2,
                         is_primary: true,
                         ..Default::default()
                     },
@@ -1840,7 +1840,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::ERROR,
                         message: "undefined variable 'CCC'".to_string(),
                         is_disk_based: true,
-                        group_id: 2,
+                        group_id: 3,
                         is_primary: true,
                         ..Default::default()
                     }
@@ -1906,7 +1906,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::WARNING,
                         message: "unreachable statement".to_string(),
                         is_disk_based: true,
-                        group_id: 4,
+                        group_id: 5,
                         is_primary: true,
                         ..Default::default()
                     }
@@ -1918,7 +1918,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::ERROR,
                         message: "undefined variable 'A'".to_string(),
                         is_disk_based: true,
-                        group_id: 3,
+                        group_id: 4,
                         is_primary: true,
                         ..Default::default()
                     },
@@ -1998,7 +1998,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::WARNING,
                         message: "undefined variable 'A'".to_string(),
                         is_disk_based: true,
-                        group_id: 6,
+                        group_id: 7,
                         is_primary: true,
                         ..Default::default()
                     }
@@ -2010,7 +2010,7 @@ async fn test_transforming_diagnostics(cx: &mut gpui::TestAppContext) {
                         severity: DiagnosticSeverity::ERROR,
                         message: "undefined variable 'BB'".to_string(),
                         is_disk_based: true,
-                        group_id: 5,
+                        group_id: 6,
                         is_primary: true,
                         ..Default::default()
                     },
@@ -3838,7 +3838,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::WARNING,
                     message: "error 1".to_string(),
-                    group_id: 1,
+                    group_id: 2,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -3848,7 +3848,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 1 hint 1".to_string(),
-                    group_id: 1,
+                    group_id: 2,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -3858,7 +3858,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 1".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -3868,7 +3868,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 2".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -3878,7 +3878,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::ERROR,
                     message: "error 2".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -3887,14 +3887,14 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
     );
 
     assert_eq!(
-        buffer.diagnostic_group::<Point>(0).collect::<Vec<_>>(),
+        buffer.diagnostic_group::<Point>(1).collect::<Vec<_>>(),
         &[
             DiagnosticEntry {
                 range: Point::new(1, 13)..Point::new(1, 15),
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 1".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -3904,7 +3904,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 2 hint 2".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: false,
                     ..Default::default()
                 }
@@ -3914,7 +3914,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::ERROR,
                     message: "error 2".to_string(),
-                    group_id: 0,
+                    group_id: 1,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -3923,14 +3923,14 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
     );
 
     assert_eq!(
-        buffer.diagnostic_group::<Point>(1).collect::<Vec<_>>(),
+        buffer.diagnostic_group::<Point>(2).collect::<Vec<_>>(),
         &[
             DiagnosticEntry {
                 range: Point::new(1, 8)..Point::new(1, 9),
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::WARNING,
                     message: "error 1".to_string(),
-                    group_id: 1,
+                    group_id: 2,
                     is_primary: true,
                     ..Default::default()
                 }
@@ -3940,7 +3940,7 @@ async fn test_grouped_diagnostics(cx: &mut gpui::TestAppContext) {
                 diagnostic: Diagnostic {
                     severity: DiagnosticSeverity::HINT,
                     message: "error 1 hint 1".to_string(),
-                    group_id: 1,
+                    group_id: 2,
                     is_primary: false,
                     ..Default::default()
                 }