Implement `Buffer::diagnostic_group`

Antonio Scandurra created

Change summary

crates/editor/src/editor.rs       |  19 ++-
crates/editor/src/items.rs        |   6 
crates/editor/src/multi_buffer.rs |   5 
crates/language/src/buffer.rs     |  34 +++--
crates/language/src/tests.rs      | 183 ++++++++++++++++++--------------
crates/project/src/worktree.rs    | 138 ++++++++++++++----------
crates/server/src/rpc.rs          |   1 
7 files changed, 222 insertions(+), 164 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -2844,19 +2844,19 @@ impl Editor {
         loop {
             let next_group = buffer
                 .diagnostics_in_range::<_, usize>(search_start..buffer.len())
-                .find_map(|entry| {
+                .find_map(|(provider_name, entry)| {
                     if entry.diagnostic.is_primary
                         && !entry.range.is_empty()
                         && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
                     {
-                        Some((entry.range, entry.diagnostic.group_id))
+                        Some((provider_name, entry.range, entry.diagnostic.group_id))
                     } else {
                         None
                     }
                 });
 
-            if let Some((primary_range, group_id)) = next_group {
-                self.activate_diagnostics(group_id, cx);
+            if let Some((provider_name, primary_range, group_id)) = next_group {
+                self.activate_diagnostics(provider_name, group_id, cx);
                 self.update_selections(
                     vec![Selection {
                         id: selection.id,
@@ -2884,7 +2884,7 @@ impl Editor {
             let primary_range_start = active_diagnostics.primary_range.start.to_offset(&buffer);
             let is_valid = buffer
                 .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone())
-                .any(|entry| {
+                .any(|(_, entry)| {
                     entry.diagnostic.is_primary
                         && !entry.range.is_empty()
                         && entry.range.start == primary_range_start
@@ -2910,7 +2910,12 @@ impl Editor {
         }
     }
 
-    fn activate_diagnostics(&mut self, group_id: usize, cx: &mut ViewContext<Self>) {
+    fn activate_diagnostics(
+        &mut self,
+        provider_name: &str,
+        group_id: usize,
+        cx: &mut ViewContext<Self>,
+    ) {
         self.dismiss_diagnostics(cx);
         self.active_diagnostics = self.display_map.update(cx, |display_map, cx| {
             let buffer = self.buffer.read(cx).snapshot(cx);
@@ -2919,7 +2924,7 @@ impl Editor {
             let mut primary_message = None;
             let mut group_end = Point::zero();
             let diagnostic_group = buffer
-                .diagnostic_group::<Point>(group_id)
+                .diagnostic_group::<Point>(provider_name, group_id)
                 .map(|entry| {
                     if entry.range.end > group_end {
                         group_end = entry.range.end;

crates/editor/src/items.rs 🔗

@@ -298,9 +298,9 @@ impl DiagnosticMessage {
         let new_diagnostic = buffer
             .read(cx)
             .diagnostics_in_range::<_, usize>(cursor_position..cursor_position)
-            .filter(|entry| !entry.range.is_empty())
-            .min_by_key(|entry| (entry.diagnostic.severity, entry.range.len()))
-            .map(|entry| entry.diagnostic);
+            .filter(|(_, entry)| !entry.range.is_empty())
+            .min_by_key(|(_, entry)| (entry.diagnostic.severity, entry.range.len()))
+            .map(|(_, entry)| entry.diagnostic);
         if new_diagnostic != self.diagnostic {
             self.diagnostic = new_diagnostic;
             cx.notify();

crates/editor/src/multi_buffer.rs 🔗

@@ -1470,6 +1470,7 @@ impl MultiBufferSnapshot {
 
     pub fn diagnostic_group<'a, O>(
         &'a self,
+        provider_name: &'a str,
         group_id: usize,
     ) -> impl Iterator<Item = DiagnosticEntry<O>> + 'a
     where
@@ -1477,13 +1478,13 @@ impl MultiBufferSnapshot {
     {
         self.as_singleton()
             .into_iter()
-            .flat_map(move |buffer| buffer.diagnostic_group(group_id))
+            .flat_map(move |buffer| buffer.diagnostic_group(provider_name, group_id))
     }
 
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         range: Range<T>,
-    ) -> impl Iterator<Item = DiagnosticEntry<O>> + 'a
+    ) -> impl Iterator<Item = (&'a str, DiagnosticEntry<O>)> + 'a
     where
         T: 'a + ToOffset,
         O: 'a + text::FromAnchor,

crates/language/src/buffer.rs 🔗

@@ -23,7 +23,7 @@ use std::{
     ffi::OsString,
     future::Future,
     iter::{Iterator, Peekable},
-    ops::{Add, Deref, DerefMut, Range, Sub},
+    ops::{Deref, DerefMut, Range, Sub},
     path::{Path, PathBuf},
     str,
     sync::Arc,
@@ -741,11 +741,6 @@ impl Buffer {
         cx.notify();
     }
 
-    pub fn all_diagnostics<'a>(&'a self) -> impl 'a + Iterator<Item = &'a DiagnosticEntry<Anchor>> {
-        // TODO - enforce ordering between sets
-        self.diagnostic_sets.iter().flat_map(|set| set.iter())
-    }
-
     pub fn update_diagnostics<T>(
         &mut self,
         provider_name: Arc<str>,
@@ -754,7 +749,7 @@ impl Buffer {
         cx: &mut ModelContext<Self>,
     ) -> Result<Operation>
     where
-        T: ToPoint + Ord + Clip + TextDimension + Add<Output = T> + Sub<Output = T> + Copy,
+        T: Copy + Ord + TextDimension + Sub<Output = T> + Clip + ToPoint,
     {
         fn compare_diagnostics(a: &Diagnostic, b: &Diagnostic) -> Ordering {
             Ordering::Equal
@@ -810,8 +805,10 @@ impl Buffer {
                     }
                 }
 
-                start = last_edit_new_end + (start - last_edit_old_end);
-                end = last_edit_new_end + (end - last_edit_old_end);
+                start = last_edit_new_end;
+                start.add_assign(&(start - last_edit_old_end));
+                end = last_edit_new_end;
+                end.add_assign(&(end - last_edit_old_end));
             }
 
             let range = start.clip(Bias::Left, content)..end.clip(Bias::Right, content);
@@ -1624,7 +1621,7 @@ impl BufferSnapshot {
         let mut highlights = None;
         let mut diagnostic_endpoints = Vec::<DiagnosticEndpoint>::new();
         if let Some(theme) = theme {
-            for entry in self.diagnostics_in_range::<_, usize>(range.clone()) {
+            for (_, entry) in self.diagnostics_in_range::<_, usize>(range.clone()) {
                 diagnostic_endpoints.push(DiagnosticEndpoint {
                     offset: entry.range.start,
                     is_start: true,
@@ -1755,14 +1752,15 @@ impl BufferSnapshot {
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         search_range: Range<T>,
-    ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
+    ) -> impl 'a + Iterator<Item = (&'a str, DiagnosticEntry<O>)>
     where
         T: 'a + Clone + ToOffset,
         O: 'a + FromAnchor,
     {
-        self.diagnostic_sets
-            .iter()
-            .flat_map(move |set| set.range(search_range.clone(), self, true))
+        self.diagnostic_sets.iter().flat_map(move |set| {
+            set.range(search_range.clone(), self, true)
+                .map(|e| (set.provider_name(), e))
+        })
     }
 
     pub fn diagnostic_groups(&self) -> Vec<DiagnosticGroup<Anchor>> {
@@ -1775,13 +1773,17 @@ impl BufferSnapshot {
 
     pub fn diagnostic_group<'a, O>(
         &'a self,
+        provider_name: &str,
         group_id: usize,
     ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
     where
         O: 'a + FromAnchor,
     {
-        todo!();
-        [].into_iter()
+        self.diagnostic_sets
+            .iter()
+            .find(|s| s.provider_name() == provider_name)
+            .into_iter()
+            .flat_map(move |s| s.group(group_id, self))
     }
 
     pub fn diagnostics_update_count(&self) -> usize {

crates/language/src/tests.rs 🔗

@@ -560,28 +560,34 @@ async fn test_diagnostics(mut cx: gpui::TestAppContext) {
                 .diagnostics_in_range::<_, Point>(Point::new(3, 0)..Point::new(5, 0))
                 .collect::<Vec<_>>(),
             &[
-                DiagnosticEntry {
-                    range: Point::new(3, 9)..Point::new(3, 11),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'BB'".to_string(),
-                        is_disk_based: true,
-                        group_id: 1,
-                        is_primary: true,
-                        ..Default::default()
-                    },
-                },
-                DiagnosticEntry {
-                    range: Point::new(4, 9)..Point::new(4, 12),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'CCC'".to_string(),
-                        is_disk_based: true,
-                        group_id: 2,
-                        is_primary: true,
-                        ..Default::default()
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(3, 9)..Point::new(3, 11),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'BB'".to_string(),
+                            is_disk_based: true,
+                            group_id: 1,
+                            is_primary: true,
+                            ..Default::default()
+                        },
                     }
-                }
+                ),
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(4, 9)..Point::new(4, 12),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'CCC'".to_string(),
+                            is_disk_based: true,
+                            group_id: 2,
+                            is_primary: true,
+                            ..Default::default()
+                        }
+                    }
+                )
             ]
         );
         assert_eq!(
@@ -642,27 +648,33 @@ async fn test_diagnostics(mut cx: gpui::TestAppContext) {
                 .diagnostics_in_range::<_, Point>(Point::new(2, 0)..Point::new(3, 0))
                 .collect::<Vec<_>>(),
             &[
-                DiagnosticEntry {
-                    range: Point::new(2, 9)..Point::new(2, 12),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::WARNING,
-                        message: "unreachable statement".to_string(),
-                        group_id: 3,
-                        is_primary: true,
-                        ..Default::default()
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(2, 9)..Point::new(2, 12),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::WARNING,
+                            message: "unreachable statement".to_string(),
+                            group_id: 3,
+                            is_primary: true,
+                            ..Default::default()
+                        }
                     }
-                },
-                DiagnosticEntry {
-                    range: Point::new(2, 9)..Point::new(2, 10),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'A'".to_string(),
-                        is_disk_based: true,
-                        group_id: 0,
-                        is_primary: true,
-                        ..Default::default()
-                    },
-                }
+                ),
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(2, 9)..Point::new(2, 10),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'A'".to_string(),
+                            is_disk_based: true,
+                            group_id: 0,
+                            is_primary: true,
+                            ..Default::default()
+                        },
+                    }
+                )
             ]
         );
         assert_eq!(
@@ -734,28 +746,34 @@ async fn test_diagnostics(mut cx: gpui::TestAppContext) {
                 .diagnostics_in_range::<_, Point>(0..buffer.len())
                 .collect::<Vec<_>>(),
             &[
-                DiagnosticEntry {
-                    range: Point::new(2, 21)..Point::new(2, 22),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'A'".to_string(),
-                        is_disk_based: true,
-                        group_id: 0,
-                        is_primary: true,
-                        ..Default::default()
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(2, 21)..Point::new(2, 22),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'A'".to_string(),
+                            is_disk_based: true,
+                            group_id: 0,
+                            is_primary: true,
+                            ..Default::default()
+                        }
                     }
-                },
-                DiagnosticEntry {
-                    range: Point::new(3, 9)..Point::new(3, 11),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'BB'".to_string(),
-                        is_disk_based: true,
-                        group_id: 4,
-                        is_primary: true,
-                        ..Default::default()
-                    },
-                }
+                ),
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: Point::new(3, 9)..Point::new(3, 11),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'BB'".to_string(),
+                            is_disk_based: true,
+                            group_id: 4,
+                            is_primary: true,
+                            ..Default::default()
+                        },
+                    }
+                )
             ]
         );
     });
@@ -829,7 +847,10 @@ async fn test_preserving_old_group_ids_and_disk_based_diagnostics(mut cx: gpui::
                 .snapshot()
                 .diagnostics_in_range::<_, PointUtf16>(PointUtf16::new(0, 0)..PointUtf16::new(4, 0))
                 .collect::<Vec<_>>(),
-            diagnostics.as_slice(),
+            diagnostics
+                .iter()
+                .map(|entry| ("lsp", entry.clone()))
+                .collect::<Vec<_>>(),
         );
     });
 
@@ -849,24 +870,30 @@ async fn test_preserving_old_group_ids_and_disk_based_diagnostics(mut cx: gpui::
                 .collect::<Vec<_>>(),
             &[
                 // The changed diagnostic is given a new group id.
-                DiagnosticEntry {
-                    range: new_diagnostics[0].range.clone(),
-                    diagnostic: Diagnostic {
-                        group_id: 3,
-                        ..new_diagnostics[0].diagnostic.clone()
-                    },
-                },
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: new_diagnostics[0].range.clone(),
+                        diagnostic: Diagnostic {
+                            group_id: 3,
+                            ..new_diagnostics[0].diagnostic.clone()
+                        },
+                    }
+                ),
                 // The old disk-based diagnostic is marked as invalid, but keeps
                 // its original group id.
-                DiagnosticEntry {
-                    range: diagnostics[1].range.clone(),
-                    diagnostic: Diagnostic {
-                        is_valid: false,
-                        ..diagnostics[1].diagnostic.clone()
-                    },
-                },
+                (
+                    "lsp",
+                    DiagnosticEntry {
+                        range: diagnostics[1].range.clone(),
+                        diagnostic: Diagnostic {
+                            is_valid: false,
+                            ..diagnostics[1].diagnostic.clone()
+                        },
+                    }
+                ),
                 // The unchanged diagnostic keeps its original group id
-                new_diagnostics[1].clone(),
+                ("lsp", new_diagnostics[1].clone()),
             ],
         );
     });

crates/project/src/worktree.rs 🔗

@@ -3729,22 +3729,25 @@ mod tests {
             .unwrap();
 
         buffer.read_with(&cx, |buffer, _| {
-            let diagnostics = buffer
-                .snapshot()
+            let snapshot = buffer.snapshot();
+            let diagnostics = snapshot
                 .diagnostics_in_range::<_, Point>(0..buffer.len())
                 .collect::<Vec<_>>();
             assert_eq!(
                 diagnostics,
-                &[DiagnosticEntry {
-                    range: Point::new(0, 9)..Point::new(0, 10),
-                    diagnostic: Diagnostic {
-                        severity: lsp::DiagnosticSeverity::ERROR,
-                        message: "undefined variable 'A'".to_string(),
-                        group_id: 0,
-                        is_primary: true,
-                        ..Default::default()
+                &[(
+                    LSP_PROVIDER_NAME.as_ref(),
+                    DiagnosticEntry {
+                        range: Point::new(0, 9)..Point::new(0, 10),
+                        diagnostic: Diagnostic {
+                            severity: lsp::DiagnosticSeverity::ERROR,
+                            message: "undefined variable 'A'".to_string(),
+                            group_id: 0,
+                            is_primary: true,
+                            ..Default::default()
+                        }
                     }
-                }]
+                )]
             )
         });
     }
@@ -3899,61 +3902,78 @@ mod tests {
                 .diagnostics_in_range::<_, Point>(0..buffer.len())
                 .collect::<Vec<_>>(),
             &[
-                DiagnosticEntry {
-                    range: Point::new(1, 8)..Point::new(1, 9),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::WARNING,
-                        message: "error 1".to_string(),
-                        group_id: 0,
-                        is_primary: true,
-                        ..Default::default()
+                (
+                    LSP_PROVIDER_NAME.as_ref(),
+                    DiagnosticEntry {
+                        range: Point::new(1, 8)..Point::new(1, 9),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::WARNING,
+                            message: "error 1".to_string(),
+                            group_id: 0,
+                            is_primary: true,
+                            ..Default::default()
+                        }
                     }
-                },
-                DiagnosticEntry {
-                    range: Point::new(1, 8)..Point::new(1, 9),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::HINT,
-                        message: "error 1 hint 1".to_string(),
-                        group_id: 0,
-                        is_primary: false,
-                        ..Default::default()
+                ),
+                (
+                    LSP_PROVIDER_NAME.as_ref(),
+                    DiagnosticEntry {
+                        range: Point::new(1, 8)..Point::new(1, 9),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::HINT,
+                            message: "error 1 hint 1".to_string(),
+                            group_id: 0,
+                            is_primary: false,
+                            ..Default::default()
+                        }
                     }
-                },
-                DiagnosticEntry {
-                    range: Point::new(1, 13)..Point::new(1, 15),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::HINT,
-                        message: "error 2 hint 1".to_string(),
-                        group_id: 1,
-                        is_primary: false,
-                        ..Default::default()
+                ),
+                (
+                    LSP_PROVIDER_NAME.as_ref(),
+                    DiagnosticEntry {
+                        range: Point::new(1, 13)..Point::new(1, 15),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::HINT,
+                            message: "error 2 hint 1".to_string(),
+                            group_id: 1,
+                            is_primary: false,
+                            ..Default::default()
+                        }
                     }
-                },
-                DiagnosticEntry {
-                    range: Point::new(1, 13)..Point::new(1, 15),
-                    diagnostic: Diagnostic {
-                        severity: DiagnosticSeverity::HINT,
-                        message: "error 2 hint 2".to_string(),
-                        group_id: 1,
-                        is_primary: false,
-                        ..Default::default()
+                ),
+                (
+                    LSP_PROVIDER_NAME.as_ref(),
+                    DiagnosticEntry {
+                        range: Point::new(1, 13)..Point::new(1, 15),
+                        diagnostic: Diagnostic {
+                            severity: DiagnosticSeverity::HINT,
+                            message: "error 2 hint 2".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()
+                ),
+                (
+                    LSP_PROVIDER_NAME.as_ref(),
+                    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()
+                        }
                     }
-                }
+                )
             ]
         );
 
         assert_eq!(
-            buffer.diagnostic_group::<Point>(0).collect::<Vec<_>>(),
+            buffer
+                .diagnostic_group::<Point>(&LSP_PROVIDER_NAME, 0)
+                .collect::<Vec<_>>(),
             &[
                 DiagnosticEntry {
                     range: Point::new(1, 8)..Point::new(1, 9),
@@ -3978,7 +3998,9 @@ mod tests {
             ]
         );
         assert_eq!(
-            buffer.diagnostic_group::<Point>(1).collect::<Vec<_>>(),
+            buffer
+                .diagnostic_group::<Point>(&LSP_PROVIDER_NAME, 1)
+                .collect::<Vec<_>>(),
             &[
                 DiagnosticEntry {
                     range: Point::new(1, 13)..Point::new(1, 15),

crates/server/src/rpc.rs 🔗

@@ -1831,6 +1831,7 @@ mod tests {
                 buffer
                     .snapshot()
                     .diagnostics_in_range::<_, Point>(0..buffer.len())
+                    .map(|(_, entry)| entry)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {