Merge pull request #1910 from zed-industries/lsp-coordinate-clamp

Don't trust LSP coordinates to be within document bounds

crates/diagnostics/src/diagnostics.rs

@@ -738,7 +738,7 @@ mod tests {
         DisplayPoint,
     };
     use gpui::TestAppContext;
-    use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, PointUtf16};
+    use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, PointUtf16, Unclipped};
     use serde_json::json;
     use unindent::Unindent as _;
     use workspace::AppState;
@@ -788,7 +788,7 @@ mod tests {
                     None,
                     vec![
                         DiagnosticEntry {
-                            range: PointUtf16::new(1, 8)..PointUtf16::new(1, 9),
+                            range: Unclipped(PointUtf16::new(1, 8))..Unclipped(PointUtf16::new(1, 9)),
                             diagnostic: Diagnostic {
                                 message:
                                     "move occurs because `x` has type `Vec<char>`, which does not implement the `Copy` trait"
@@ -801,7 +801,7 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(2, 8)..PointUtf16::new(2, 9),
+                            range: Unclipped(PointUtf16::new(2, 8))..Unclipped(PointUtf16::new(2, 9)),
                             diagnostic: Diagnostic {
                                 message:
                                     "move occurs because `y` has type `Vec<char>`, which does not implement the `Copy` trait"
@@ -814,7 +814,7 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(3, 6)..PointUtf16::new(3, 7),
+                            range: Unclipped(PointUtf16::new(3, 6))..Unclipped(PointUtf16::new(3, 7)),
                             diagnostic: Diagnostic {
                                 message: "value moved here".to_string(),
                                 severity: DiagnosticSeverity::INFORMATION,
@@ -825,7 +825,7 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(4, 6)..PointUtf16::new(4, 7),
+                            range: Unclipped(PointUtf16::new(4, 6))..Unclipped(PointUtf16::new(4, 7)),
                             diagnostic: Diagnostic {
                                 message: "value moved here".to_string(),
                                 severity: DiagnosticSeverity::INFORMATION,
@@ -836,7 +836,7 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(7, 6)..PointUtf16::new(7, 7),
+                            range: Unclipped(PointUtf16::new(7, 6))..Unclipped(PointUtf16::new(7, 7)),
                             diagnostic: Diagnostic {
                                 message: "use of moved value\nvalue used here after move".to_string(),
                                 severity: DiagnosticSeverity::ERROR,
@@ -847,7 +847,7 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(8, 6)..PointUtf16::new(8, 7),
+                            range: Unclipped(PointUtf16::new(8, 6))..Unclipped(PointUtf16::new(8, 7)),
                             diagnostic: Diagnostic {
                                 message: "use of moved value\nvalue used here after move".to_string(),
                                 severity: DiagnosticSeverity::ERROR,
@@ -939,7 +939,7 @@ mod tests {
                     PathBuf::from("/test/consts.rs"),
                     None,
                     vec![DiagnosticEntry {
-                        range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15),
+                        range: Unclipped(PointUtf16::new(0, 15))..Unclipped(PointUtf16::new(0, 15)),
                         diagnostic: Diagnostic {
                             message: "mismatched types\nexpected `usize`, found `char`".to_string(),
                             severity: DiagnosticSeverity::ERROR,
@@ -1040,7 +1040,8 @@ mod tests {
                     None,
                     vec![
                         DiagnosticEntry {
-                            range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15),
+                            range: Unclipped(PointUtf16::new(0, 15))
+                                ..Unclipped(PointUtf16::new(0, 15)),
                             diagnostic: Diagnostic {
                                 message: "mismatched types\nexpected `usize`, found `char`"
                                     .to_string(),
@@ -1052,7 +1053,8 @@ mod tests {
                             },
                         },
                         DiagnosticEntry {
-                            range: PointUtf16::new(1, 15)..PointUtf16::new(1, 15),
+                            range: Unclipped(PointUtf16::new(1, 15))
+                                ..Unclipped(PointUtf16::new(1, 15)),
                             diagnostic: Diagnostic {
                                 message: "unresolved name `c`".to_string(),
                                 severity: DiagnosticSeverity::ERROR,

crates/editor/src/multi_buffer.rs

@@ -11,7 +11,7 @@ use language::{
     char_kind, AutoindentMode, Buffer, BufferChunks, BufferSnapshot, CharKind, Chunk, CursorShape,
     DiagnosticEntry, Event, File, IndentSize, Language, OffsetRangeExt, OffsetUtf16, Outline,
     OutlineItem, Point, PointUtf16, Selection, TextDimension, ToOffset as _, ToOffsetUtf16 as _,
-    ToPoint as _, ToPointUtf16 as _, TransactionId,
+    ToPoint as _, ToPointUtf16 as _, TransactionId, Unclipped,
 };
 use smallvec::SmallVec;
 use std::{
@@ -1749,20 +1749,20 @@ impl MultiBufferSnapshot {
         *cursor.start() + overshoot
     }
 
-    pub fn clip_point_utf16(&self, point: PointUtf16, bias: Bias) -> PointUtf16 {
+    pub fn clip_point_utf16(&self, point: Unclipped<PointUtf16>, bias: Bias) -> PointUtf16 {
         if let Some((_, _, buffer)) = self.as_singleton() {
             return buffer.clip_point_utf16(point, bias);
         }
 
         let mut cursor = self.excerpts.cursor::<PointUtf16>();
-        cursor.seek(&point, Bias::Right, &());
+        cursor.seek(&point.0, Bias::Right, &());
         let overshoot = if let Some(excerpt) = cursor.item() {
             let excerpt_start = excerpt
                 .buffer
                 .offset_to_point_utf16(excerpt.range.context.start.to_offset(&excerpt.buffer));
             let buffer_point = excerpt
                 .buffer
-                .clip_point_utf16(excerpt_start + (point - cursor.start()), bias);
+                .clip_point_utf16(Unclipped(excerpt_start + (point.0 - cursor.start())), bias);
             buffer_point.saturating_sub(excerpt_start)
         } else {
             PointUtf16::zero()
@@ -3274,12 +3274,6 @@ impl ToOffset for Point {
     }
 }
 
-impl ToOffset for PointUtf16 {
-    fn to_offset<'a>(&self, snapshot: &MultiBufferSnapshot) -> usize {
-        snapshot.point_utf16_to_offset(*self)
-    }
-}
-
 impl ToOffset for usize {
     fn to_offset<'a>(&self, snapshot: &MultiBufferSnapshot) -> usize {
         assert!(*self <= snapshot.len(), "offset is out of range");
@@ -3293,6 +3287,12 @@ impl ToOffset for OffsetUtf16 {
     }
 }
 
+impl ToOffset for PointUtf16 {
+    fn to_offset<'a>(&self, snapshot: &MultiBufferSnapshot) -> usize {
+        snapshot.point_utf16_to_offset(*self)
+    }
+}
+
 impl ToOffsetUtf16 for OffsetUtf16 {
     fn to_offset_utf16(&self, _snapshot: &MultiBufferSnapshot) -> OffsetUtf16 {
         *self
@@ -4158,12 +4158,14 @@ mod tests {
                     }
 
                     for _ in 0..ch.len_utf16() {
-                        let left_point_utf16 = snapshot.clip_point_utf16(point_utf16, Bias::Left);
+                        let left_point_utf16 =
-                        let right_point_utf16 = snapshot.clip_point_utf16(point_utf16, Bias::Right);
+                            snapshot.clip_point_utf16(Unclipped(point_utf16), Bias::Left);
+                        let right_point_utf16 =
+                            snapshot.clip_point_utf16(Unclipped(point_utf16), Bias::Right);
                         let buffer_left_point_utf16 =
-                            buffer.clip_point_utf16(buffer_point_utf16, Bias::Left);
+                            buffer.clip_point_utf16(Unclipped(buffer_point_utf16), Bias::Left);
                         let buffer_right_point_utf16 =
-                            buffer.clip_point_utf16(buffer_point_utf16, Bias::Right);
+                            buffer.clip_point_utf16(Unclipped(buffer_point_utf16), Bias::Right);
                         assert_eq!(
                             left_point_utf16,
                             excerpt_start.lines_utf16()

crates/editor/src/selections_collection.rs

@@ -544,11 +544,21 @@ impl<'a> MutableSelectionsCollection<'a> {
         T: ToOffset,
     {
         let buffer = self.buffer.read(self.cx).snapshot(self.cx);
+        let ranges = ranges
+            .into_iter()
+            .map(|range| range.start.to_offset(&buffer)..range.end.to_offset(&buffer));
+        self.select_offset_ranges(ranges);
+    }
+
+    fn select_offset_ranges<I>(&mut self, ranges: I)
+    where
+        I: IntoIterator<Item = Range<usize>>,
+    {
         let selections = ranges
             .into_iter()
             .map(|range| {
-                let mut start = range.start.to_offset(&buffer);
+                let mut start = range.start;
-                let mut end = range.end.to_offset(&buffer);
+                let mut end = range.end;
                 let reversed = if start > end {
                     mem::swap(&mut start, &mut end);
                     true

crates/language/src/buffer_tests.rs

@@ -1337,6 +1337,7 @@ fn test_random_collaboration(cx: &mut MutableAppContext, mut rng: StdRng) {
                         (0..entry_count).map(|_| {
                             let range = buffer.random_byte_range(0, &mut rng);
                             let range = range.to_point_utf16(buffer);
+                            let range = range.start..range.end;
                             DiagnosticEntry {
                                 range,
                                 diagnostic: Diagnostic {

crates/language/src/diagnostic_set.rs

@@ -71,7 +71,7 @@ impl DiagnosticSet {
             diagnostics: SumTree::from_iter(
                 entries.into_iter().map(|entry| DiagnosticEntry {
                     range: buffer.anchor_before(entry.range.start)
-                        ..buffer.anchor_after(entry.range.end),
+                        ..buffer.anchor_before(entry.range.end),
                     diagnostic: entry.diagnostic,
                 }),
                 buffer,

crates/language/src/language.rs

@@ -1053,8 +1053,8 @@ pub fn point_to_lsp(point: PointUtf16) -> lsp::Position {
     lsp::Position::new(point.row, point.column)
 }
 
-pub fn point_from_lsp(point: lsp::Position) -> PointUtf16 {
+pub fn point_from_lsp(point: lsp::Position) -> Unclipped<PointUtf16> {
-    PointUtf16::new(point.line, point.character)
+    Unclipped(PointUtf16::new(point.line, point.character))
 }
 
 pub fn range_to_lsp(range: Range<PointUtf16>) -> lsp::Range {
@@ -1064,7 +1064,7 @@ pub fn range_to_lsp(range: Range<PointUtf16>) -> lsp::Range {
     }
 }
 
-pub fn range_from_lsp(range: lsp::Range) -> Range<PointUtf16> {
+pub fn range_from_lsp(range: lsp::Range) -> Range<Unclipped<PointUtf16>> {
     let mut start = point_from_lsp(range.start);
     let mut end = point_from_lsp(range.end);
     if start > end {

crates/project/src/lsp_command.rs

@@ -128,8 +128,8 @@ impl LspCommand for PrepareRename {
             ) = message
             {
                 let Range { start, end } = range_from_lsp(range);
-                if buffer.clip_point_utf16(start, Bias::Left) == start
+                if buffer.clip_point_utf16(start, Bias::Left) == start.0
-                    && buffer.clip_point_utf16(end, Bias::Left) == end
+                    && buffer.clip_point_utf16(end, Bias::Left) == end.0
                 {
                     return Ok(Some(buffer.anchor_after(start)..buffer.anchor_before(end)));
                 }

crates/project/src/project.rs

@@ -26,6 +26,7 @@ use language::{
     CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent,
     File as _, Language, LanguageRegistry, LanguageServerName, LocalFile, OffsetRangeExt,
     Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction,
+    Unclipped,
 };
 use lsp::{
     DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer, LanguageString,
@@ -252,7 +253,7 @@ pub struct Symbol {
     pub label: CodeLabel,
     pub name: String,
     pub kind: lsp::SymbolKind,
-    pub range: Range<PointUtf16>,
+    pub range: Range<Unclipped<PointUtf16>>,
     pub signature: [u8; 32],
 }
 
@@ -2597,7 +2598,7 @@ impl Project {
         language_server_id: usize,
         abs_path: PathBuf,
         version: Option<i32>,
-        diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
+        diagnostics: Vec<DiagnosticEntry<Unclipped<PointUtf16>>>,
         cx: &mut ModelContext<Project>,
     ) -> Result<(), anyhow::Error> {
         let (worktree, relative_path) = self
@@ -2635,7 +2636,7 @@ impl Project {
     fn update_buffer_diagnostics(
         &mut self,
         buffer: &ModelHandle<Buffer>,
-        mut diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
+        mut diagnostics: Vec<DiagnosticEntry<Unclipped<PointUtf16>>>,
         version: Option<i32>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
@@ -2659,7 +2660,7 @@ impl Project {
         let mut sanitized_diagnostics = Vec::new();
         let edits_since_save = Patch::new(
             snapshot
-                .edits_since::<PointUtf16>(buffer.read(cx).saved_version())
+                .edits_since::<Unclipped<PointUtf16>>(buffer.read(cx).saved_version())
                 .collect(),
         );
         for entry in diagnostics {
@@ -2679,13 +2680,14 @@ impl Project {
             let mut range = snapshot.clip_point_utf16(start, Bias::Left)
                 ..snapshot.clip_point_utf16(end, Bias::Right);
 
-            // Expand empty ranges by one character
+            // Expand empty ranges by one codepoint
             if range.start == range.end {
+                // This will be go to the next boundary when being clipped
                 range.end.column += 1;
-                range.end = snapshot.clip_point_utf16(range.end, Bias::Right);
+                range.end = snapshot.clip_point_utf16(Unclipped(range.end), Bias::Right);
                 if range.start == range.end && range.end.column > 0 {
                     range.start.column -= 1;
-                    range.start = snapshot.clip_point_utf16(range.start, Bias::Left);
+                    range.end = snapshot.clip_point_utf16(Unclipped(range.end), Bias::Left);
                 }
             }
 
@@ -3288,7 +3290,7 @@ impl Project {
             return Task::ready(Ok(Default::default()));
         };
 
-        let position = position.to_point_utf16(source_buffer);
+        let position = Unclipped(position.to_point_utf16(source_buffer));
         let anchor = source_buffer.anchor_after(position);
 
         if worktree.read(cx).as_local().is_some() {
@@ -3307,7 +3309,7 @@ impl Project {
                             lsp::TextDocumentIdentifier::new(
                                 lsp::Url::from_file_path(buffer_abs_path).unwrap(),
                             ),
-                            point_to_lsp(position),
+                            point_to_lsp(position.0),
                         ),
                         context: Default::default(),
                         work_done_progress_params: Default::default(),
@@ -3350,7 +3352,7 @@ impl Project {
                                     let range = range_from_lsp(edit.range);
                                     let start = snapshot.clip_point_utf16(range.start, Bias::Left);
                                     let end = snapshot.clip_point_utf16(range.end, Bias::Left);
-                                    if start != range.start || end != range.end {
+                                    if start != range.start.0 || end != range.end.0 {
                                         log::info!("completion out of expected range");
                                         return None;
                                     }
@@ -3362,7 +3364,7 @@ impl Project {
                                 // If the language server does not provide a range, then infer
                                 // the range based on the syntax tree.
                                 None => {
-                                    if position != clipped_position {
+                                    if position.0 != clipped_position {
                                         log::info!("completion out of expected range");
                                         return None;
                                     }
@@ -5117,22 +5119,30 @@ impl Project {
         _: Arc<Client>,
         mut cx: AsyncAppContext,
     ) -> Result<proto::GetCompletionsResponse> {
-        let position = envelope
-            .payload
-            .position
-            .and_then(language::proto::deserialize_anchor)
-            .ok_or_else(|| anyhow!("invalid position"))?;
-        let version = deserialize_version(envelope.payload.version);
         let buffer = this.read_with(&cx, |this, cx| {
             this.opened_buffers
                 .get(&envelope.payload.buffer_id)
                 .and_then(|buffer| buffer.upgrade(cx))
                 .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))
         })?;
+
+        let position = envelope
+            .payload
+            .position
+            .and_then(language::proto::deserialize_anchor)
+            .map(|p| {
+                buffer.read_with(&cx, |buffer, _| {
+                    buffer.clip_point_utf16(Unclipped(p.to_point_utf16(buffer)), Bias::Left)
+                })
+            })
+            .ok_or_else(|| anyhow!("invalid position"))?;
+
+        let version = deserialize_version(envelope.payload.version);
         buffer
             .update(&mut cx, |buffer, _| buffer.wait_for_version(version))
             .await;
         let version = buffer.read_with(&cx, |buffer, _| buffer.version());
+
         let completions = this
             .update(&mut cx, |this, cx| this.completions(&buffer, position, cx))
             .await?;
@@ -5619,8 +5629,8 @@ impl Project {
                 },
 
                 name: serialized_symbol.name,
-                range: PointUtf16::new(start.row, start.column)
+                range: Unclipped(PointUtf16::new(start.row, start.column))
-                    ..PointUtf16::new(end.row, end.column),
+                    ..Unclipped(PointUtf16::new(end.row, end.column)),
                 kind,
                 signature: serialized_symbol
                     .signature
@@ -5706,10 +5716,10 @@ impl Project {
 
             let mut lsp_edits = lsp_edits.into_iter().peekable();
             let mut edits = Vec::new();
-            while let Some((mut range, mut new_text)) = lsp_edits.next() {
+            while let Some((range, mut new_text)) = lsp_edits.next() {
                 // Clip invalid ranges provided by the language server.
-                range.start = snapshot.clip_point_utf16(range.start, Bias::Left);
+                let mut range = snapshot.clip_point_utf16(range.start, Bias::Left)
-                range.end = snapshot.clip_point_utf16(range.end, Bias::Left);
+                    ..snapshot.clip_point_utf16(range.end, Bias::Left);
 
                 // Combine any LSP edits that are adjacent.
                 //
@@ -5721,11 +5731,11 @@ impl Project {
                 // In order for the diffing logic below to work properly, any edits that
                 // cancel each other out must be combined into one.
                 while let Some((next_range, next_text)) = lsp_edits.peek() {
-                    if next_range.start > range.end {
+                    if next_range.start.0 > range.end {
-                        if next_range.start.row > range.end.row + 1
+                        if next_range.start.0.row > range.end.row + 1
-                            || next_range.start.column > 0
+                            || next_range.start.0.column > 0
                             || snapshot.clip_point_utf16(
-                                PointUtf16::new(range.end.row, u32::MAX),
+                                Unclipped(PointUtf16::new(range.end.row, u32::MAX)),
                                 Bias::Left,
                             ) > range.end
                         {
@@ -5733,7 +5743,7 @@ impl Project {
                         }
                         new_text.push('\n');
                     }
-                    range.end = next_range.end;
+                    range.end = snapshot.clip_point_utf16(next_range.end, Bias::Left);
                     new_text.push_str(next_text);
                     lsp_edits.next();
                 }
@@ -6054,13 +6064,13 @@ fn serialize_symbol(symbol: &Symbol) -> proto::Symbol {
         path: symbol.path.path.to_string_lossy().to_string(),
         name: symbol.name.clone(),
         kind: unsafe { mem::transmute(symbol.kind) },
-        start: Some(proto::Point {
+        start: Some(proto::PointUtf16 {
-            row: symbol.range.start.row,
+            row: symbol.range.start.0.row,
-            column: symbol.range.start.column,
+            column: symbol.range.start.0.column,
         }),
-        end: Some(proto::Point {
+        end: Some(proto::PointUtf16 {
-            row: symbol.range.end.row,
+            row: symbol.range.end.0.row,
-            column: symbol.range.end.column,
+            column: symbol.range.end.0.column,
         }),
         signature: symbol.signature.to_vec(),
     }

crates/project/src/project_tests.rs

@@ -1239,7 +1239,7 @@ async fn test_empty_diagnostic_ranges(cx: &mut gpui::TestAppContext) {
                 &buffer,
                 vec![
                     DiagnosticEntry {
-                        range: PointUtf16::new(0, 10)..PointUtf16::new(0, 10),
+                        range: Unclipped(PointUtf16::new(0, 10))..Unclipped(PointUtf16::new(0, 10)),
                         diagnostic: Diagnostic {
                             severity: DiagnosticSeverity::ERROR,
                             message: "syntax error 1".to_string(),
@@ -1247,7 +1247,7 @@ async fn test_empty_diagnostic_ranges(cx: &mut gpui::TestAppContext) {
                         },
                     },
                     DiagnosticEntry {
-                        range: PointUtf16::new(1, 10)..PointUtf16::new(1, 10),
+                        range: Unclipped(PointUtf16::new(1, 10))..Unclipped(PointUtf16::new(1, 10)),
                         diagnostic: Diagnostic {
                             severity: DiagnosticSeverity::ERROR,
                             message: "syntax error 2".to_string(),

crates/project/src/worktree.rs

@@ -20,6 +20,7 @@ use gpui::{
     executor, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext,
     Task,
 };
+use language::Unclipped;
 use language::{
     proto::{deserialize_version, serialize_line_ending, serialize_version},
     Buffer, DiagnosticEntry, PointUtf16, Rope,
@@ -65,7 +66,7 @@ pub struct LocalWorktree {
     _background_scanner_task: Option<Task<()>>,
     poll_task: Option<Task<()>>,
     share: Option<ShareState>,
-    diagnostics: HashMap<Arc<Path>, Vec<DiagnosticEntry<PointUtf16>>>,
+    diagnostics: HashMap<Arc<Path>, Vec<DiagnosticEntry<Unclipped<PointUtf16>>>>,
     diagnostic_summaries: TreeMap<PathKey, DiagnosticSummary>,
     client: Arc<Client>,
     fs: Arc<dyn Fs>,
@@ -499,7 +500,10 @@ impl LocalWorktree {
         })
     }
 
-    pub fn diagnostics_for_path(&self, path: &Path) -> Option<Vec<DiagnosticEntry<PointUtf16>>> {
+    pub fn diagnostics_for_path(
+        &self,
+        path: &Path,
+    ) -> Option<Vec<DiagnosticEntry<Unclipped<PointUtf16>>>> {
         self.diagnostics.get(path).cloned()
     }
 
@@ -507,7 +511,7 @@ impl LocalWorktree {
         &mut self,
         language_server_id: usize,
         worktree_path: Arc<Path>,
-        diagnostics: Vec<DiagnosticEntry<PointUtf16>>,
+        diagnostics: Vec<DiagnosticEntry<Unclipped<PointUtf16>>>,
         _: &mut ModelContext<Worktree>,
     ) -> Result<bool> {
         self.diagnostics.remove(&worktree_path);

crates/rope/src/rope.rs

@@ -1,16 +1,22 @@
 mod offset_utf16;
 mod point;
 mod point_utf16;
+mod unclipped;
 
 use arrayvec::ArrayString;
 use bromberg_sl2::{DigestString, HashMatrix};
 use smallvec::SmallVec;
-use std::{cmp, fmt, io, mem, ops::Range, str};
+use std::{
+    cmp, fmt, io, mem,
+    ops::{AddAssign, Range},
+    str,
+};
 use sum_tree::{Bias, Dimension, SumTree};
 
 pub use offset_utf16::OffsetUtf16;
 pub use point::Point;
 pub use point_utf16::PointUtf16;
+pub use unclipped::Unclipped;
 
 #[cfg(test)]
 const CHUNK_BASE: usize = 6;
@@ -260,6 +266,14 @@ impl Rope {
     }
 
     pub fn point_utf16_to_offset(&self, point: PointUtf16) -> usize {
+        self.point_utf16_to_offset_impl(point, false)
+    }
+
+    pub fn unclipped_point_utf16_to_offset(&self, point: Unclipped<PointUtf16>) -> usize {
+        self.point_utf16_to_offset_impl(point.0, true)
+    }
+
+    fn point_utf16_to_offset_impl(&self, point: PointUtf16, clip: bool) -> usize {
         if point >= self.summary().lines_utf16() {
             return self.summary().len;
         }
@@ -269,20 +283,20 @@ impl Rope {
         cursor.start().1
             + cursor
                 .item()
-                .map_or(0, |chunk| chunk.point_utf16_to_offset(overshoot))
+                .map_or(0, |chunk| chunk.point_utf16_to_offset(overshoot, clip))
     }
 
-    pub fn point_utf16_to_point(&self, point: PointUtf16) -> Point {
+    pub fn unclipped_point_utf16_to_point(&self, point: Unclipped<PointUtf16>) -> Point {
-        if point >= self.summary().lines_utf16() {
+        if point.0 >= self.summary().lines_utf16() {
             return self.summary().lines;
         }
         let mut cursor = self.chunks.cursor::<(PointUtf16, Point)>();
-        cursor.seek(&point, Bias::Left, &());
+        cursor.seek(&point.0, Bias::Left, &());
-        let overshoot = point - cursor.start().0;
+        let overshoot = Unclipped(point.0 - cursor.start().0);
         cursor.start().1
-            + cursor
+            + cursor.item().map_or(Point::zero(), |chunk| {
-                .item()
+                chunk.unclipped_point_utf16_to_point(overshoot)
-                .map_or(Point::zero(), |chunk| chunk.point_utf16_to_point(overshoot))
+            })
     }
 
     pub fn clip_offset(&self, mut offset: usize, bias: Bias) -> usize {
@@ -330,11 +344,11 @@ impl Rope {
         }
     }
 
-    pub fn clip_point_utf16(&self, point: PointUtf16, bias: Bias) -> PointUtf16 {
+    pub fn clip_point_utf16(&self, point: Unclipped<PointUtf16>, bias: Bias) -> PointUtf16 {
         let mut cursor = self.chunks.cursor::<PointUtf16>();
-        cursor.seek(&point, Bias::Right, &());
+        cursor.seek(&point.0, Bias::Right, &());
         if let Some(chunk) = cursor.item() {
-            let overshoot = point - cursor.start();
+            let overshoot = Unclipped(point.0 - cursor.start());
             *cursor.start() + chunk.clip_point_utf16(overshoot, bias)
         } else {
             self.summary().lines_utf16()
@@ -711,45 +725,61 @@ impl Chunk {
         point_utf16
     }
 
-    fn point_utf16_to_offset(&self, target: PointUtf16) -> usize {
+    fn point_utf16_to_offset(&self, target: PointUtf16, clip: bool) -> usize {
         let mut offset = 0;
         let mut point = PointUtf16::new(0, 0);
+
         for ch in self.0.chars() {
-            if point >= target {
+            if point == target {
-                if point > target {
-                    panic!("point {:?} is inside of character {:?}", target, ch);
-                }
                 break;
             }
 
             if ch == '\n' {
                 point.row += 1;
+                point.column = 0;
                 if point.row > target.row {
+                    if clip {
+                        // Return the offset of the newline
+                        return offset;
+                    }
                     panic!(
                         "point {:?} is beyond the end of a line with length {}",
                         target, point.column
                     );
                 }
-                point.column = 0;
             } else {
                 point.column += ch.len_utf16() as u32;
             }
+
+            if point > target {
+                if clip {
+                    // Return the offset of the codepoint which we have landed within, bias left
+                    return offset;
+                }
+                panic!("point {:?} is inside of codepoint {:?}", target, ch);
+            }
+
             offset += ch.len_utf8();
         }
+
         offset
     }
 
-    fn point_utf16_to_point(&self, target: PointUtf16) -> Point {
+    fn unclipped_point_utf16_to_point(&self, target: Unclipped<PointUtf16>) -> Point {
         let mut point = Point::zero();
         let mut point_utf16 = PointUtf16::zero();
+
         for ch in self.0.chars() {
-            if point_utf16 >= target {
+            if point_utf16 == target.0 {
-                if point_utf16 > target {
-                    panic!("point {:?} is inside of character {:?}", target, ch);
-                }
                 break;
             }
 
+            if point_utf16 > target.0 {
+                // If the point is past the end of a line or inside of a code point,
+                // return the last valid point before the target.
+                return point;
+            }
+
             if ch == '\n' {
                 point_utf16 += PointUtf16::new(1, 0);
                 point += Point::new(1, 0);
@@ -758,6 +788,7 @@ impl Chunk {
                 point += Point::new(0, ch.len_utf8() as u32);
             }
         }
+
         point
     }
 
@@ -777,11 +808,11 @@ impl Chunk {
         unreachable!()
     }
 
-    fn clip_point_utf16(&self, target: PointUtf16, bias: Bias) -> PointUtf16 {
+    fn clip_point_utf16(&self, target: Unclipped<PointUtf16>, bias: Bias) -> PointUtf16 {
         for (row, line) in self.0.split('\n').enumerate() {
-            if row == target.row as usize {
+            if row == target.0.row as usize {
                 let mut code_units = line.encode_utf16();
-                let mut column = code_units.by_ref().take(target.column as usize).count();
+                let mut column = code_units.by_ref().take(target.0.column as usize).count();
                 if char::decode_utf16(code_units).next().transpose().is_err() {
                     match bias {
                         Bias::Left => column -= 1,
@@ -917,7 +948,7 @@ impl std::ops::Add<Self> for TextSummary {
     type Output = Self;
 
     fn add(mut self, rhs: Self) -> Self::Output {
-        self.add_assign(&rhs);
+        AddAssign::add_assign(&mut self, &rhs);
         self
     }
 }
@@ -1114,15 +1145,15 @@ mod tests {
         );
 
         assert_eq!(
-            rope.clip_point_utf16(PointUtf16::new(0, 1), Bias::Left),
+            rope.clip_point_utf16(Unclipped(PointUtf16::new(0, 1)), Bias::Left),
             PointUtf16::new(0, 0)
         );
         assert_eq!(
-            rope.clip_point_utf16(PointUtf16::new(0, 1), Bias::Right),
+            rope.clip_point_utf16(Unclipped(PointUtf16::new(0, 1)), Bias::Right),
             PointUtf16::new(0, 2)
         );
         assert_eq!(
-            rope.clip_point_utf16(PointUtf16::new(0, 3), Bias::Right),
+            rope.clip_point_utf16(Unclipped(PointUtf16::new(0, 3)), Bias::Right),
             PointUtf16::new(0, 2)
         );
 
@@ -1238,7 +1269,7 @@ mod tests {
             }
 
             let mut offset_utf16 = OffsetUtf16(0);
-            let mut point_utf16 = PointUtf16::zero();
+            let mut point_utf16 = Unclipped(PointUtf16::zero());
             for unit in expected.encode_utf16() {
                 let left_offset = actual.clip_offset_utf16(offset_utf16, Bias::Left);
                 let right_offset = actual.clip_offset_utf16(offset_utf16, Bias::Right);
@@ -1250,15 +1281,15 @@ mod tests {
                 let left_point = actual.clip_point_utf16(point_utf16, Bias::Left);
                 let right_point = actual.clip_point_utf16(point_utf16, Bias::Right);
                 assert!(right_point >= left_point);
-                // Ensure translating UTF-16 points to offsets doesn't panic.
+                // Ensure translating valid UTF-16 points to offsets doesn't panic.
                 actual.point_utf16_to_offset(left_point);
                 actual.point_utf16_to_offset(right_point);
 
                 offset_utf16.0 += 1;
                 if unit == b'\n' as u16 {
-                    point_utf16 += PointUtf16::new(1, 0);
+                    point_utf16.0 += PointUtf16::new(1, 0);
                 } else {
-                    point_utf16 += PointUtf16::new(0, 1);
+                    point_utf16.0 += PointUtf16::new(0, 1);
                 }
             }
 

crates/rope/src/unclipped.rs

@@ -0,0 +1,57 @@
+use crate::{ChunkSummary, TextDimension, TextSummary};
+use std::ops::{Add, AddAssign, Sub, SubAssign};
+
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct Unclipped<T>(pub T);
+
+impl<T> From<T> for Unclipped<T> {
+    fn from(value: T) -> Self {
+        Unclipped(value)
+    }
+}
+
+impl<'a, T: sum_tree::Dimension<'a, ChunkSummary>> sum_tree::Dimension<'a, ChunkSummary>
+    for Unclipped<T>
+{
+    fn add_summary(&mut self, summary: &'a ChunkSummary, _: &()) {
+        self.0.add_summary(summary, &());
+    }
+}
+
+impl<T: TextDimension> TextDimension for Unclipped<T> {
+    fn from_text_summary(summary: &TextSummary) -> Self {
+        Unclipped(T::from_text_summary(summary))
+    }
+
+    fn add_assign(&mut self, other: &Self) {
+        TextDimension::add_assign(&mut self.0, &other.0);
+    }
+}
+
+impl<T: Add<T, Output = T>> Add<Unclipped<T>> for Unclipped<T> {
+    type Output = Unclipped<T>;
+
+    fn add(self, rhs: Unclipped<T>) -> Self::Output {
+        Unclipped(self.0 + rhs.0)
+    }
+}
+
+impl<T: Sub<T, Output = T>> Sub<Unclipped<T>> for Unclipped<T> {
+    type Output = Unclipped<T>;
+
+    fn sub(self, rhs: Unclipped<T>) -> Self::Output {
+        Unclipped(self.0 - rhs.0)
+    }
+}
+
+impl<T: AddAssign<T>> AddAssign<Unclipped<T>> for Unclipped<T> {
+    fn add_assign(&mut self, rhs: Unclipped<T>) {
+        self.0 += rhs.0;
+    }
+}
+
+impl<T: SubAssign<T>> SubAssign<Unclipped<T>> for Unclipped<T> {
+    fn sub_assign(&mut self, rhs: Unclipped<T>) {
+        self.0 -= rhs.0;
+    }
+}

crates/rpc/proto/zed.proto

@@ -412,8 +412,10 @@ message Symbol {
     string name = 4;
     int32 kind = 5;
     string path = 6;
-    Point start = 7;
+    // Cannot use generate anchors for unopend files,
-    Point end = 8;
+    // so we are forced to use point coords instead
+    PointUtf16 start = 7;
+    PointUtf16 end = 8;
     bytes signature = 9;
 }
 
@@ -1042,7 +1044,7 @@ message Range {
     uint64 end = 2;
 }
 
-message Point {
+message PointUtf16 {
     uint32 row = 1;
     uint32 column = 2;
 }

crates/text/src/text.rs

@@ -1594,8 +1594,12 @@ impl BufferSnapshot {
         self.visible_text.point_utf16_to_offset(point)
     }
 
-    pub fn point_utf16_to_point(&self, point: PointUtf16) -> Point {
+    pub fn unclipped_point_utf16_to_offset(&self, point: Unclipped<PointUtf16>) -> usize {
-        self.visible_text.point_utf16_to_point(point)
+        self.visible_text.unclipped_point_utf16_to_offset(point)
+    }
+
+    pub fn unclipped_point_utf16_to_point(&self, point: Unclipped<PointUtf16>) -> Point {
+        self.visible_text.unclipped_point_utf16_to_point(point)
     }
 
     pub fn offset_utf16_to_offset(&self, offset: OffsetUtf16) -> usize {
@@ -1803,7 +1807,10 @@ impl BufferSnapshot {
     }
 
     pub fn anchor_at<T: ToOffset>(&self, position: T, bias: Bias) -> Anchor {
-        let offset = position.to_offset(self);
+        self.anchor_at_offset(position.to_offset(self), bias)
+    }
+
+    fn anchor_at_offset(&self, offset: usize, bias: Bias) -> Anchor {
         if bias == Bias::Left && offset == 0 {
             Anchor::MIN
         } else if bias == Bias::Right && offset == self.len() {
@@ -1840,7 +1847,7 @@ impl BufferSnapshot {
         self.visible_text.clip_offset_utf16(offset, bias)
     }
 
-    pub fn clip_point_utf16(&self, point: PointUtf16, bias: Bias) -> PointUtf16 {
+    pub fn clip_point_utf16(&self, point: Unclipped<PointUtf16>, bias: Bias) -> PointUtf16 {
         self.visible_text.clip_point_utf16(point, bias)
     }
 
@@ -2354,32 +2361,20 @@ pub trait ToOffset {
 }
 
 impl ToOffset for Point {
-    fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize {
+    fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
         snapshot.point_to_offset(*self)
     }
 }
 
-impl ToOffset for PointUtf16 {
-    fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize {
-        snapshot.point_utf16_to_offset(*self)
-    }
-}
-
 impl ToOffset for usize {
-    fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize {
+    fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
         assert!(*self <= snapshot.len(), "offset {self} is out of range");
         *self
     }
 }
 
-impl ToOffset for OffsetUtf16 {
-    fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize {
-        snapshot.offset_utf16_to_offset(*self)
-    }
-}
-
 impl ToOffset for Anchor {
-    fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize {
+    fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
         snapshot.summary_for_anchor(self)
     }
 }
@@ -2390,31 +2385,43 @@ impl<'a, T: ToOffset> ToOffset for &'a T {
     }
 }
 
+impl ToOffset for PointUtf16 {
+    fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
+        snapshot.point_utf16_to_offset(*self)
+    }
+}
+
+impl ToOffset for Unclipped<PointUtf16> {
+    fn to_offset(&self, snapshot: &BufferSnapshot) -> usize {
+        snapshot.unclipped_point_utf16_to_offset(*self)
+    }
+}
+
 pub trait ToPoint {
     fn to_point(&self, snapshot: &BufferSnapshot) -> Point;
 }
 
 impl ToPoint for Anchor {
-    fn to_point<'a>(&self, snapshot: &BufferSnapshot) -> Point {
+    fn to_point(&self, snapshot: &BufferSnapshot) -> Point {
         snapshot.summary_for_anchor(self)
     }
 }
 
 impl ToPoint for usize {
-    fn to_point<'a>(&self, snapshot: &BufferSnapshot) -> Point {
+    fn to_point(&self, snapshot: &BufferSnapshot) -> Point {
         snapshot.offset_to_point(*self)
     }
 }
 
-impl ToPoint for PointUtf16 {
+impl ToPoint for Point {
-    fn to_point<'a>(&self, snapshot: &BufferSnapshot) -> Point {
+    fn to_point(&self, _: &BufferSnapshot) -> Point {
-        snapshot.point_utf16_to_point(*self)
+        *self
     }
 }
 
-impl ToPoint for Point {
+impl ToPoint for Unclipped<PointUtf16> {
-    fn to_point<'a>(&self, _: &BufferSnapshot) -> Point {
+    fn to_point(&self, snapshot: &BufferSnapshot) -> Point {
-        *self
+        snapshot.unclipped_point_utf16_to_point(*self)
     }
 }
 
@@ -2423,25 +2430,25 @@ pub trait ToPointUtf16 {
 }
 
 impl ToPointUtf16 for Anchor {
-    fn to_point_utf16<'a>(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
+    fn to_point_utf16(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
         snapshot.summary_for_anchor(self)
     }
 }
 
 impl ToPointUtf16 for usize {
-    fn to_point_utf16<'a>(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
+    fn to_point_utf16(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
         snapshot.offset_to_point_utf16(*self)
     }
 }
 
 impl ToPointUtf16 for PointUtf16 {
-    fn to_point_utf16<'a>(&self, _: &BufferSnapshot) -> PointUtf16 {
+    fn to_point_utf16(&self, _: &BufferSnapshot) -> PointUtf16 {
         *self
     }
 }
 
 impl ToPointUtf16 for Point {
-    fn to_point_utf16<'a>(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
+    fn to_point_utf16(&self, snapshot: &BufferSnapshot) -> PointUtf16 {
         snapshot.point_to_point_utf16(*self)
     }
 }
@@ -2451,45 +2458,23 @@ pub trait ToOffsetUtf16 {
 }
 
 impl ToOffsetUtf16 for Anchor {
-    fn to_offset_utf16<'a>(&self, snapshot: &BufferSnapshot) -> OffsetUtf16 {
+    fn to_offset_utf16(&self, snapshot: &BufferSnapshot) -> OffsetUtf16 {
         snapshot.summary_for_anchor(self)
     }
 }
 
 impl ToOffsetUtf16 for usize {
-    fn to_offset_utf16<'a>(&self, snapshot: &BufferSnapshot) -> OffsetUtf16 {
+    fn to_offset_utf16(&self, snapshot: &BufferSnapshot) -> OffsetUtf16 {
         snapshot.offset_to_offset_utf16(*self)
     }
 }
 
 impl ToOffsetUtf16 for OffsetUtf16 {
-    fn to_offset_utf16<'a>(&self, _snapshot: &BufferSnapshot) -> OffsetUtf16 {
+    fn to_offset_utf16(&self, _snapshot: &BufferSnapshot) -> OffsetUtf16 {
         *self
     }
 }
 
-pub trait Clip {
-    fn clip(&self, bias: Bias, snapshot: &BufferSnapshot) -> Self;
-}
-
-impl Clip for usize {
-    fn clip(&self, bias: Bias, snapshot: &BufferSnapshot) -> Self {
-        snapshot.clip_offset(*self, bias)
-    }
-}
-
-impl Clip for Point {
-    fn clip(&self, bias: Bias, snapshot: &BufferSnapshot) -> Self {
-        snapshot.clip_point(*self, bias)
-    }
-}
-
-impl Clip for PointUtf16 {
-    fn clip(&self, bias: Bias, snapshot: &BufferSnapshot) -> Self {
-        snapshot.clip_point_utf16(*self, bias)
-    }
-}
-
 pub trait FromAnchor {
     fn from_anchor(anchor: &Anchor, snapshot: &BufferSnapshot) -> Self;
 }