Skip to content

Commit

Permalink
Clamp Position::character to line length
Browse files Browse the repository at this point in the history
LSP says about Position::character

> If the character value is greater than the line length it defaults back to the line length.

but from_proto::offset() doesn't implement this.

A client might for example request code actions for a whole line by sending
Position::character=99999.  I don't think there is ever a reason (besides laziness) why the
client can't specify the line length instead but I guess we should not crash but follow protocol.

Technically it should be a warning, not an error but warning is not shown by default so keep
it at error I guess.

Fixes #18240
  • Loading branch information
krobelus committed Oct 18, 2024
1 parent 72b214f commit 94a4c3a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ test-fixture = { path = "./crates/test-fixture" }
test-utils = { path = "./crates/test-utils" }

# In-tree crates that are published separately and follow semver. See lib/README.md
line-index = { version = "0.1.1" }
line-index = { version = "0.1.2" }
la-arena = { version = "0.3.1" }
lsp-server = { version = "0.7.6" }

Expand Down
12 changes: 10 additions & 2 deletions crates/rust-analyzer/src/lsp/from_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,18 @@ pub(crate) fn offset(
.ok_or_else(|| format_err!("Invalid wide col offset"))?
}
};
let text_size = line_index.index.offset(line_col).ok_or_else(|| {
let line_range = line_index.index.line(line_col.line).ok_or_else(|| {
format_err!("Invalid offset {line_col:?} (line index length: {:?})", line_index.index.len())
})?;
Ok(text_size)
let col = TextSize::from(line_col.col);
let clamped_len = col.min(line_range.len());
if clamped_len < col {
tracing::error!(
"Position {line_col:?} column exceeds line length {}, clamping it",
u32::from(line_range.len()),
);
}
Ok(line_range.start() + clamped_len)
}

pub(crate) fn text_range(
Expand Down

0 comments on commit 94a4c3a

Please sign in to comment.