diff --git a/src/bin/pmacs_fake_lsp.rs b/src/bin/pmacs_fake_lsp.rs index 4f48f87..9be1cd0 100644 --- a/src/bin/pmacs_fake_lsp.rs +++ b/src/bin/pmacs_fake_lsp.rs @@ -38,9 +38,13 @@ //! error — a conforming full-only server, for testing that the //! client never requests delta without the negotiated capability. //! * If launched with `PMACS_FAKE_LSP_MODE=rangeonly`: advertises a -//! range-only `semanticTokensProvider` (`"range": true`, `full` -//! null) and rejects `semanticTokens/full` — per LSP, `full` and -//! `range` are optional, independent capabilities. +//! range-only `semanticTokensProvider` (`"range": true`, no `full`) +//! and rejects `semanticTokens/full` — per LSP, `full` and `range` +//! are optional, independent capabilities. +//! * If launched with `PMACS_FAKE_LSP_MODE=rangeonly16`: `rangeonly` +//! plus UTF-16 position encoding, with strict UTF-16 bounds +//! validation on `/range` — rejects a client that sent raw byte +//! columns for non-ASCII text. //! * If launched with `PMACS_FAKE_LSP_MODE=sighelp`: additionally //! advertises `signatureHelpProvider` with `(` / `,` triggers, so a //! test can drive the Arc 1d auto-trigger. Every other mode omits the @@ -187,13 +191,20 @@ fn main() { // `range` WITHOUT `full` — the /full arm below rejects // in this mode, so a client that ignores the split gets // a visible failure instead of silent staleness. - if mode == "rangeonly" { + if mode.starts_with("rangeonly") { let p = &mut resp["result"]["capabilities"]["semanticTokensProvider"]; if let Some(obj) = p.as_object_mut() { obj.remove("full"); obj.insert("range".into(), serde_json::Value::from(true)); } } + // `rangeonly16` additionally negotiates UTF-16, so the + // /range arm can validate that the client converted its + // byte columns to UTF-16 code units. + if mode == "rangeonly16" { + resp["result"]["capabilities"]["positionEncoding"] = + serde_json::Value::from("utf-16"); + } // Arc 1d: advertise signature help only in `sighelp`, so // every other mode keeps the no-auto-trigger path (the // `textDocument/signatureHelp` arm below still answers @@ -790,7 +801,7 @@ fn main() { ("textDocument/semanticTokens/full", Some(idv)) => { // `rangeonly`: a range-only provider rejects /full — // the client should have sent a range request. - if mode == "rangeonly" { + if mode.starts_with("rangeonly") { let resp = serde_json::json!({ "jsonrpc": "2.0", "id": idv, @@ -834,6 +845,27 @@ fn main() { write_frame(&mut stdout, &resp); } ("textDocument/semanticTokens/range", Some(idv)) => { + // `rangeonly16`: strict bounds validation in UTF-16 + // units. A client that sent raw byte columns for + // non-ASCII text overshoots the last line's UTF-16 + // length and is rejected — the fixture for the + // outbound-position conversion. + if mode == "rangeonly16" + && let Ok(sink) = std::env::var("PMACS_FAKE_RANGE_SINK") + { + let _ = std::fs::write(&sink, format!("{params}")); + } + if mode == "rangeonly16" + && let Some(message) = utf16_range_error(¶ms, &open_docs) + { + let resp = serde_json::json!({ + "jsonrpc": "2.0", + "id": idv, + "error": { "code": -32602, "message": message } + }); + write_frame(&mut stdout, &resp); + continue; + } // T M4.5: same shape as /full, scoped to a range. // One token: line 1 col 0 len 3, variable. let resp = serde_json::json!({ @@ -1053,6 +1085,47 @@ fn document_end_position(text: &str) -> (u64, u64) { (line, col) } +/// `rangeonly16` bounds validation: the request's end position must not +/// exceed the document end measured in UTF-16 code units (the +/// negotiated encoding). Byte-column overshoot on non-ASCII text is +/// exactly the client bug this catches. +fn utf16_range_error( + params: &serde_json::Value, + open_docs: &HashMap, +) -> Option { + // Fail-CLOSED: a fixture that silently skips validation on an + // unexpected state (missing uri / unrecorded doc) reads as a pass. + let Some(uri) = params + .get("textDocument") + .and_then(|t| t.get("uri")) + .and_then(serde_json::Value::as_str) + else { + return Some("range request carried no textDocument.uri".into()); + }; + let Some(text) = open_docs.get(uri) else { + return Some(format!("no didOpen text recorded for {uri}")); + }; + let (mut line, mut col) = (0u64, 0u64); + for ch in text.chars() { + if ch == '\n' { + line += 1; + col = 0; + } else { + col += ch.len_utf16() as u64; + } + } + let end = params.get("range")?.get("end")?; + let end_line = end.get("line")?.as_u64()?; + let end_col = end.get("character")?.as_u64()?; + if end_line > line || (end_line == line && end_col > col) { + Some(format!( + "invalid utf-16 range end {end_line}:{end_col}; document ends at {line}:{col}" + )) + } else { + None + } +} + fn inlay_range_error( params: &serde_json::Value, open_docs: &HashMap, diff --git a/src/lsp.rs b/src/lsp.rs index e777c98..9849b3e 100644 --- a/src/lsp.rs +++ b/src/lsp.rs @@ -2003,11 +2003,16 @@ impl LspManager { diagnostics: &[Value], ) -> Result { let uri = uri.into(); + // Same encoding rule as every outbound range: columns are pmacs + // byte offsets and must convert to the negotiated units — a + // UTF-16 server receiving byte columns resolves the wrong range + // for non-ASCII lines (same bug class as the semantic-token + // range fix in this change). let params = json!({ "textDocument": { "uri": uri.clone() }, "range": { - "start": { "line": start_line, "character": start_col }, - "end": { "line": end_line, "character": end_col }, + "start": self.outbound_position(sid, &uri, start_line, start_col), + "end": self.outbound_position(sid, &uri, end_line, end_col), }, "context": { "diagnostics": diagnostics }, }); @@ -2038,7 +2043,7 @@ impl LspManager { "textDocument": { "uri": uri.clone() }, "range": { "start": self.outbound_position(sid, &uri, start_line, start_col), - "end": self.outbound_position(sid, &uri, end_line, end_col), + "end": self.outbound_position(sid, &uri, end_line, end_col), }, }); let req_id = self.send_request(sid, "textDocument/inlayHint", params)?; @@ -2081,11 +2086,17 @@ impl LspManager { end_col: u32, ) -> Result { let uri = uri.into(); + // Columns arrive as pmacs byte offsets; convert per the + // negotiated position encoding, exactly like the inlay-hint + // range. A UTF-16 server receiving raw byte columns gets an + // invalid end character for non-ASCII text ("é" is two bytes + // but one UTF-16 unit) and may reject the request — fatal for + // a range-only provider whose ONLY pull path this is. let params = json!({ "textDocument": { "uri": uri.clone() }, "range": { - "start": { "line": start_line, "character": start_col }, - "end": { "line": end_line, "character": end_col }, + "start": self.outbound_position(sid, &uri, start_line, start_col), + "end": self.outbound_position(sid, &uri, end_line, end_col), }, }); let req_id = self.send_request(sid, "textDocument/semanticTokens/range", params)?; diff --git a/tests/m4_acceptance.rs b/tests/m4_acceptance.rs index 8d4dd16..bb8b77f 100644 --- a/tests/m4_acceptance.rs +++ b/tests/m4_acceptance.rs @@ -4582,6 +4582,59 @@ fn arc1c_range_only_server_is_served_range_requests() { ); } +/// Arc 1c review fix — a range-only server that negotiated UTF-16. +/// The whole-document range's columns are derived from pmacs byte +/// offsets; they must go through `outbound_position` like every other +/// outbound position. The last line ends in non-ASCII ("é" = 2 bytes, +/// 1 UTF-16 unit), and the fake validates the end bound strictly in +/// UTF-16 units — raw byte columns overshoot and are rejected, leaving +/// the store empty and this test failing. +#[test] +fn arc1c_range_only_utf16_server_gets_converted_bounds() { + use pmacs::editor::EditorState; + + let dir = tempfile::tempdir().expect("tempdir"); + let a_path = dir.path().join("a.rs"); + std::fs::write(&a_path, "fn a() {}\nlet x = \u{e9}\u{e9};".as_bytes()).expect("write a"); + let a_disp = a_path.display().to_string(); + + let mut state = EditorState::new(); + let fake = fake_lsp_path(); + state + .lua_host + .lua() + .load(format!( + "pmacs.lsp.config.rust = {{ + command = '{fake}', + env = {{ PMACS_FAKE_LSP_MODE = 'rangeonly16' }}, + }}" + )) + .exec() + .expect("override rust config"); + state + .lua_host + .lua() + .load(format!("pmacs.buffer.find_or_open('{a_disp}')")) + .exec() + .expect("open a.rs"); + + let has_tokens = format!( + "(function() \ + local sid \ + for _,r in ipairs(pmacs.lsp.list()) do \ + if r.state and r.state.kind=='initialized' then sid=r.id end \ + end \ + if not sid then return false end \ + local t = pmacs.semantic_tokens.tokens(sid, 'file://{a_disp}') \ + return t ~= nil and #t > 0 \ + end)()" + ); + assert!( + pump_lua_flag(&mut state, &has_tokens, 5), + "a UTF-16 range-only server must receive converted (not byte) columns" + ); +} + /// Arc 1d — a server-declared NON-ASCII trigger character works. LSP /// trigger characters are strings; the fake declares "«" (2 UTF-8 /// bytes), and the codepoint-aware `char_before` must match it.