editor: Support walking through overlapping diagnostics (zed-industri…

…es#11139)

While looking into how to implement zed-industries#4901, noticed that the current
`Goto next/previous diagnostic` behaved a bit weirdly. That is, when
there are multiple errors that have overlapping ranges, only the first
one can be chosen to be active by the `go_to_diagnostic_impl`.

### Previous behavior:


https://github.com/zed-industries/zed/assets/71292737/95897675-f5ee-40e5-869f-0a40066eb8e3

Doesn't go through all the diagnostics, and going backwards and forwards
doesn't show the same diagnostic always.

### New behavior:


https://github.com/zed-industries/zed/assets/71292737/81f7945a-7ad8-4a34-b286-cc2799b10500

Should always go through the diagnostics in a consistent manner.

Release Notes:
* Improved the behavioral consistency of "Go to Next/Previous
Diagnostic"

crates/editor/src/editor.rs

@@ -1447,6 +1447,7 @@ impl CodeActionsMenu {
 struct ActiveDiagnosticGroup {
  primary_range: Range<Anchor>,
  primary_message: String,
+ group_id: usize,
  blocks: HashMap<BlockId, Diagnostic>,
  is_valid: bool,
 }
@@ -8015,7 +8016,7 @@ impl Editor {
  });
  let mut search_start = if let Some(active_primary_range) = active_primary_range.as_ref() {
  if active_primary_range.contains(&selection.head()) {
- *active_primary_range.end()
+ *active_primary_range.start()
  } else {
  selection.head()
  }
@@ -8024,24 +8025,38 @@ impl Editor {
  };
  let snapshot = self.snapshot(cx);
  loop {
- let mut diagnostics = if direction == Direction::Prev {
+ let diagnostics = if direction == Direction::Prev {
  buffer.diagnostics_in_range::<_, usize>(0..search_start, true)
  } else {
  buffer.diagnostics_in_range::<_, usize>(search_start..buffer.len(), false)
  }
  .filter(|diagnostic| !snapshot.intersects_fold(diagnostic.range.start));
- let group = diagnostics.find_map(|entry| {
- if entry.diagnostic.is_primary
- && entry.diagnostic.severity <= DiagnosticSeverity::WARNING
- && !entry.range.is_empty()
- && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
- && !entry.range.contains(&search_start)
- {
- Some((entry.range, entry.diagnostic.group_id))
- } else {
- None
- }
- });
+ let group = diagnostics
+ // relies on diagnostics_in_range to return diagnostics with the same starting range to
+ // be sorted in a stable way
+ // skip until we are at current active diagnostic, if it exists
+ .skip_while(|entry| {
+ (match direction {
+ Direction::Prev => entry.range.start >= search_start,
+ Direction::Next => entry.range.start <= search_start,
+ }) && self
+ .active_diagnostics
+ .as_ref()
+ .is_some_and(|a| a.group_id != entry.diagnostic.group_id)
+ })
+ .find_map(|entry| {
+ if entry.diagnostic.is_primary
+ && entry.diagnostic.severity <= DiagnosticSeverity::WARNING
+ && !entry.range.is_empty()
+ // if we match with the active diagnostic, skip it
+ && Some(entry.diagnostic.group_id)
+ != self.active_diagnostics.as_ref().map(|d| d.group_id)
+ {
+ Some((entry.range, entry.diagnostic.group_id))
+ } else {
+ None
+ }
+ });
 
  if let Some((primary_range, group_id)) = group {
  if self.activate_diagnostics(group_id, cx) {
@@ -9042,6 +9057,7 @@ impl Editor {
  Some(ActiveDiagnosticGroup {
  primary_range,
  primary_message,
+ group_id,
  blocks,
  is_valid: true,
  })

crates/language/src/buffer.rs

@@ -3159,7 +3159,21 @@ impl BufferSnapshot {
  .iter_mut()
  .enumerate()
  .flat_map(|(ix, iter)| Some((ix, iter.peek()?)))
- .min_by(|(_, a), (_, b)| a.range.start.cmp(&b.range.start))?;
+ .min_by(|(_, a), (_, b)| {
+ let cmp = a
+ .range
+ .start
+ .cmp(&b.range.start)
+ // when range is equal, sort by diagnostic severity
+ .then(a.diagnostic.severity.cmp(&b.diagnostic.severity))
+ // and stabilize order with group_id
+ .then(a.diagnostic.group_id.cmp(&b.diagnostic.group_id));
+ if reversed {
+ cmp.reverse()
+ } else {
+ cmp
+ }
+ })?;
  iterators[next_ix].next()
  })
  }