Skip to content

lsp/dap: one JSON string decoder, and the lexer reads CRLF (#880, #881) - #900

Merged
InauguralPhysicist merged 1 commit into
mainfrom
fix/880-881-lsp-escapes
Aug 6, 2026
Merged

lsp/dap: one JSON string decoder, and the lexer reads CRLF (#880, #881)#900
InauguralPhysicist merged 1 commit into
mainfrom
fix/880-881-lsp-escapes

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Three bugs, stacked. Fixing only the filed one leaves the issue's own acceptance criterion unmet.

1. The filed defect — the JSON-RPC unescaper

src/eigenlsp.c and src/eigsdap.c each hand-rolled the same five-escape subset, dropped \r, \b, \f and \uXXXX, and had a default: arm that re-emitted the backslash verbatim. JSON requires a CR to be escaped, so a Windows client's document arrived with a literal backslash-r in its text.

2. The layer underneath — the language can't read CRLF

With the CR correctly decoded, it reached the lexer, which rejected it:

$ eigenscript win.eigs          # any file saved by a Windows editor
Syntax error line 1: unexpected character ''
Syntax error line 2: unexpected character ''
2 parse error(s) — aborting

EigenScript could not read a CRLF source file at all. Not a tooling bug — the language, and a straight blocker on the Windows Tier-1 roadmap. It's also why the LSP was unusable: the unescaper was only the first of two gates.

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 — pinned by a test, because the tempting fix (normalize the whole buffer to LF) would silently change such a program and shift the byte offsets the LSP reports as positions.

A lone CR (classic pre-OS X Mac) is deliberately not a line break.

3. The one the extraction exposed — json_decode itself

Rather than write a sixth escape decoder, I extracted the runtime's own JSON string decoder — the one already carrying #724's surrogate-pair handling and #776's trailing-backslash overflow guard — as eigs_json_decode_string_body, shared by json_decode, the LSP and the DAP.

That made a third bug visible: json_decode was missing \b and \f too. Its default: arm dropped the backslash, so:

json_decode of "{\"s\": \"a\\bc\"}"    # -> "abc", silently. 'b', not 0x08.

Same subset gap, in the language's primary parser rather than in the tooling.

#881 — say what the encoding is

Positions are byte offsets, which is the correct choice here: LANGUAGE_CONTRACT.md makes the byte model a language-level promise (len of "café" is 5). 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. Now "positionEncoding":"utf-8".

Verification

A CRLF document now yields diagnostics byte-identical to the LF document:

--- LF endings ---                    --- CRLF endings ---
line=0 col=0 W001 unused 'a'          line=0 col=0 W001 unused 'a'
line=1 col=0 W001 unused 'b'          line=1 col=0 W001 unused 'b'
line=2 col=5 E003 undefined name…     line=2 col=5 E003 undefined name…
line=2 col=0 W001 unused 'c'          line=2 col=0 W001 unused 'c'

tests/test_lsp.py compares the full diagnostic shape, not a count — the old failure produced one bogus syntax error and zero real diagnostics, which a count-only check could have passed.

New suite section [99k]: a CRLF script runs, a CR inside a string literal survives as data, and LF sources are unaffected. JH98–JH100 pin \b, \f and \r in json_decode.

  • Release suite: 3794/3794
  • ASan + UBSan, detect_leaks=1: 3792/3792, leak tally 0
  • LSP suite: 85/85

Closes #880
Closes #881

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings August 5, 2026 23:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Windows/CRLF interoperability across the EigenScript toolchain by (1) centralizing JSON string escape decoding so json_decode, the LSP, and the DAP all interpret JSON-RPC payloads consistently, and (2) teaching the lexer to accept CRLF source files without changing string-literal semantics or byte-offset accounting.

Changes:

  • Extracted and reused the runtime JSON string-body decoder (eigs_json_decode_string_body) for json_decode, eigenlsp, and eigsdap, including missing \b/\f handling and existing \uXXXX/surrogate-pair logic.
  • Updated the lexer to treat the CR in CRLF as ignorable whitespace outside string literals so CRLF files tokenize/parse correctly.
  • Declared LSP positionEncoding: "utf-8" and added targeted regression tests (LSP diagnostic shape equivalence for LF vs CRLF; JSON escape hard cases; suite section [99k]).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/test_lsp.py Asserts positionEncoding: utf-8 and pins LF vs CRLF diagnostic equivalence.
tests/test_json_hard.eigs Adds regression coverage for json_decode handling of \b, \f, and \r.
tests/run_all_tests.sh Adds suite section [99k] covering CRLF scripts and CR-in-literal preservation.
src/lexer.c Accepts CRLF sources by skipping the CR of CRLF pairs outside literals.
src/eigsdap.c Replaces local JSON unescape logic with shared runtime decoder.
src/eigenscript.h Exposes eigs_json_decode_string_body for shared use.
src/eigenlsp.c Replaces local JSON unescape logic with shared decoder; advertises positionEncoding: utf-8.
src/builtins.c Extracts the JSON string-body decode routine and adds \b/\f support.
CHANGELOG.md Documents CRLF + LSP/DAP decoder fixes and the positionEncoding change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings August 6, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/test_lsp.py:634

  • The new CRLF diagnostic check sends textDocument/didOpen without the required initialize/shutdown/exit handshake (unlike the rest of this file). That makes the test less representative of real clients and could break if the server later starts enforcing protocol ordering (or changes its EOF behavior).
    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"))]))

Three bugs, stacked. Fixing only the filed one leaves the issue's own
acceptance criterion unmet.

1. The filed defect. src/eigenlsp.c and src/eigsdap.c each hand-rolled
   the same five-escape subset and dropped \r, \b, \f and \uXXXX, with a
   default arm that re-emitted the backslash verbatim. JSON requires a CR
   to be escaped, so a Windows client's document arrived with a literal
   backslash-r in its text.

2. The layer underneath. With the CR decoded, it reached the LEXER,
   which rejected it — EigenScript could not read a CRLF source file AT
   ALL:

       $ eigenscript win.eigs
       Syntax error line 1: unexpected character ''
       Syntax error line 2: unexpected character ''

   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 (pinned by a test — a normalize-the-whole-buffer fix
   would have silently changed such a program, and would also have
   shifted the byte offsets the LSP reports). A lone CR is deliberately
   not a line break.

3. The one the extraction exposed. Rather than add a sixth copy of an
   escape decoder, the runtime's own JSON string decoder — the one
   carrying #724's surrogate-pair handling and #776's trailing-backslash
   overflow guard — is now eigs_json_decode_string_body, shared by
   json_decode, the LSP and the DAP. That made visible that json_decode
   ITSELF was missing \b and \f: its default arm dropped the backslash,
   so `json_decode of "a\bc"` silently returned "abc". Same subset gap,
   in the language's primary parser rather than the tooling.

the correct choice here — LANGUAGE_CONTRACT.md makes the byte model a
language-level promise — so the server was internally consistent; the
defect was not SAYING so. LSP 3.17 reads a server that negotiates
nothing 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.

Verification: a CRLF document now yields diagnostics byte-identical to
the LF document (tests/test_lsp.py compares the full shape, not just a
count — the old failure produced one bogus syntax error and zero real
diagnostics, which a count check could have passed). New suite section
[99k] covers a CRLF script running, a CR inside a string literal
surviving, and LF sources being unaffected. JH98-JH100 pin \b, \f and \r
in json_decode.

Suite 3794/3794 release, 3792/3792 ASan+UBSan with detect_leaks=1, leak
tally 0. LSP suite 85/85.

Closes #880
Closes #881

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 6, 2026 02:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/builtins.c:1156

  • In eigs_json_decode_string_body(), the default: escape arm still accepts unknown JSON escapes (e.g. \q) silently: it drops the backslash and appends the following byte without setting either parse flag. This means strict json_decode can succeed on invalid JSON strings, contradicting the strict-decode intent documented in builtin_json_decode and RFC 8259 escape rules.
                default: strbuf_append_char(out, s[*pos]); break;

@InauguralPhysicist
InauguralPhysicist merged commit 7af4495 into main Aug 6, 2026
19 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the fix/880-881-lsp-escapes branch August 6, 2026 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants