diff --git a/CHANGELOG.md b/CHANGELOG.md index e252e30..c54f40c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -255,6 +255,36 @@ All notable changes to EigenScript are documented here. ### Fixed +- **EigenScript could not read a CRLF source file at all, and that is + why the LSP was useless on Windows documents (#880).** The issue named + the JSON-RPC unescaper, which was real: `src/eigenlsp.c` and + `src/eigsdap.c` each hand-rolled the same five-escape subset, dropped + `\r`, `\b`, `\f` and `\uXXXX`, and re-emitted the backslash — so a + Windows client's text arrived with a literal backslash-r in it. But + fixing that only got the CR as far as the lexer, which rejected it: + `eigenscript win.eigs` died with `unexpected character` on **every + line**. Not a tooling bug — the language, and a straight blocker on + the Windows Tier-1 roadmap. The lexer now treats the CR of a CRLF pair + as whitespace outside string literals (a CR *inside* a literal is data + and is preserved; a lone CR is deliberately not a line break). + Rather than write a sixth escape decoder, the runtime's own JSON + string decoder — the one carrying #724's surrogate-pair handling — was + extracted as `eigs_json_decode_string_body`, and all three callers now + share it. Which made a third bug visible: **`json_decode` was missing + `\b` and `\f` too.** Its default arm dropped the backslash, so + `json_decode of "a\bc"` silently returned `abc` — the same subset gap, + in the language's primary parser rather than in the tooling. A CRLF + document now yields diagnostics byte-identical to the LF one. + +- **The LSP advertises `positionEncoding: utf-8` (#881).** Positions are + byte offsets, which is deliberate — `LANGUAGE_CONTRACT.md` makes the + byte model a language-level promise (`len of "café"` is 5) — so the + server was internally consistent. The defect was not *saying* so: LSP + 3.17 reads a server that negotiates no `positionEncoding` as `utf-16`, + so clients decoded byte offsets as UTF-16 code units and every range + after a non-ASCII character landed in the wrong place, drifting + further along the line with each one. + - **`unobserved:` leaked its depth on every exit edge but one, silently killing the observer for the rest of the process (#871, found while fixing it).** `g_unobserved_depth` is a runtime counter that only diff --git a/src/builtins.c b/src/builtins.c index 9645c6f..1cfcb9f 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1072,11 +1072,16 @@ static int eigs_json_read_hex4(const char *s, int *pos, unsigned int *out) { return 1; } -static Value* eigs_json_parse_string(const char *s, int *pos) { - if (s[*pos] != '"') { g_json_parse_err = 1; return NULL; } /* #495 */ - (*pos)++; - strbuf buf; - strbuf_init(&buf); +/* #880: THE JSON string-body decoder. `s[*pos]` is the first byte after the + * opening quote; decodes into `out` and leaves *pos past the closing quote. + * + * Extracted so the LSP and DAP JSON-RPC readers share it instead of each + * hand-rolling a five-escape subset. Both of those dropped \r, \b, \f and + * \uXXXX and re-emitted the backslash verbatim, which made every CRLF + * document unusable — JSON requires a CR to be escaped, so a Windows client's + * text arrived with literal backslash-r in it and produced a bogus syntax + * error and zero real diagnostics. */ +void eigs_json_decode_string_body(const char *s, int *pos, strbuf *out) { while (s[*pos] && s[*pos] != '"') { if (s[*pos] == '\\') { (*pos)++; @@ -1089,12 +1094,17 @@ static Value* eigs_json_parse_string(const char *s, int *pos) { * fix one layer down. */ if (s[*pos] == '\0') break; switch (s[*pos]) { - case '"': strbuf_append_char(&buf, '"'); break; - case '\\': strbuf_append_char(&buf, '\\'); break; - case 'n': strbuf_append_char(&buf, '\n'); break; - case 'r': strbuf_append_char(&buf, '\r'); break; - case 't': strbuf_append_char(&buf, '\t'); break; - case '/': strbuf_append_char(&buf, '/'); break; + case '"': strbuf_append_char(out, '"'); break; + case '\\': strbuf_append_char(out, '\\'); break; + case 'n': strbuf_append_char(out, '\n'); break; + case 'r': strbuf_append_char(out, '\r'); break; + case 't': strbuf_append_char(out, '\t'); break; + /* #880: \b and \f are RFC 8259 escapes and were missing + * here as well — the default arm dropped the backslash, so + * json_decode silently turned "a\bc" into "abc". */ + case 'b': strbuf_append_char(out, '\b'); break; + case 'f': strbuf_append_char(out, '\f'); break; + case '/': strbuf_append_char(out, '/'); break; case 'u': { unsigned int cp; if (!eigs_json_read_hex4(s, pos, &cp)) { @@ -1103,7 +1113,7 @@ static Value* eigs_json_parse_string(const char *s, int *pos) { * decode raises; lenient callers get U+FFFD and the * offending text is parsed normally from here. */ g_json_parse_recoverable = 1; - eigs_json_append_cp(&buf, 0xFFFD); + eigs_json_append_cp(out, 0xFFFD); break; } if (cp >= 0xD800 && cp <= 0xDBFF) { @@ -1124,34 +1134,42 @@ static Value* eigs_json_parse_string(const char *s, int *pos) { } if (paired) { cp = 0x10000u + ((cp - 0xD800u) << 10) + (lo - 0xDC00u); - eigs_json_append_cp(&buf, cp); + eigs_json_append_cp(out, cp); } else { g_json_parse_recoverable = 1; - eigs_json_append_cp(&buf, 0xFFFD); + eigs_json_append_cp(out, 0xFFFD); } } else if (cp >= 0xDC00 && cp <= 0xDFFF) { /* #724: lone low surrogate — strict raise + U+FFFD */ g_json_parse_recoverable = 1; - eigs_json_append_cp(&buf, 0xFFFD); + eigs_json_append_cp(out, 0xFFFD); } else if (cp == 0) { /* #724: NUL cannot live in a C-terminated string * (EMBEDDING.md) — strict raise + lenient U+FFFD. */ g_json_parse_recoverable = 1; - eigs_json_append_cp(&buf, 0xFFFD); + eigs_json_append_cp(out, 0xFFFD); } else { - eigs_json_append_cp(&buf, cp); + eigs_json_append_cp(out, cp); } break; } - default: strbuf_append_char(&buf, s[*pos]); break; + default: strbuf_append_char(out, s[*pos]); break; } } else { - strbuf_append_char(&buf, s[*pos]); + strbuf_append_char(out, s[*pos]); } (*pos)++; } if (s[*pos] == '"') (*pos)++; else g_json_parse_err = 1; /* #495: unterminated string (hit EOF) */ +} + +static Value* eigs_json_parse_string(const char *s, int *pos) { + if (s[*pos] != '"') { g_json_parse_err = 1; return NULL; } /* #495 */ + (*pos)++; + strbuf buf; + strbuf_init(&buf); + eigs_json_decode_string_body(s, pos, &buf); Value *v = make_str(buf.data); strbuf_free(&buf); return v; diff --git a/src/eigenlsp.c b/src/eigenlsp.c index ba8c99c..4d501fd 100644 --- a/src/eigenlsp.c +++ b/src/eigenlsp.c @@ -124,21 +124,14 @@ static char* json_get_string(const char *json, const char *key) { p++; strbuf sb; strbuf_init(&sb); - while (*p && *p != '"') { - if (*p == '\\' && *(p+1)) { - p++; - switch (*p) { - case 'n': strbuf_append_char(&sb, '\n'); break; - case 't': strbuf_append_char(&sb, '\t'); break; - case '\\': strbuf_append_char(&sb, '\\'); break; - case '"': strbuf_append_char(&sb, '"'); break; - case '/': strbuf_append_char(&sb, '/'); break; - default: strbuf_append_char(&sb, '\\'); strbuf_append_char(&sb, *p); break; - } - } else { - strbuf_append_char(&sb, *p); - } - p++; + /* #880: one shared decoder with json_decode (eigenscript.h). The local + * five-escape switch that used to live here dropped \r, \b, \f and + * \uXXXX and re-emitted the backslash verbatim — so a CRLF document + * arrived with literal backslash-r in its text and every Windows client + * got a bogus syntax error and zero real diagnostics. */ + { + int pos = 0; + eigs_json_decode_string_body(p, &pos, &sb); } return strbuf_finish(&sb); } @@ -687,6 +680,15 @@ static void handle_initialize(int id) { lsp_response(id, "{" "\"capabilities\":{" + /* #881: positions here are BYTE offsets, which is deliberate — + * LANGUAGE_CONTRACT.md makes the byte model a language-level + * promise (`len of "café"` is 5), so the server is internally + * consistent. The defect was not saying so: LSP 3.17 treats a + * server that negotiates no positionEncoding as utf-16, so a + * client decoded byte offsets as UTF-16 code units and every + * range after a non-ASCII character landed in the wrong place, + * drifting further along the line with each one. */ + "\"positionEncoding\":\"utf-8\"," "\"textDocumentSync\":1," "\"completionProvider\":{\"triggerCharacters\":[\".\",\" \"]}," "\"hoverProvider\":true," diff --git a/src/eigenscript.h b/src/eigenscript.h index 3d9b45b..b346bfe 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -1199,6 +1199,11 @@ char* value_to_string(Value *v); void eigs_num_text(char *buf, size_t nbuf, double n); void observer_ensure_fresh(Value *v); void eigs_json_escape_string(strbuf *out, const char *s); +/* #880: decode a JSON string body (s[*pos] = first byte after the opening + * quote) into `out`, leaving *pos past the closing quote. One decoder for + * json_decode, the LSP, and the DAP — they used to disagree on which escapes + * exist. */ +void eigs_json_decode_string_body(const char *s, int *pos, strbuf *out); /* ---- Registration ---- */ diff --git a/src/eigsdap.c b/src/eigsdap.c index e6f9daf..e7697de 100644 --- a/src/eigsdap.c +++ b/src/eigsdap.c @@ -83,22 +83,14 @@ static char *json_get_string(const char *json, const char *key) { p++; strbuf sb; strbuf_init(&sb); - while (*p && *p != '"') { - if (*p == '\\' && p[1]) { - p++; - switch (*p) { - case 'n': strbuf_append_char(&sb, '\n'); break; - case 't': strbuf_append_char(&sb, '\t'); break; - case '\\': strbuf_append_char(&sb, '\\'); break; - case '"': strbuf_append_char(&sb, '"'); break; - case '/': strbuf_append_char(&sb, '/'); break; - default: strbuf_append_char(&sb, '\\'); - strbuf_append_char(&sb, *p); break; - } - } else { - strbuf_append_char(&sb, *p); - } - p++; + /* #880: one shared decoder with json_decode (eigenscript.h). The local + * five-escape switch that used to live here dropped \r, \b, \f and + * \uXXXX and re-emitted the backslash verbatim — so a CRLF document + * arrived with literal backslash-r in its text and every Windows client + * got a bogus syntax error and zero real diagnostics. */ + { + int pos = 0; + eigs_json_decode_string_body(p, &pos, &sb); } return strbuf_finish(&sb); } diff --git a/src/lexer.c b/src/lexer.c index c69723f..a25ede0 100644 --- a/src/lexer.c +++ b/src/lexer.c @@ -254,6 +254,10 @@ TokenList tokenize(const char *source) { if (*p == '\n') { p++; line++; col = 0; } continue; } + /* #880: a CRLF blank line — swallow the CR so the '\n' below sees + * an empty line rather than falling through to indent handling + * with a stray carriage return as the first "real" character. */ + if (*p == '\r' && p[1] == '\n') p++; if (*p == '\n') { p++; line++; col = 0; continue; @@ -283,7 +287,15 @@ TokenList tokenize(const char *source) { at_line_start = 0; } - if (*p == ' ' || *p == '\t') { + /* #880: EigenScript could not read a CRLF source file at all — + * `eigenscript win.eigs` died with "unexpected character" on every + * line, which also made the language server useless on any document + * a Windows editor saved. The CR of a CRLF pair is skipped here so + * the '\n' does the line break; a CR inside a string LITERAL is + * untouched (that path scans its own bytes), so a program that + * genuinely embeds one is unaffected. A lone CR as a line terminator + * (classic Mac, pre-OS X) is deliberately not a line break. */ + if (*p == ' ' || *p == '\t' || (*p == '\r' && p[1] == '\n')) { p++; col++; continue; } diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index c61ddfe..d5891f5 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -4325,6 +4325,48 @@ else echo "$SK_LIVE" | head -5 fi rm -rf "$SK_DIR" +# [99k] CRLF source files (#880). EigenScript could not read one AT ALL — +# `eigenscript win.eigs` died with "unexpected character" on every line — which +# is also why the language server was useless on any document a Windows editor +# saved: fixing the JSON-RPC unescaper only got the CR as far as the lexer, +# which then rejected it. +echo "[99k] CRLF source files (#880)" +CR_DIR=$(mktemp -d /tmp/eigs_crlf880_XXXX) +printf 'a is 1\r\n\r\nif a > 0:\r\n print of "crlf works"\r\n' > "$CR_DIR/win.eigs" +CR_OUT=$(./eigenscript "$CR_DIR/win.eigs" 2>&1); CR_RC=$? +TOTAL=$((TOTAL + 1)) +if [ "$CR_RC" -eq 0 ] && [ "$CR_OUT" = "crlf works" ]; then + PASS=$((PASS + 1)) + echo " PASS: a CRLF source file runs (was: 'unexpected character' on every line)" +else + FAIL=$((FAIL + 1)) + echo " FAIL: CRLF source file should run (rc=$CR_RC out=$CR_OUT)" +fi + +# A CR inside a string LITERAL is data, not a line ending, and must survive. +printf 'lit is "a\rb"\r\nprint of (str of (len of lit))\r\n' > "$CR_DIR/lit.eigs" +CR_LIT=$(./eigenscript "$CR_DIR/lit.eigs" 2>&1); CR_LIT_RC=$? +TOTAL=$((TOTAL + 1)) +if [ "$CR_LIT_RC" -eq 0 ] && [ "$CR_LIT" = "3" ]; then + PASS=$((PASS + 1)) + echo " PASS: a CR inside a string literal is preserved as data" +else + FAIL=$((FAIL + 1)) + echo " FAIL: CR inside a string literal must be preserved (rc=$CR_LIT_RC out=$CR_LIT)" +fi + +# LF files must be byte-for-byte unaffected. +printf 'a is 1\n\nif a > 0:\n print of "lf works"\n' > "$CR_DIR/lf.eigs" +CR_LF=$(./eigenscript "$CR_DIR/lf.eigs" 2>&1); CR_LF_RC=$? +TOTAL=$((TOTAL + 1)) +if [ "$CR_LF_RC" -eq 0 ] && [ "$CR_LF" = "lf works" ]; then + PASS=$((PASS + 1)) + echo " PASS: LF sources are unaffected" +else + FAIL=$((FAIL + 1)) + echo " FAIL: LF sources must be unaffected (rc=$CR_LF_RC out=$CR_LF)" +fi +rm -rf "$CR_DIR" echo "" # [99i] Uniform -Werror=switch gate (#817 follow-up; #835 extended it to diff --git a/tests/test_json_hard.eigs b/tests/test_json_hard.eigs index e9bf5db..99d3076 100644 --- a/tests/test_json_hard.eigs +++ b/tests/test_json_hard.eigs @@ -421,4 +421,19 @@ assert of [jb_huge == ("{\"v\": " + (str of 1.23e15) + "}"), "JH97 json_build ag jb_huge2 is json_build of ["v", 1.23e300] assert of [jb_huge2 == "{\"v\": 1.23e+300}", "JH97 json_build keeps exponent form past 2^53"] +# --- #880: \b and \f are RFC 8259 escapes and were missing from the decoder. +# The default arm dropped the backslash, so json_decode silently turned +# "a\bc" into "abc" — the same subset gap that made the LSP unusable on CRLF. +esc_b is json_decode of "{\"s\": \"a\\bc\"}" +assert of [len of esc_b.s == 3, "JH98 backslash-b is one character"] +assert of [ord of (char_at of [esc_b.s, 1]) == 8, "JH98 backslash-b is BACKSPACE (0x08), not 'b'"] + +esc_f is json_decode of "{\"s\": \"a\\fc\"}" +assert of [len of esc_f.s == 3, "JH99 backslash-f is one character"] +assert of [ord of (char_at of [esc_f.s, 1]) == 12, "JH99 backslash-f is FORM FEED (0x0c), not 'f'"] + +# \r was already handled here; pin it alongside so the trio cannot regress. +esc_r2 is json_decode of "{\"s\": \"a\\rc\"}" +assert of [ord of (char_at of [esc_r2.s, 1]) == 13, "JH100 backslash-r is CR (0x0d)"] + print of "json hard: all passed" diff --git a/tests/test_lsp.py b/tests/test_lsp.py index 628fd3c..11cbe4a 100755 --- a/tests/test_lsp.py +++ b/tests/test_lsp.py @@ -142,6 +142,13 @@ def main(): check("advertises documentFormattingProvider", caps.get("documentFormattingProvider") is True) check("advertises renameProvider", caps.get("renameProvider") is True) check("advertises codeActionProvider", caps.get("codeActionProvider") is True) + # #881: positions here are BYTE offsets, which is deliberate (the byte + # model is a language-level promise). The defect was not SAYING so: LSP + # 3.17 reads a server that negotiates nothing as utf-16, so a client + # decoded byte offsets as UTF-16 code units and every range after a + # non-ASCII character landed in the wrong place. + check("advertises positionEncoding utf-8 (#881)", + caps.get("positionEncoding") == "utf-8") stp = caps.get("semanticTokensProvider") check("advertises semanticTokensProvider (full)", isinstance(stp, dict) and stp.get("full") is True) @@ -618,6 +625,24 @@ def main(): check("semanticTokens carries accurate lengths (22 → len 2)", any(ty == ni and L == 2 for (_, _, L, ty) in toks)) + # #880: a CRLF document must behave exactly like the LF one. The + # JSON-RPC unescaper used to drop \r (re-emitting the backslash), so a + # Windows document arrived with literal backslash-r in its text and + # produced one bogus syntax error and ZERO real diagnostics. + crlf_src = "a is 1\nb is 2\nc is undefined_thing\n" + lf_diags = diagnostics(converse([did_open(crlf_src)])) + crlf_diags = diagnostics(converse([did_open(crlf_src.replace("\n", "\r\n"))])) + + def shape(ds): + return sorted((d["range"]["start"]["line"], d["range"]["start"]["character"], + d.get("code", ""), d["message"]) for d in ds) + + check("CRLF document yields real diagnostics, not a syntax error (#880)", + len(crlf_diags) > 0 and + not any("unexpected character" in d["message"] for d in crlf_diags)) + check("CRLF document diagnostics match the LF ones exactly (#880)", + shape(crlf_diags) == shape(lf_diags)) + # If the LSP was built under a sanitizer, fail on any report it emitted. check("no sanitizer reports from the LSP process", not SANITIZER_HITS) if SANITIZER_HITS: