mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Merge #11182

Browse source 
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()](1ba9a924d7/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 https://github.com/rust-analyzer/rust-analyzer/issues/10138#issuecomment-913727554 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 <david.hotham@metaswitch.com>
This commit is contained in:
bors[bot] 2022-01-31 11:16:22 +00:00 • committed by GitHub
commit 0808ade4e4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 38 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
crates/ide_db/src/line_index.rs
View file

@ -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 {

24
crates/rust-analyzer/src/from_proto.rs
View file

@ -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,

33
crates/rust-analyzer/src/handlers.rs
View file

@ -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();

5
crates/rust-analyzer/src/lsp_utils.rs
View file

@ -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;