fix(rome_lsp): fix panics on out of bounds formatting range
MODIFY
crates/rome_formatter/src/lib.rs
crates/rome_formatter/src/lib.rs
@@ -386,12 +386,19 @@ pub enum FormatError {
/// In case a node can't be formatted because it either misses a require child element or
/// a child is present that should not (e.g. a trailing comma after a rest element).
SyntaxError,
+ /// In case range formatting failed because the provided range was larger
+ /// than the formatted syntax tree
+ RangeError { input: TextRange, tree: TextRange },
}
impl std::fmt::Display for FormatError {
fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
FormatError::SyntaxError => fmt.write_str("syntax error"),
+ FormatError::RangeError { input, tree } => std::write!(
+ fmt,
+ "formatting range {input:?} is larger than syntax tree {tree:?}"
+ ),
}
}
}
@@ -853,6 +860,14 @@ where
));
}
+ let root_range = root.text_range();
+ if range.start() < root_range.start() || range.end() > root_range.end() {
+ return Err(FormatError::RangeError {
+ input: range,
+ tree: root_range,
+ });
+ }
+
// Find the tokens corresponding to the start and end of the range
let start_token = root.token_at_offset(range.start());
let end_token = root.token_at_offset(range.end());

MODIFY
crates/rome_js_formatter/src/lib.rs
crates/rome_js_formatter/src/lib.rs
@@ -692,9 +692,9 @@ pub(crate) mod separated;
#[cfg(test)]
mod test {
use crate::check_reformat::{check_reformat, CheckReformatParams};
- use crate::{format_node, JsFormatContext};
+ use crate::{format_node, format_range, JsFormatContext};
use rome_js_parser::parse;
- use rome_js_syntax::SourceType;
+ use rome_js_syntax::{SourceType, TextRange, TextSize};
#[ignore]
#[test]
@@ -722,4 +722,20 @@ test.expect(t => {
"type B8 = /*1*/ (C);\ntype B9 = (/*1*/ C);\ntype B10 = /*1*/ /*2*/ C;\n"
);
}
+
+ #[test]
+ fn format_range_out_of_bounds() {
+ let src = "statement();";
+
+ let syntax = SourceType::js_module();
+ let tree = parse(src, 0, syntax);
+
+ let result = format_range(
+ JsFormatContext::default(),
+ &tree.syntax(),
+ TextRange::new(TextSize::from(0), TextSize::of(src) + TextSize::from(5)),
+ );
+
+ assert!(result.is_err());
+ }
}

MODIFY
crates/rome_lsp/Cargo.toml
crates/rome_lsp/Cargo.toml
@@ -25,7 +25,7 @@ tower-lsp = { version = "0.17.0"}
tokio = { version = "1.15.0", features = ["full" ] }
tracing = { version = "0.1.31", default-features = false, features = ["std", "max_level_trace", "release_max_level_warn"] }
tracing-tree = "0.2.0"
-tracing-subscriber = "0.3.5"
+tracing-subscriber = { version = "0.3.5", features = ["env-filter"] }
parking_lot = "0.12.0"
futures = "0.3"

