fix(rome_lsp): improve the handling of and overflow errors in `LineIndex`
MODIFY
crates/rome_lsp/src/handlers/analysis.rs
crates/rome_lsp/src/handlers/analysis.rs
@@ -96,9 +96,10 @@ pub(crate) fn code_actions(
return None;
}
- let action = utils::code_fix_to_lsp(&url, &doc.line_index, &diagnostics, action);
- has_fixes |= action.diagnostics.is_some();
+ let action =
+ utils::code_fix_to_lsp(&url, &doc.line_index, &diagnostics, action).ok()?;
+ has_fixes |= action.diagnostics.is_some();
Some(CodeActionOrCommand::CodeAction(action))
})
.chain(fix_all)
@@ -169,10 +170,7 @@ fn fix_all(
vec![lsp::TextEdit {
range: lsp::Range {
start: lsp::Position::new(0, 0),
- end: lsp::Position::new(
- line_index.newlines.len().try_into().unwrap_or(u32::MAX),
- 0,
- ),
+ end: lsp::Position::new(line_index.len(), 0),
},
new_text: fixed.code,
}],

MODIFY
crates/rome_lsp/src/handlers/formatting.rs
crates/rome_lsp/src/handlers/formatting.rs
@@ -32,7 +32,7 @@ pub(crate) fn format(
Err(err) => return Err(Error::from(err)),
};
- let num_lines: u32 = doc.line_index.newlines.len().try_into()?;
+ let num_lines: u32 = doc.line_index.len();
let range = Range {
start: Position::default(),
@@ -89,23 +89,17 @@ pub(crate) fn format_range(
// Recalculate the actual range that was reformatted from the formatter result
let formatted_range = match formatted.range() {
Some(range) => {
- let start_loc = doc.line_index.line_col(range.start());
- let end_loc = doc.line_index.line_col(range.end());
+ let start_loc = utils::position(&doc.line_index, range.start())?;
+ let end_loc = utils::position(&doc.line_index, range.end())?;
Range {
- start: Position {
- line: start_loc.line,
- character: start_loc.col,
- },
- end: Position {
- line: end_loc.line,
- character: end_loc.col,
- },
+ start: start_loc,
+ end: end_loc,
}
}
None => Range {
start: Position::default(),
end: Position {
- line: doc.line_index.newlines.len().try_into()?,
+ line: doc.line_index.len(),
character: 0,
},
},
@@ -147,23 +141,17 @@ pub(crate) fn format_on_type(
// Recalculate the actual range that was reformatted from the formatter result
let formatted_range = match formatted.range() {
Some(range) => {
- let start_loc = doc.line_index.line_col(range.start());
- let end_loc = doc.line_index.line_col(range.end());
+ let start_loc = utils::position(&doc.line_index, range.start())?;
+ let end_loc = utils::position(&doc.line_index, range.end())?;
Range {
- start: Position {
- line: start_loc.line,
- character: start_loc.col,
- },
- end: Position {
- line: end_loc.line,
- character: end_loc.col,
- },
+ start: start_loc,
+ end: end_loc,
}
}
None => Range {
start: Position::default(),
end: Position {
- line: doc.line_index.newlines.len().try_into()?,
+ line: doc.line_index.len(),
character: 0,
},
},

MODIFY
crates/rome_lsp/src/handlers/rename.rs
crates/rome_lsp/src/handlers/rename.rs
@@ -30,7 +30,7 @@ pub(crate) fn rename(session: &Session, params: RenameParams) -> Result<Option<W
})?;
let mut changes = HashMap::new();
- changes.insert(url, utils::text_edit(&doc.line_index, result.indels));
+ changes.insert(url, utils::text_edit(&doc.line_index, result.indels)?);
let workspace_edit = WorkspaceEdit {
changes: Some(changes),

MODIFY
crates/rome_lsp/src/handlers/text_document.rs
crates/rome_lsp/src/handlers/text_document.rs
@@ -3,7 +3,7 @@ use std::ops::Range;
use anyhow::Result;
use rome_service::workspace::{ChangeFileParams, CloseFileParams, Language, OpenFileParams};
use tower_lsp::lsp_types;
-use tracing::error;
+use tracing::{error, field};
use crate::{documents::Document, session::Session, utils};
@@ -38,7 +38,7 @@ pub(crate) async fn did_open(
}
/// Handler for `textDocument/didChange` LSP notification
-#[tracing::instrument(level = "trace", skip(session), err)]
+#[tracing::instrument(level = "trace", skip_all, fields(url = field::display(&params.text_document.uri), version = params.text_document.version), err)]
pub(crate) async fn did_change(
session: &Session,
params: lsp_types::DidChangeTextDocumentParams,
@@ -50,18 +50,25 @@ pub(crate) async fn did_change(
let doc = session.document(&url)?;
let mut content = doc.content;
+ tracing::trace!("old document: {content:?}");
+
for change in params.content_changes {
match change.range {
Some(range) => {
- let range = utils::text_range(&doc.line_index, range)?;
- content.replace_range(Range::<usize>::from(range), &change.text);
+ let text_range = utils::text_range(&doc.line_index, range)?;
+ let range = Range::<usize>::from(text_range);
+ tracing::trace!("replace range {range:?} with {:?}", change.text);
+ content.replace_range(range, &change.text);
}
None => {
+ tracing::trace!("replace content {:?}", change.text);
content = change.text;
}
}
}
+ tracing::trace!("new document: {content:?}");
+
let doc = Document::new(version, &content);
session.workspace.change_file(ChangeFileParams {

MODIFY
crates/rome_lsp/src/utils.rs
crates/rome_lsp/src/utils.rs
@@ -21,15 +21,21 @@ use tower_lsp::jsonrpc::Error as LspError;
use tower_lsp::lsp_types::{self as lsp};
use tracing::error;
-pub(crate) fn position(line_index: &LineIndex, offset: TextSize) -> lsp::Position {
- let line_col = line_index.line_col(offset);
- lsp::Position::new(line_col.line, line_col.col)
+pub(crate) fn position(line_index: &LineIndex, offset: TextSize) -> Result<lsp::Position> {
+ let line_col = line_index.line_col(offset).with_context(|| {
+ format!(
+ "could not convert offset {offset:?} into a line-column index into string {:?}",
+ line_index.text()
+ )
+ })?;
+
+ Ok(lsp::Position::new(line_col.line, line_col.col))
}
-pub(crate) fn range(line_index: &LineIndex, range: TextRange) -> lsp::Range {
- let start = position(line_index, range.start());
- let end = position(line_index, range.end());
- lsp::Range::new(start, end)
+pub(crate) fn range(line_index: &LineIndex, range: TextRange) -> Result<lsp::Range> {
+ let start = position(line_index, range.start())?;
+ let end = position(line_index, range.end())?;
+ Ok(lsp::Range::new(start, end))
}
pub(crate) fn offset(line_index: &LineIndex, position: lsp::Position) -> Result<TextSize> {
@@ -49,7 +55,7 @@ pub(crate) fn text_range(line_index: &LineIndex, range: lsp::Range) -> Result<Te
Ok(TextRange::new(start, end))
}
-pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Vec<lsp::TextEdit> {
+pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Result<Vec<lsp::TextEdit>> {
let mut result: Vec<lsp::TextEdit> = Vec::new();
let mut offset = TextSize::from(0);
@@ -59,7 +65,7 @@ pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Vec<lsp::Text
offset += range.len();
}
CompressedOp::DiffOp(DiffOp::Insert { range }) => {
- let start = position(line_index, offset);
+ let start = position(line_index, offset)?;
// Merge with a previous delete operation if possible
let last_edit = result.last_mut().filter(|text_edit| {
@@ -76,9 +82,9 @@ pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Vec<lsp::Text
}
}
CompressedOp::DiffOp(DiffOp::Delete { range }) => {
- let start = position(line_index, offset);
+ let start = position(line_index, offset)?;
offset += range.len();
- let end = position(line_index, offset);
+ let end = position(line_index, offset)?;
result.push(lsp::TextEdit {
range: lsp::Range::new(start, end),
@@ -87,7 +93,10 @@ pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Vec<lsp::Text
}
CompressedOp::EqualLines { line_count } => {
- let mut line_col = line_index.line_col(offset);
+ let mut line_col = line_index
+ .line_col(offset)
+ .expect("diff length is overflowing the line count in the original file");
+
line_col.line += line_count.get() + 1;
line_col.col = 0;
@@ -102,7 +111,7 @@ pub(crate) fn text_edit(line_index: &LineIndex, diff: TextEdit) -> Vec<lsp::Text
}
}
- result
+ Ok(result)
}
pub(crate) fn code_fix_to_lsp(
@@ -110,7 +119,7 @@ pub(crate) fn code_fix_to_lsp(
line_index: &LineIndex,
diagnostics: &[lsp::Diagnostic],
action: CodeAction,
-) -> lsp::CodeAction {
+) -> Result<lsp::CodeAction> {
// Mark diagnostics emitted by the same rule as resolved by this action
let diagnostics: Vec<_> = if matches!(action.category, ActionCategory::QuickFix) {
diagnostics
@@ -148,7 +157,7 @@ pub(crate) fn code_fix_to_lsp(
let suggestion = action.suggestion;
let mut changes = HashMap::new();
- let edits = text_edit(line_index, suggestion.suggestion);
+ let edits = text_edit(line_index, suggestion.suggestion)?;
changes.insert(url.clone(), edits);
@@ -158,7 +167,7 @@ pub(crate) fn code_fix_to_lsp(
change_annotations: None,
};
- lsp::CodeAction {
+ Ok(lsp::CodeAction {
title: print_markup(&suggestion.msg),
kind: Some(lsp::CodeActionKind::from(kind)),
diagnostics: if !diagnostics.is_empty() {
@@ -175,7 +184,7 @@ pub(crate) fn code_fix_to_lsp(
},
disabled: None,
data: None,
- }
+ })
}
/// Convert an [rome_diagnostics::Diagnostic] to a [lsp::Diagnostic], using the span
@@ -188,7 +197,7 @@ pub(crate) fn diagnostic_to_lsp<D: Diagnostic>(
line_index: &LineIndex,
) -> Option<lsp::Diagnostic> {
let location = diagnostic.location()?;
- let span = range(line_index, location.span?);
+ let span = range(line_index, location.span?).ok()?;
let severity = match diagnostic.severity() {
Severity::Fatal | Severity::Error => lsp::DiagnosticSeverity::ERROR,
@@ -255,12 +264,17 @@ impl Visit for RelatedInformationVisitor<'_> {
None => return Ok(()),
};
+ let range = match range(self.line_index, span) {
+ Ok(range) => range,
+ Err(_) => return Ok(()),
+ };
+
let related_information = self.related_information.get_or_insert_with(Vec::new);
related_information.push(lsp::DiagnosticRelatedInformation {
location: lsp::Location {
uri: self.url.clone(),
- range: range(self.line_index, span),
+ range,
},
message: String::new(),
});
@@ -337,7 +351,7 @@ line 7 new";
let line_index = LineIndex::new(OLD);
let diff = TextEdit::from_unicode_words(OLD, NEW);
- let text_edit = super::text_edit(&line_index, diff);
+ let text_edit = super::text_edit(&line_index, diff).unwrap();
assert_eq!(
text_edit.as_slice(),
@@ -380,7 +394,7 @@ line 7 new";
let line_index = LineIndex::new(OLD);
let diff = TextEdit::from_unicode_words(OLD, NEW);
- let text_edit = super::text_edit(&line_index, diff);
+ let text_edit = super::text_edit(&line_index, diff).unwrap();
assert_eq!(
text_edit.as_slice(),

fix(rome_lsp): improve the handling of UTF-8 and overflow errors in `LineIndex` (#3745)