Skip to content

Commit 0c22997

Browse files
committed
Clamp Position::character to line length
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. Not sure how to update the lockfile (lib/README.md doesn't say how). Fixes #18240
1 parent 5982d9c commit 0c22997

File tree

5 files changed

+50
-5
lines changed

5 files changed

+50
-5
lines changed

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ test-fixture = { path = "./crates/test-fixture" }
9696
test-utils = { path = "./crates/test-utils" }
9797

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

crates/rust-analyzer/src/lsp/from_proto.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,20 @@ pub(crate) fn offset(
3535
.ok_or_else(|| format_err!("Invalid wide col offset"))?
3636
}
3737
};
38-
let text_size = line_index.index.offset(line_col).ok_or_else(|| {
39-
format_err!("Invalid offset {line_col:?} (line index length: {:?})", line_index.index.len())
40-
})?;
38+
let (text_size, clamped_length) =
39+
line_index.index.offset_clamped(line_col).ok_or_else(|| {
40+
format_err!(
41+
"Invalid offset {line_col:?} (line index length: {:?})",
42+
line_index.index.len()
43+
)
44+
})?;
45+
if let Some(clamped_length) = clamped_length {
46+
tracing::error!(
47+
"Position {line_col:?} column exceeds line length {}, clamping it",
48+
line_col,
49+
u32::from(clamped_length),
50+
);
51+
}
4152
Ok(text_size)
4253
}
4354

lib/line-index/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "line-index"
3-
version = "0.1.1"
3+
version = "0.1.2"
44
description = "Maps flat `TextSize` offsets to/from `(line, column)` representation."
55
license = "MIT OR Apache-2.0"
66
repository = "https://github.com/rust-lang/rust-analyzer/tree/master/lib/line-index"

lib/line-index/src/lib.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,22 @@ impl LineIndex {
136136
self.start_offset(line_col.line as usize).map(|start| start + TextSize::from(line_col.col))
137137
}
138138

139+
/// Transforms the `LineCol` into a `TextSize`. If the column exceeds the line length,
140+
/// the line, it is clamped to that.
141+
pub fn offset_clamped(&self, line_col: LineCol) -> Option<(TextSize, Option<TextSize>)> {
142+
self.start_offset(line_col.line as usize).map(|start| {
143+
let next_newline =
144+
self.newlines.get(line_col.line as usize).copied().unwrap_or(self.len);
145+
let line_length = next_newline - start;
146+
let col = TextSize::from(line_col.col);
147+
if col > line_length {
148+
(start + line_length, Some(line_length))
149+
} else {
150+
(start + col, None)
151+
}
152+
})
153+
}
154+
139155
fn start_offset(&self, line: usize) -> Option<TextSize> {
140156
match line.checked_sub(1) {
141157
None => Some(TextSize::from(0)),

lib/line-index/src/tests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,21 @@ fn test_every_chars() {
195195
}
196196
}
197197
}
198+
199+
#[test]
200+
fn test_offset_clamped() {
201+
macro_rules! validate {
202+
($text:expr, $line:expr, $col:expr, $expected:expr) => {
203+
let line_index = LineIndex::new($text);
204+
let line_col = LineCol { line: $line, col: $col };
205+
assert_eq!(line_index.offset_clamped(line_col), $expected);
206+
};
207+
}
208+
209+
validate!("", 0, 0, Some((TextSize::new(0), None)));
210+
validate!("", 0, 1, Some((TextSize::new(0), Some(TextSize::new(0)))));
211+
validate!("\n", 1, 0, Some((TextSize::new(1), None)));
212+
validate!("\nabc", 1, 1, Some((TextSize::new(2), None)));
213+
validate!("\nabc", 1, 3, Some((TextSize::new(4), None)));
214+
validate!("\nabc", 1, 4, Some((TextSize::new(4), Some(TextSize::new(3)))));
215+
}

0 commit comments

Comments
 (0)