MODIFY
crates/rome_lsp/src/documents.rs
crates/rome_lsp/src/documents.rs
@@ -31,13 +31,13 @@ impl TryFrom<&str> for EditorLanguage {
///
/// [`textDocument`]: https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#textDocumentItem
#[derive(Clone)]
-pub struct Document {
- pub version: i32,
- pub line_index: LineIndex,
+pub(crate) struct Document {
+ pub(crate) version: i32,
+ pub(crate) line_index: LineIndex,
}
impl Document {
- pub fn new(version: i32, text: &str) -> Self {
+ pub(crate) fn new(version: i32, text: &str) -> Self {
Self {
version,
line_index: LineIndex::new(text),

MODIFY
crates/rome_lsp/src/handlers/analysis.rs
crates/rome_lsp/src/handlers/analysis.rs
use std::collections::HashMap;
-use anyhow::Result;
+use anyhow::{Context, Result};
use rome_analyze::ActionCategory;
use rome_fs::RomePath;
use rome_service::workspace::{FixFileParams, PullActionsParams};
@@ -18,6 +18,7 @@ const FIX_ALL: CodeActionKind = CodeActionKind::new("source.fixAll");
/// Queries the [`AnalysisServer`] for code actions of the file matching [FileId]
///
/// If the AnalysisServer has no matching file, results in error.
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) fn code_actions(
session: &Session,
params: CodeActionParams,
@@ -49,7 +50,12 @@ pub(crate) fn code_actions(
let doc = session.document(&url)?;
let diagnostics = params.context.diagnostics;
- let cursor_range = crate::utils::text_range(&doc.line_index, params.range);
+ let cursor_range = utils::text_range(&doc.line_index, params.range).with_context(|| {
+ format!(
+ "failed to access range {:?} in document {url}",
+ params.range
+ )
+ })?;
let result = session.workspace.pull_actions(PullActionsParams {
path: rome_path.clone(),
@@ -100,6 +106,7 @@ pub(crate) fn code_actions(
}
/// Generate a "fix all" code action for the given document
+#[tracing::instrument(level = "trace", skip(session), err)]
fn fix_all(
session: &Session,
url: &lsp::Url,
@@ -123,7 +130,7 @@ fn fix_all(
lsp::NumberOrString::Number(_) => None,
})?;
- let diag_range = utils::text_range(line_index, d.range);
+ let diag_range = utils::text_range(line_index, d.range).ok()?;
let has_matching_rule = fixed.actions.iter().any(|action| {
code == action.rule_name && action.range.intersect(diag_range).is_some()
});

MODIFY
crates/rome_lsp/src/handlers/formatting.rs
crates/rome_lsp/src/handlers/formatting.rs
-use crate::line_index::LineCol;
use crate::session::Session;
-use anyhow::Result;
+use crate::utils;
+use anyhow::{Context, Result};
use rome_formatter::IndentStyle;
use rome_rowan::TextRange;
use rome_service::workspace::{FormatFileParams, FormatOnTypeParams, FormatRangeParams};
use tower_lsp::lsp_types::*;
use tracing::trace;
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) fn format(
session: &Session,
params: DocumentFormattingParams,
@@ -46,6 +47,7 @@ pub(crate) fn format(
Ok(Some(edits))
}
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) fn format_range(
session: &Session,
params: DocumentRangeFormattingParams,
@@ -60,14 +62,18 @@ pub(crate) fn format_range(
IndentStyle::Tab
};
- let start_index = doc.line_index.offset(LineCol {
- line: params.range.start.line,
- col: params.range.start.character,
- });
- let end_index = doc.line_index.offset(LineCol {
- line: params.range.end.line,
- col: params.range.end.character,
- });
+ let start_index = utils::offset(&doc.line_index, params.range.start).with_context(|| {
+ format!(
+ "failed to access position {:?} in document {url}",
+ params.range.start
+ )
+ })?;
+ let end_index = utils::offset(&doc.line_index, params.range.end).with_context(|| {
+ format!(
+ "failed to access position {:?} in document {url}",
+ params.range.end
+ )
+ })?;
let format_range = TextRange::new(start_index, end_index);
let formatted = session.workspace.format_range(FormatRangeParams {
@@ -107,6 +113,7 @@ pub(crate) fn format_range(
}]))
}
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) fn format_on_type(
session: &Session,
params: DocumentOnTypeFormattingParams,
@@ -123,10 +130,8 @@ pub(crate) fn format_on_type(
IndentStyle::Tab
};
- let offset = doc.line_index.offset(LineCol {
- line: position.line,
- col: position.character,
- });
+ let offset = utils::offset(&doc.line_index, position)
+ .with_context(|| format!("failed to access position {position:?} in document {url}"))?;
let formatted = session.workspace.format_on_type(FormatOnTypeParams {
path: rome_path,

MODIFY
crates/rome_lsp/src/handlers/rename.rs
crates/rome_lsp/src/handlers/rename.rs
use std::collections::HashMap;
use crate::{session::Session, utils};
-use anyhow::Result;
+use anyhow::{Context, Result};
use tower_lsp::lsp_types::{RenameParams, TextEdit, WorkspaceEdit};
use tracing::trace;
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) fn rename(session: &Session, params: RenameParams) -> Result<Option<WorkspaceEdit>> {
let url = params.text_document_position.text_document.uri;
let rome_path = session.file_path(&url);
@@ -12,8 +13,13 @@ pub(crate) fn rename(session: &Session, params: RenameParams) -> Result<Option<W
trace!("Renaming...");
let doc = session.document(&url)?;
- let cursor_range =
- crate::utils::offset(&doc.line_index, params.text_document_position.position);
+ let cursor_range = utils::offset(&doc.line_index, params.text_document_position.position)
+ .with_context(|| {
+ format!(
+ "failed to access position {:?} in document {url}",
+ params.text_document_position.position
+ )
+ })?;
let result = session
.workspace

MODIFY
crates/rome_lsp/src/handlers/text_document.rs
crates/rome_lsp/src/handlers/text_document.rs
@@ -6,6 +6,7 @@ use tracing::error;
use crate::{documents::Document, session::Session};
/// Handler for `textDocument/didOpen` LSP notification
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) async fn did_open(
session: &Session,
params: lsp_types::DidOpenTextDocumentParams,
@@ -33,6 +34,7 @@ pub(crate) async fn did_open(
}
/// Handler for `textDocument/didChange` LSP notification
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) async fn did_change(
session: &Session,
params: lsp_types::DidChangeTextDocumentParams,
@@ -66,6 +68,7 @@ pub(crate) async fn did_change(
}
/// Handler for `textDocument/didClose` LSP notification
+#[tracing::instrument(level = "trace", skip(session), err)]
pub(crate) async fn did_close(
session: &Session,
params: lsp_types::DidCloseTextDocumentParams,

MODIFY
crates/rome_lsp/src/line_index.rs
crates/rome_lsp/src/line_index.rs
//! representation.
//!
//! Copied from rust-analyzer
-use std::{collections::HashMap, iter};
-use rome_rowan::{TextRange, TextSize};
+use rome_rowan::TextSize;
#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct LineIndex {
+pub(crate) struct LineIndex {
/// Offset the the beginning of each line, zero-based
pub(crate) newlines: Vec<TextSize>,
- /// List of non-ASCII characters on each line
- pub(crate) utf16_lines: HashMap<u32, Vec<Utf16Char>>,
}
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub struct LineColUtf16 {
+pub(crate) struct LineCol {
/// Zero-based
- pub line: u32,
- /// Zero-based
- pub col: u32,
-}
-
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-pub struct LineCol {
- /// Zero-based
- pub line: u32,
+ pub(crate) line: u32,
/// Zero-based utf8 offset
- pub col: u32,
-}
-
-#[derive(Clone, Debug, Hash, PartialEq, Eq)]
-pub(crate) struct Utf16Char {
- /// Start offset of a character inside a line, zero-based
- pub(crate) start: TextSize,
- /// End offset of a character inside a line, zero-based
- pub(crate) end: TextSize,
-}
-
-#[allow(unused)]
-impl Utf16Char {
- /// Returns the length in 8-bit UTF-8 code units.
- fn len(&self) -> TextSize {
- self.end - self.start
- }
-
- /// Returns the length in 16-bit UTF-16 code units.
- fn len_utf16(&self) -> usize {
- if self.len() == TextSize::from(4) {
- 2
- } else {
- 1
- }
- }
+ pub(crate) col: u32,
}
-#[allow(unused)]
impl LineIndex {
- pub fn new(text: &str) -> LineIndex {
- let mut utf16_lines = HashMap::default();
- let mut utf16_chars = Vec::new();
-
+ pub(crate) fn new(text: &str) -> LineIndex {
let mut newlines = vec![0.into()];
let mut curr_row = 0.into();
- let mut curr_col = 0.into();
- let mut line = 0;
+
for c in text.chars() {
let c_len = TextSize::of(c);
curr_row += c_len;
+
if c == '\n' {
newlines.push(curr_row);
-
- // Save any utf-16 characters seen in the previous line
- if !utf16_chars.is_empty() {
- utf16_lines.insert(line, utf16_chars);
- utf16_chars = Vec::new();
}
-
- // Prepare for processing the next line
- curr_col = 0.into();
- line += 1;
- continue;
- }
-
- if !c.is_ascii() {
- utf16_chars.push(Utf16Char {
- start: curr_col,
- end: curr_col + c_len,
- });
- }
-
- curr_col += c_len;
}
- // Save any utf-16 characters seen in the last line
- if !utf16_chars.is_empty() {
- utf16_lines.insert(line, utf16_chars);
+ LineIndex { newlines }
}
- LineIndex {
- newlines,
- utf16_lines,
- }
- }
-
- pub fn line_col(&self, offset: TextSize) -> LineCol {
+ pub(crate) fn line_col(&self, offset: TextSize) -> LineCol {
let line = self.newlines.partition_point(|&it| it <= offset) - 1;
let line_start_offset = self.newlines[line];
let col = offset - line_start_offset;
@@ -114,68 +46,21 @@ impl LineIndex {
}
}
- pub fn offset(&self, line_col: LineCol) -> TextSize {
- self.newlines[line_col.line as usize] + TextSize::from(line_col.col)
- }
-
- pub fn to_utf16(&self, line_col: LineCol) -> LineColUtf16 {
- let col = self.utf8_to_utf16_col(line_col.line, line_col.col.into());
- LineColUtf16 {
- line: line_col.line,
- col: col as u32,
+ pub(crate) fn offset(&self, line_col: LineCol) -> Option<TextSize> {
+ let line_index = usize::try_from(line_col.line).ok()?;
+ let line_offset = self.newlines.get(line_index)?;
+ Some(*line_offset + TextSize::from(line_col.col))
}
}
- pub fn to_utf8(&self, line_col: LineColUtf16) -> LineCol {
- let col = self.utf16_to_utf8_col(line_col.line, line_col.col);
- LineCol {
- line: line_col.line,
- col: col.into(),
- }
- }
-
- pub fn lines(&self, range: TextRange) -> impl Iterator<Item = TextRange> + '_ {
- let lo = self.newlines.partition_point(|&it| it < range.start());
- let hi = self.newlines.partition_point(|&it| it <= range.end());
- let all = iter::once(range.start())
- .chain(self.newlines[lo..hi].iter().copied())
- .chain(iter::once(range.end()));
-
- all.clone()
- .zip(all.skip(1))
- .map(|(lo, hi)| TextRange::new(lo, hi))
- .filter(|it| !it.is_empty())
- }
-
- fn utf8_to_utf16_col(&self, line: u32, col: TextSize) -> usize {
- let mut res: usize = col.into();
- if let Some(utf16_chars) = self.utf16_lines.get(&line) {
- for c in utf16_chars {
- if c.end <= col {
- res -= usize::from(c.len()) - c.len_utf16();
- } else {
- // From here on, all utf16 characters come *after* the character we are mapping,
- // so we don't need to take them into account
- break;
- }
- }
- }
- res
- }
-
- fn utf16_to_utf8_col(&self, line: u32, mut col: u32) -> TextSize {
- if let Some(utf16_chars) = self.utf16_lines.get(&line) {
- for c in utf16_chars {
- if col > u32::from(c.start) {
- col += u32::from(c.len()) - c.len_utf16() as u32;
- } else {
- // From here on, all utf16 characters come *after* the character we are mapping,
- // so we don't need to take them into account
- break;
- }
- }
- }
+#[cfg(test)]
+mod tests {
+ use super::{LineCol, LineIndex};
- col.into()
+ #[test]
+ fn out_of_bounds() {
+ let line_index = LineIndex::new("abcde\nfghij\n");
+ let offset = line_index.offset(LineCol { line: 5, col: 0 });
+ assert!(offset.is_none());
}
}

MODIFY
crates/rome_lsp/src/main.rs
crates/rome_lsp/src/main.rs
use tracing::{debug_span, Instrument};
-use tracing_subscriber::{layer::SubscriberExt, Registry};
+use tracing_subscriber::{layer::SubscriberExt, prelude::*, registry, EnvFilter};
use tracing_tree::HierarchicalLayer;
use rome_lsp::server::run_server;
@@ -8,18 +8,26 @@ use rome_lsp::server::run_server;
#[global_allocator]
static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;
+/// This filter enables:
+/// - All spans and events at level info or higher
+/// - All spans and events in the `rome_lsp` and `rome_js_parser` crates
+const LOGGING_FILTER: &str = "info,rome_lsp=trace,rome_js_parser=trace";
+
#[tokio::main]
async fn main() {
let stdin = tokio::io::stdin();
let stdout = tokio::io::stdout();
- let layer = HierarchicalLayer::default()
+ registry()
+ .with(
+ HierarchicalLayer::default()
.with_indent_lines(true)
.with_indent_amount(2)
- .with_bracketed_fields(true);
-
- let subscriber = Registry::default().with(layer);
- tracing::subscriber::set_global_default(subscriber).unwrap();
+ .with_bracketed_fields(true)
+ .with_targets(true)
+ .with_filter(EnvFilter::new(LOGGING_FILTER)),
+ )
+ .init();
let span = debug_span!("Running LSP Server", pid = std::process::id());
run_server(stdin, stdout).instrument(span).await

MODIFY
crates/rome_lsp/src/server.rs
crates/rome_lsp/src/server.rs
@@ -39,6 +39,7 @@ impl LSPServer {
#[tower_lsp::async_trait]
impl LanguageServer for LSPServer {
+ #[tracing::instrument(level = "trace", skip(self))]
async fn initialize(&self, params: InitializeParams) -> LspResult<InitializeResult> {
info!("Starting Rome Language Server...");
@@ -58,7 +59,10 @@ impl LanguageServer for LSPServer {
Ok(init)
}
- async fn initialized(&self, _: InitializedParams) {
+ #[tracing::instrument(level = "trace", skip(self))]
+ async fn initialized(&self, params: InitializedParams) {
+ let _ = params;
+
info!("Attempting to load the configuration from 'rome.json' file");
match load_config(&self.session.fs) {
@@ -133,7 +137,10 @@ impl LanguageServer for LSPServer {
handlers::formatting::format_on_type(&self.session, params).map_err(into_lsp_error)
}
- async fn did_change_configuration(&self, _params: DidChangeConfigurationParams) {
+ #[tracing::instrument(level = "trace", skip(self))]
+ async fn did_change_configuration(&self, params: DidChangeConfigurationParams) {
+ let _ = params;
+
let diags_enabled_prev = self.session.diagnostics_enabled();
self.session.fetch_client_configuration().await;

MODIFY
crates/rome_lsp/src/session.rs
crates/rome_lsp/src/session.rs
@@ -98,6 +98,7 @@ impl Session {
/// Computes diagnostics for the file matching the provided url and publishes
/// them to the client. Called from [`handlers::text_document`] when a file's
/// contents changes.
+ #[tracing::instrument(level = "trace", skip(self), err)]
pub(crate) async fn update_diagnostics(&self, url: lsp_types::Url) -> anyhow::Result<()> {
let rome_path = self.file_path(&url);
let doc = self.document(&url)?;

MODIFY
crates/rome_lsp/src/utils.rs
crates/rome_lsp/src/utils.rs
@@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::fmt::{Debug, Display};
use crate::line_index::{LineCol, LineIndex};
+use anyhow::{Context, Result};
use rome_analyze::ActionCategory;
use rome_console::fmt::Termcolor;
use rome_console::fmt::{self, Formatter};
@@ -26,18 +27,21 @@ pub(crate) fn range(line_index: &LineIndex, range: TextRange) -> lsp::Range {
lsp::Range::new(start, end)
}
-pub(crate) fn offset(line_index: &LineIndex, position: lsp::Position) -> TextSize {
+pub(crate) fn offset(line_index: &LineIndex, position: lsp::Position) -> Result<TextSize> {
let line_col = LineCol {
line: position.line as u32,
col: position.character as u32,
};
- line_index.offset(line_col)
+
+ line_index
+ .offset(line_col)
+ .with_context(|| format!("position {position:?} is out of range"))
}
-pub(crate) fn text_range(line_index: &LineIndex, range: lsp::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::Range) -> Result<TextRange> {
+ let start = offset(line_index, range.start)?;
+ let end = offset(line_index, range.end)?;
+ Ok(TextRange::new(start, end))
}
pub(crate) fn code_fix_to_lsp(

MODIFY
crates/rome_service/src/file_handlers/javascript.rs
crates/rome_service/src/file_handlers/javascript.rs
use rome_analyze::{AnalysisFilter, ControlFlow, Never, RuleCategories, RuleFilter};
use rome_diagnostics::{Applicability, CodeSuggestion, Diagnostic};
-use rome_formatter::{IndentStyle, Printed};
+use rome_formatter::{FormatError, IndentStyle, Printed};
use rome_fs::RomePath;
use rome_js_analyze::analyze;
use rome_js_analyze::utils::rename::RenameError;
@@ -278,6 +278,14 @@ fn format_on_type(
let tree = parse.syntax();
+ let range = tree.text_range();
+ if offset < range.start() || offset > range.end() {
+ return Err(RomeError::FormatError(FormatError::RangeError {
+ input: TextRange::at(offset, TextSize::from(0)),
+ tree: range,
+ }));
+ }
+
let token = match tree.token_at_offset(offset) {
// File is empty, do nothing
TokenAtOffset::None => panic!("empty file"),

fix(rome_lsp): fix panics on out of bounds formatting range (#2935)