1073dd6 · helix · git.iko.soy

git.iko.soy/helix

robustly handle invalid LSP ranges (#6512)

commit1073dd632932d0c9131f6413a5ced69ee7096e60 authorPascal Kuthe <pascal.kuthe@semimod.de> dateMon 3 Apr 2023, 03:58 (4 years ago) parentbfe8d26

2 files changed+26−15

compare with master

unifiedsplit

helix-lsp/src/lib.rs+20−5 helix-term/src/ui/completion.rs+6−10

helix-lsp/src/lib.rs+20−5

@@ -132,7 +132,11 @@ pub mod util {

132132 ) -> Option<usize> {

133133 let pos_line = pos.line as usize;

134134 if pos_line > doc.len_lines() - 1 {

135− return None;

135+ // If it extends past the end, truncate it to the end. This is because the

136+ // way the LSP describes the range including the last newline is by

137+ // specifying a line number after what we would call the last line.

138+ log::warn!("LSP position {pos:?} out of range assuming EOF");

139+ return Some(doc.len_chars());

136140 }

137141

138142 // We need to be careful here to fully comply ith the LSP spec.

@@ -242,9 +246,20 @@ pub mod util {

242246

243247 pub fn lsp_range_to_range(

244248 doc: &Rope,

245− range: lsp::Range,

249+ mut range: lsp::Range,

246250 offset_encoding: OffsetEncoding,

247251 ) -> Option<Range> {

252+ // This is sort of an edgecase. It's not clear from the spec how to deal with

253+ // ranges where end < start. They don't make much sense but vscode simply caps start to end

254+ // and because it's not specified quite a few LS rely on this as a result (for example the TS server)

255+ if range.start > range.end {

256+ log::error!(

257+ "Invalid LSP range start {:?} > end {:?}, using an empty range at the end instead",

258+ range.start,

259+ range.end

260+ );

261+ range.start = range.end;

262+ }

248263 let start = lsp_pos_to_pos(doc, range.start, offset_encoding)?;

249264 let end = lsp_pos_to_pos(doc, range.end, offset_encoding)?;

250265

@@ -951,16 +966,16 @@ mod tests {

951966

952967 test_case!("", (0, 0) => Some(0));

953968 test_case!("", (0, 1) => Some(0));

954− test_case!("", (1, 0) => None);

969+ test_case!("", (1, 0) => Some(0));

955970 test_case!("\n\n", (0, 0) => Some(0));

956971 test_case!("\n\n", (1, 0) => Some(1));

957972 test_case!("\n\n", (1, 1) => Some(1));

958973 test_case!("\n\n", (2, 0) => Some(2));

959− test_case!("\n\n", (3, 0) => None);

974+ test_case!("\n\n", (3, 0) => Some(2));

960975 test_case!("test\n\n\n\ncase", (4, 3) => Some(11));

961976 test_case!("test\n\n\n\ncase", (4, 4) => Some(12));

962977 test_case!("test\n\n\n\ncase", (4, 5) => Some(12));

963− test_case!("", (u32::MAX, u32::MAX) => None);

978+ test_case!("", (u32::MAX, u32::MAX) => Some(0));

964979 }

965980

966981 #[test]

helix-term/src/ui/completion.rs+6−10

@@ -141,16 +141,12 @@ impl Completion {

141141 }

142142 };

143143

144− let start_offset =

145− match util::lsp_pos_to_pos(doc.text(), edit.range.start, offset_encoding) {

146− Some(start) => start as i128 - primary_cursor as i128,

147− None => return Transaction::new(doc.text()),

148− };

149− let end_offset =

150− match util::lsp_pos_to_pos(doc.text(), edit.range.end, offset_encoding) {

151− Some(end) => end as i128 - primary_cursor as i128,

152− None => return Transaction::new(doc.text()),

153− };

144+ let Some(range) = util::lsp_range_to_range(doc.text(), edit.range, offset_encoding) else{

145+ return Transaction::new(doc.text());

146+ };

147+

148+ let start_offset = range.anchor as i128 - primary_cursor as i128;

149+ let end_offset = range.head as i128 - primary_cursor as i128;

154150

155151 (Some((start_offset, end_offset)), edit.new_text)

156152 } else {