Merge #11182

11182: fix: don't panic on seeing an unexpected offset r=Veykril a=dimbleby

Intended as a fix, or at least a sticking plaster, for #11081.

I have arranged that [offset()](https://github.com/rust-analyzer/rust-analyzer/blob/1ba9a924d7b161c52e605e157ee16d582e4a8684/crates/ide_db/src/line_index.rs#L105-L107) returns `Option<TextSize>` instead of going out of bounds; other changes are the result of following the compiler after doing this.

Perhaps there's still an issue here - I suppose the server and client have gotten out of sync and that probably shouldn't happen in the first place?  I see that #10138 (comment) suggests what sounds like a more substantial fix which I think might be aimed in this direction.  So perhaps that one should be left open to cover such things?

Meanwhile, I hope that not-crashing is a good improvement: and I can confirm that it works out just fine in the repro I have at #11081.

Co-authored-by: David Hotham <[email protected]>

Author: bors[bot]

crates/ide_db/src/line_index.rs

@@ -102,8 +102,10 @@ impl LineIndex {
         LineCol { line: line as u32, col: col.into() }
     }
 
-    pub fn offset(&self, line_col: LineCol) -> TextSize {
-        self.newlines[line_col.line as usize] + TextSize::from(line_col.col)
+    pub fn offset(&self, line_col: LineCol) -> Option<TextSize> {
+        self.newlines
+            .get(line_col.line as usize)
+            .map(|offset| offset + TextSize::from(line_col.col))
     }
 
     pub fn to_utf16(&self, line_col: LineCol) -> LineColUtf16 {

crates/rust-analyzer/src/from_proto.rs

@@ -1,4 +1,5 @@
 //! Conversion lsp_types types to rust-analyzer specific ones.
+use anyhow::format_err;
 use ide::{Annotation, AnnotationKind, AssistKind, LineCol, LineColUtf16};
 use ide_db::base_db::{FileId, FilePosition, FileRange};
 use syntax::{TextRange, TextSize};
@@ -22,7 +23,7 @@ pub(crate) fn vfs_path(url: &lsp_types::Url) -> Result<vfs::VfsPath> {
     abs_path(url).map(vfs::VfsPath::from)
 }
 
-pub(crate) fn offset(line_index: &LineIndex, position: lsp_types::Position) -> TextSize {
+pub(crate) fn offset(line_index: &LineIndex, position: lsp_types::Position) -> Result<TextSize> {
     let line_col = match line_index.encoding {
         OffsetEncoding::Utf8 => {
             LineCol { line: position.line as u32, col: position.character as u32 }
@@ -33,13 +34,16 @@ pub(crate) fn offset(line_index: &LineIndex, position: lsp_types::Position) -> T
             line_index.index.to_utf8(line_col)
         }
     };
-    line_index.index.offset(line_col)
+    let text_size =
+        line_index.index.offset(line_col).ok_or_else(|| format_err!("Invalid offset"))?;
+    Ok(text_size)
 }
 
-pub(crate) fn text_range(line_index: &LineIndex, range: lsp_types::Range) -> TextRange {
-    let start = offset(line_index, range.start);
-    let end = offset(line_index, range.end);
-    TextRange::new(start, end)
+pub(crate) fn text_range(line_index: &LineIndex, range: lsp_types::Range) -> Result<TextRange> {
+    let start = offset(line_index, range.start)?;
+    let end = offset(line_index, range.end)?;
+    let text_range = TextRange::new(start, end);
+    Ok(text_range)
 }
 
 pub(crate) fn file_id(snap: &GlobalStateSnapshot, url: &lsp_types::Url) -> Result<FileId> {
@@ -52,7 +56,7 @@ pub(crate) fn file_position(
 ) -> Result<FilePosition> {
     let file_id = file_id(snap, &tdpp.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let offset = offset(&line_index, tdpp.position);
+    let offset = offset(&line_index, tdpp.position)?;
     Ok(FilePosition { file_id, offset })
 }
 
@@ -63,7 +67,7 @@ pub(crate) fn file_range(
 ) -> Result<FileRange> {
     let file_id = file_id(snap, &text_document_identifier.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let range = text_range(&line_index, range);
+    let range = text_range(&line_index, range)?;
     Ok(FileRange { file_id, range })
 }
 
@@ -96,7 +100,7 @@ pub(crate) fn annotation(
             let line_index = snap.file_line_index(file_id)?;
 
             Ok(Annotation {
-                range: text_range(&line_index, code_lens.range),
+                range: text_range(&line_index, code_lens.range)?,
                 kind: AnnotationKind::HasImpls {
                     position: file_position(snap, params.text_document_position_params)?,
                     data: None,
@@ -108,7 +112,7 @@ pub(crate) fn annotation(
             let line_index = snap.file_line_index(file_id)?;
 
             Ok(Annotation {
-                range: text_range(&line_index, code_lens.range),
+                range: text_range(&line_index, code_lens.range)?,
                 kind: AnnotationKind::HasReferences {
                     position: file_position(snap, params)?,
                     data: None,

crates/rust-analyzer/src/handlers.rs

@@ -108,7 +108,7 @@ pub(crate) fn handle_syntax_tree(
     let _p = profile::span("handle_syntax_tree");
     let id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(id)?;
-    let text_range = params.range.map(|r| from_proto::text_range(&line_index, r));
+    let text_range = params.range.and_then(|r| from_proto::text_range(&line_index, r).ok());
     let res = snap.analysis.syntax_tree(id, text_range)?;
     Ok(res)
 }
@@ -149,7 +149,7 @@ pub(crate) fn handle_expand_macro(
     let _p = profile::span("handle_expand_macro");
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let offset = from_proto::offset(&line_index, params.position);
+    let offset = from_proto::offset(&line_index, params.position)?;
 
     let res = snap.analysis.expand_macro(FilePosition { file_id, offset })?;
     Ok(res.map(|it| lsp_ext::ExpandedMacro { name: it.name, expansion: it.expansion }))
@@ -166,7 +166,7 @@ pub(crate) fn handle_selection_range(
         .positions
         .into_iter()
         .map(|position| {
-            let offset = from_proto::offset(&line_index, position);
+            let offset = from_proto::offset(&line_index, position)?;
             let mut ranges = Vec::new();
             {
                 let mut range = TextRange::new(offset, offset);
@@ -205,19 +205,20 @@ pub(crate) fn handle_matching_brace(
     let _p = profile::span("handle_matching_brace");
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let res = params
+    params
         .positions
         .into_iter()
         .map(|position| {
             let offset = from_proto::offset(&line_index, position);
-            let offset = match snap.analysis.matching_brace(FilePosition { file_id, offset }) {
-                Ok(Some(matching_brace_offset)) => matching_brace_offset,
-                Err(_) | Ok(None) => offset,
-            };
-            to_proto::position(&line_index, offset)
+            offset.map(|offset| {
+                let offset = match snap.analysis.matching_brace(FilePosition { file_id, offset }) {
+                    Ok(Some(matching_brace_offset)) => matching_brace_offset,
+                    Err(_) | Ok(None) => offset,
+                };
+                to_proto::position(&line_index, offset)
+            })
         })
-        .collect();
-    Ok(res)
+        .collect()
 }
 
 pub(crate) fn handle_join_lines(
@@ -232,7 +233,7 @@ pub(crate) fn handle_join_lines(
 
     let mut res = TextEdit::default();
     for range in params.ranges {
-        let range = from_proto::text_range(&line_index, range);
+        let range = from_proto::text_range(&line_index, range)?;
         let edit = snap.analysis.join_lines(&config, FileRange { file_id, range })?;
         match res.union(edit) {
             Ok(()) => (),
@@ -675,7 +676,7 @@ pub(crate) fn handle_runnables(
     let _p = profile::span("handle_runnables");
     let file_id = from_proto::file_id(&snap, &params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let offset = params.position.map(|it| from_proto::offset(&line_index, it));
+    let offset = params.position.and_then(|it| from_proto::offset(&line_index, it).ok());
     let cargo_spec = CargoTargetSpec::for_file(&snap, file_id)?;
 
     let expect_test = match offset {
@@ -839,7 +840,7 @@ pub(crate) fn handle_completion_resolve(
 
     let file_id = from_proto::file_id(&snap, &resolve_data.position.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let offset = from_proto::offset(&line_index, resolve_data.position.position);
+    let offset = from_proto::offset(&line_index, resolve_data.position.position)?;
 
     let additional_edits = snap
         .analysis
@@ -1089,7 +1090,7 @@ pub(crate) fn handle_code_action(
             .ranges
             .iter()
             .copied()
-            .map(|range| from_proto::text_range(&line_index, range))
+            .filter_map(|range| from_proto::text_range(&line_index, range).ok())
             .any(|fix_range| fix_range.intersect(frange.range).is_some());
         if intersect_fix_range {
             res.push(fix.action.clone());
@@ -1111,7 +1112,7 @@ pub(crate) fn handle_code_action_resolve(
 
     let file_id = from_proto::file_id(&snap, &params.code_action_params.text_document.uri)?;
     let line_index = snap.file_line_index(file_id)?;
-    let range = from_proto::text_range(&line_index, params.code_action_params.range);
+    let range = from_proto::text_range(&line_index, params.code_action_params.range)?;
     let frange = FileRange { file_id, range };
 
     let mut assists_config = snap.config.assist();

crates/rust-analyzer/src/lsp_utils.rs

@@ -151,8 +151,9 @@ pub(crate) fn apply_document_changes(
                     line_index.index = Arc::new(ide::LineIndex::new(old_text));
                 }
                 index_valid = IndexValid::UpToLineExclusive(range.start.line);
-                let range = from_proto::text_range(&line_index, range);
-                old_text.replace_range(Range::<usize>::from(range), &change.text);
+                if let Ok(range) = from_proto::text_range(&line_index, range) {
+                    old_text.replace_range(Range::<usize>::from(range), &change.text);
+                }
             }
             None => {
                 *old_text = change.text;