editor: Assert ordering in selections of resolve_selections (#38861)

Inspired by the recent anchor assertions, this asserts that the produced
selections are always ordered at various resolutions stages, this is an
invariant within `SelectionsCollection` but something breaks it
somewhere causing us to seek cursors backwards which panics.

Related to ZED-13X

Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -3058,7 +3058,7 @@ impl Editor {
             self.buffer.update(cx, |buffer, cx| {
                 buffer.set_active_selections(
                     &selection_anchors,
-                    self.selections.line_mode,
+                    self.selections.line_mode(),
                     self.cursor_shape,
                     cx,
                 )
@@ -6900,7 +6900,7 @@ impl Editor {
         if !EditorSettings::get_global(cx).selection_highlight {
             return None;
         }
-        if self.selections.count() != 1 || self.selections.line_mode {
+        if self.selections.count() != 1 || self.selections.line_mode() {
             return None;
         }
         let selection = self.selections.newest::<Point>(cx);
@@ -12269,7 +12269,7 @@ impl Editor {
             let mut is_first = true;
             for selection in &mut selections {
                 let is_entire_line =
-                    (selection.is_empty() && cut_no_selection_line) || self.selections.line_mode;
+                    (selection.is_empty() && cut_no_selection_line) || self.selections.line_mode();
                 if is_entire_line {
                     selection.start = Point::new(selection.start.row, 0);
                     if !selection.is_empty() && selection.end.column == 0 {
@@ -12369,7 +12369,7 @@ impl Editor {
             for selection in &selections {
                 let mut start = selection.start;
                 let mut end = selection.end;
-                let is_entire_line = selection.is_empty() || self.selections.line_mode;
+                let is_entire_line = selection.is_empty() || self.selections.line_mode();
                 if is_entire_line {
                     start = Point::new(start.row, 0);
                     end = cmp::min(max_point, Point::new(end.row + 1, 0));
@@ -21357,7 +21357,7 @@ impl Editor {
                 if self.leader_id.is_none() {
                     buffer.set_active_selections(
                         &self.selections.disjoint_anchors_arc(),
-                        self.selections.line_mode,
+                        self.selections.line_mode(),
                         self.cursor_shape,
                         cx,
                     );

crates/editor/src/editor_tests.rs

@@ -5370,8 +5370,8 @@ async fn test_manipulate_text(cx: &mut TestAppContext) {
         «HeLlO, wOrLD!ˇ»
     "});
 
-    // Test selections with `line_mode = true`.
-    cx.update_editor(|editor, _window, _cx| editor.selections.line_mode = true);
+    // Test selections with `line_mode() = true`.
+    cx.update_editor(|editor, _window, _cx| editor.selections.set_line_mode(true));
     cx.set_state(indoc! {"
         «The quick brown
         fox jumps over

crates/editor/src/element.rs

@@ -1371,7 +1371,7 @@ impl EditorElement {
 
                     let layout = SelectionLayout::new(
                         selection,
-                        editor.selections.line_mode,
+                        editor.selections.line_mode(),
                         editor.cursor_shape,
                         &snapshot.display_snapshot,
                         is_newest,
@@ -3149,7 +3149,7 @@ impl EditorElement {
                 let newest = editor.selections.newest::<Point>(cx);
                 SelectionLayout::new(
                     newest,
-                    editor.selections.line_mode,
+                    editor.selections.line_mode(),
                     editor.cursor_shape,
                     &snapshot.display_snapshot,
                     true,

crates/editor/src/hover_links.rs

@@ -666,9 +666,7 @@ pub(crate) fn find_url(
 ) -> Option<(Range<text::Anchor>, String)> {
     const LIMIT: usize = 2048;
 
-    let Ok(snapshot) = buffer.read_with(&cx, |buffer, _| buffer.snapshot()) else {
-        return None;
-    };
+    let snapshot = buffer.read_with(&cx, |buffer, _| buffer.snapshot()).ok()?;
 
     let offset = position.to_offset(&snapshot);
     let mut token_start = offset;

crates/editor/src/items.rs

@@ -190,7 +190,7 @@ impl FollowableItem for Editor {
             self.buffer.update(cx, |buffer, cx| {
                 buffer.set_active_selections(
                     &self.selections.disjoint_anchors_arc(),
-                    self.selections.line_mode,
+                    self.selections.line_mode(),
                     self.cursor_shape,
                     cx,
                 );

crates/editor/src/selections_collection.rs

@@ -1,6 +1,6 @@
 use std::{
     cell::Ref,
-    cmp, iter, mem,
+    cmp, fmt, iter, mem,
     ops::{Deref, DerefMut, Range, Sub},
     sync::Arc,
 };
@@ -29,7 +29,7 @@ pub struct SelectionsCollection {
     display_map: Entity<DisplayMap>,
     buffer: Entity<MultiBuffer>,
     next_selection_id: usize,
-    pub line_mode: bool,
+    line_mode: bool,
     /// The non-pending, non-overlapping selections.
     /// The [SelectionsCollection::pending] selection could possibly overlap these
     disjoint: Arc<[Selection<Anchor>]>,
@@ -424,6 +424,14 @@ impl SelectionsCollection {
     pub fn next_selection_id(&self) -> usize {
         self.next_selection_id
     }
+
+    pub fn line_mode(&self) -> bool {
+        self.line_mode
+    }
+
+    pub fn set_line_mode(&mut self, line_mode: bool) {
+        self.line_mode = line_mode;
+    }
 }
 
 pub struct MutableSelectionsCollection<'a> {
@@ -914,10 +922,10 @@ fn selection_to_anchor_selection<T>(
 where
     T: ToOffset + Ord,
 {
-    let end_bias = if selection.end > selection.start {
-        Bias::Left
-    } else {
+    let end_bias = if selection.start == selection.end {
         Bias::Right
+    } else {
+        Bias::Left
     };
     Selection {
         id: selection.id,
@@ -928,51 +936,61 @@ where
     }
 }
 
-// Panics if passed selections are not in order
-fn resolve_selections_display<'a>(
+fn resolve_selections_point<'a>(
     selections: impl 'a + IntoIterator<Item = &'a Selection<Anchor>>,
     map: &'a DisplaySnapshot,
-) -> impl 'a + Iterator<Item = Selection<DisplayPoint>> {
+) -> impl 'a + Iterator<Item = Selection<Point>> {
     let (to_summarize, selections) = selections.into_iter().tee();
     let mut summaries = map
         .buffer_snapshot
         .summaries_for_anchors::<Point, _>(to_summarize.flat_map(|s| [&s.start, &s.end]))
         .into_iter();
-    let mut selections = selections
-        .map(move |s| {
-            let start = summaries.next().unwrap();
-            let end = summaries.next().unwrap();
-
-            let display_start = map.point_to_display_point(start, Bias::Left);
-            let display_end = if start == end {
-                map.point_to_display_point(end, Bias::Right)
-            } else {
-                map.point_to_display_point(end, Bias::Left)
-            };
-
-            Selection {
-                id: s.id,
-                start: display_start,
-                end: display_end,
-                reversed: s.reversed,
-                goal: s.goal,
-            }
-        })
-        .peekable();
-    iter::from_fn(move || {
-        let mut selection = selections.next()?;
-        while let Some(next_selection) = selections.peek() {
-            if selection.end >= next_selection.start {
-                selection.end = cmp::max(selection.end, next_selection.end);
-                selections.next();
-            } else {
-                break;
-            }
+    selections.map(move |s| {
+        let start = summaries.next().unwrap();
+        let end = summaries.next().unwrap();
+        assert!(start <= end, "start: {:?}, end: {:?}", start, end);
+        Selection {
+            id: s.id,
+            start,
+            end,
+            reversed: s.reversed,
+            goal: s.goal,
         }
-        Some(selection)
     })
 }
 
+// Panics if passed selections are not in order
+fn resolve_selections_display<'a>(
+    selections: impl 'a + IntoIterator<Item = &'a Selection<Anchor>>,
+    map: &'a DisplaySnapshot,
+) -> impl 'a + Iterator<Item = Selection<DisplayPoint>> {
+    let selections = resolve_selections_point(selections, map).map(move |s| {
+        let display_start = map.point_to_display_point(s.start, Bias::Left);
+        let display_end = map.point_to_display_point(
+            s.end,
+            if s.start == s.end {
+                Bias::Right
+            } else {
+                Bias::Left
+            },
+        );
+        assert!(
+            display_start <= display_end,
+            "display_start: {:?}, display_end: {:?}",
+            display_start,
+            display_end
+        );
+        Selection {
+            id: s.id,
+            start: display_start,
+            end: display_end,
+            reversed: s.reversed,
+            goal: s.goal,
+        }
+    });
+    coalesce_selections(selections)
+}
+
 // Panics if passed selections are not in order
 pub(crate) fn resolve_selections<'a, D, I>(
     selections: I,
@@ -988,11 +1006,13 @@ where
             .dimensions_from_points::<D>(to_convert.flat_map(|s| {
                 let start = map.display_point_to_point(s.start, Bias::Left);
                 let end = map.display_point_to_point(s.end, Bias::Right);
+                assert!(start <= end, "start: {:?}, end: {:?}", start, end);
                 [start, end]
             }));
     selections.map(move |s| {
         let start = converted_endpoints.next().unwrap();
         let end = converted_endpoints.next().unwrap();
+        assert!(start <= end, "start: {:?}, end: {:?}", start, end);
         Selection {
             id: s.id,
             start,
@@ -1002,3 +1022,33 @@ where
         }
     })
 }
+
+fn coalesce_selections<D: Ord + fmt::Debug + Copy>(
+    selections: impl Iterator<Item = Selection<D>>,
+) -> impl Iterator<Item = Selection<D>> {
+    let mut selections = selections.peekable();
+    iter::from_fn(move || {
+        let mut selection = selections.next()?;
+        while let Some(next_selection) = selections.peek() {
+            if selection.end >= next_selection.start {
+                if selection.reversed == next_selection.reversed {
+                    selection.end = cmp::max(selection.end, next_selection.end);
+                    selections.next();
+                } else {
+                    selection.end = cmp::max(selection.start, next_selection.start);
+                    break;
+                }
+            } else {
+                break;
+            }
+        }
+        assert!(
+            selection.start <= selection.end,
+            "selection.start: {:?}, selection.end: {:?}, selection.reversed: {:?}",
+            selection.start,
+            selection.end,
+            selection.reversed
+        );
+        Some(selection)
+    })
+}

crates/vim/src/vim.rs

@@ -822,7 +822,7 @@ impl Vim {
         editor.set_collapse_matches(false);
         editor.set_input_enabled(true);
         editor.set_autoindent(true);
-        editor.selections.line_mode = false;
+        editor.selections.set_line_mode(false);
         editor.unregister_addon::<VimAddon>();
         editor.set_relative_line_number(None, cx);
         if let Some(vim) = Vim::globals(cx).focused_vim()
@@ -1787,7 +1787,9 @@ impl Vim {
             editor.set_collapse_matches(true);
             editor.set_input_enabled(vim.editor_input_enabled());
             editor.set_autoindent(vim.should_autoindent());
-            editor.selections.line_mode = matches!(vim.mode, Mode::VisualLine);
+            editor
+                .selections
+                .set_line_mode(matches!(vim.mode, Mode::VisualLine));
 
             let hide_edit_predictions = !matches!(vim.mode, Mode::Insert | Mode::Replace);
             editor.set_edit_predictions_hidden_for_vim_mode(hide_edit_predictions, window, cx);

crates/vim/src/visual.rs

@@ -609,8 +609,8 @@ impl Vim {
         self.store_visual_marks(window, cx);
         self.update_editor(cx, |vim, editor, cx| {
             let mut original_columns: HashMap<_, _> = Default::default();
-            let line_mode = line_mode || editor.selections.line_mode;
-            editor.selections.line_mode = false;
+            let line_mode = line_mode || editor.selections.line_mode();
+            editor.selections.set_line_mode(false);
 
             editor.transact(window, cx, |editor, window, cx| {
                 editor.change_selections(Default::default(), window, cx, |s| {
@@ -692,7 +692,7 @@ impl Vim {
     pub fn visual_yank(&mut self, line_mode: bool, window: &mut Window, cx: &mut Context<Self>) {
         self.store_visual_marks(window, cx);
         self.update_editor(cx, |vim, editor, cx| {
-            let line_mode = line_mode || editor.selections.line_mode;
+            let line_mode = line_mode || editor.selections.line_mode();
 
             // For visual line mode, adjust selections to avoid yanking the next line when on \n
             if line_mode && vim.mode != Mode::VisualBlock {
@@ -710,7 +710,7 @@ impl Vim {
                 });
             }
 
-            editor.selections.line_mode = line_mode;
+            editor.selections.set_line_mode(line_mode);
             let kind = if line_mode {
                 MotionKind::Linewise
             } else {