From 2a9fa0e2dc54af138ec6e3f9c849fabfcf9a57c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Varl=C4=B1?= Date: Mon, 6 Jan 2025 05:22:28 +0000 Subject: [PATCH] Ensure `end >= start` in `lsp::Range` (#22690) Should resolve https://github.com/zed-industries/zed/issues/21714. In some conditions that I'm not sure of, Zed sends LSP requests with `start > end` position, and zls has an [assertion for end >= start](https://github.com/zigtools/zls/blob/f253553b8230ecd94824ecc3e2da2eb1643e7512/src/offsets.zig#L492), and that causes zls to crash, like: ```bash # first `textDocument/inlayHint` request with `end >= start` [2025-01-05T19:33:09+00:00 TRACE lsp] outgoing message:{"jsonrpc":"2.0","id":1043,"method":"textDocument/inlayHint","params":{"textDocument":{"uri":"file:///Users/burak/Code/parzig/src/parquet/decoding.zig"},"range":{"start":{"line":0,"character":0},"end":{"line":24,"character":0}}}} # successful response [2025-01-05T19:33:09+00:00 TRACE lsp::input_handler] incoming message: {"jsonrpc":"2.0","id":1043,"result":[{"position":{"line":0,"character":9},"label":": type","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":1,"character":22},"label":": type","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":4,"character":13},"label":": [](unknown type)","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":4,"character":30},"label":"T:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\ncomptime type\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":4,"character":33},"label":"n:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\nusize\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":5,"character":23},"label":": bool","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":6,"character":19},"label":": usize","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":9,"character":26},"label":": [](unknown type)","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":9,"character":43},"label":"T:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\ncomptime type\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":9,"character":47},"label":"n:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\nusize\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":21,"character":13},"label":": [](unknown type)","kind":1,"paddingLeft":false,"paddingRight":false},{"position":{"line":21,"character":30},"label":"T:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\ncomptime type\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":21,"character":33},"label":"n:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\nusize\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":22,"character":33},"label":"T:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\ncomptime type\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":22,"character":36},"label":"buf:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\n[]T\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":22,"character":41},"label":"bit_width:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\nu8\n```"},"paddingLeft":false,"paddingRight":true},{"position":{"line":22,"character":52},"label":"reader:","kind":2,"tooltip":{"kind":"markdown","value":"```zig\nanytype\n```"},"paddingLeft":false,"paddingRight":true}]} [2025-01-05T19:33:09+00:00 TRACE lsp] Took 14.855ms to receive response to "textDocument/inlayHint" id 1043 # problematic `textDocument/inlayHint` request with `start > end` [2025-01-05T19:33:09+00:00 TRACE lsp] outgoing message:{"jsonrpc":"2.0","id":1044,"method":"textDocument/inlayHint","params":{"textDocument":{"uri":"file:///Users/burak/Code/parzig/src/parquet/decoding.zig"},"range":{"start":{"line":50,"character":25},"end":{"line":25,"character":0}}}} # zls crashes here, and after this point, all LSP requests fail [2025-01-05T19:33:09+00:00 TRACE lsp] incoming stderr message:thread 5391652 panic: reached unreachable code [2025-01-05T19:33:09+00:00 ERROR lsp] cannot read LSP message headers ``` In LSP specification for [`Range`](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range) type, it says: > ... If you want to specify a range that contains a line including the line ending character(s) then use an end position denoting the start of the next line. I feel like zls's assertion is sensible, so I've updated the generic `range_to_lsp` function rather than doing something specific to zls. But let me know if this seems incorrect. zls was crashing after 5-10 minutes of working with a Zig codebase before, and after this change, I tested for an hour and didn't experience any crashes. Release Notes: - Ensure `end >= start` in `lsp::Range`, which should fix Zig/zls crashes. --------- Co-authored-by: Michael Sloan --- crates/language/src/language.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 9e006f72ad..2eb1532ff3 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -1850,16 +1850,20 @@ pub fn point_from_lsp(point: lsp::Position) -> Unclipped { } pub fn range_to_lsp(range: Range) -> lsp::Range { - lsp::Range { - start: point_to_lsp(range.start), - end: point_to_lsp(range.end), + let mut start = point_to_lsp(range.start); + let mut end = point_to_lsp(range.end); + if start > end { + log::error!("range_to_lsp called with inverted range {start:?}-{end:?}"); + mem::swap(&mut start, &mut end); } + lsp::Range { start, end } } pub fn range_from_lsp(range: lsp::Range) -> Range> { let mut start = point_from_lsp(range.start); let mut end = point_from_lsp(range.end); if start > end { + log::warn!("range_from_lsp called with inverted range {start:?}-{end:?}"); mem::swap(&mut start, &mut end); } start..end