Skip to content

fix(syntax): implement Janet backtick long strings - #91

Merged
takeokunn merged 1 commit into
mainfrom
fix/janet-long-strings
Aug 3, 2026
Merged

fix(syntax): implement Janet backtick long strings#91
takeokunn merged 1 commit into
mainfrom
fix/janet-long-strings

Conversation

@takeokunn

Copy link
Copy Markdown
Collaborator

Independent of the lint-rule PR chain — this touches only packages/core/syntax.

classify_janet had no backtick arm, so a backtick was neither a delimiter nor whitespace and got absorbed into an atom — which meant a long string's contents were parsed as code. Triple-backtick docstrings are the dominant Janet idiom, so this is not an edge case.

Differential over 241 real .janet files

janet, spork, jpm, circlet, janet-sh — all 241 of which Janet's own reader accepts, so every paredit failure was paredit's.

parse failures      9 → 2      (7 fixed, 0 newly broken)
trees changed            42     (all reductions; 9880 phantom atoms removed)
trees identical         190
top-level form counts vs Janet's own reader:  230/232 → 239/239

The two remaining failures are a different, pre-existing defect I deliberately left alone: ' (quote) is missing from classify_janet's prefix table, so it glues onto a following " (spork/cjanet.janet:31, janet/test/suite-peg.janet:405). Identical on the pre-change binary. It's the same class of bug in another reader macro and it silently mis-shapes every 'foo in every Janet file — worth its own PR.

What parse.c specifies, verified against a 1.41.3-dev build

  • A single backtick opens one. root (:643) is unconditional — there is no run-length test.
  • The close is exactly N, not at least N. stringend returns 0, re-dispatching the revealing character, so surplus backticks open the next datum: ```ab x` ```` is two values.
  • The opener is the whole run, so an empty long string is unwritable — Janet errors on ``.
  • Newlines yes, escapes no — the in-string branch is push_buf with no backslash case at all.
  • @ shares the extent, as a buffer literal.
  • Backtick is not a symbol character (symchars[3] = 0x07fffffe leaves bit 0 clear), so it also ends the token before it. Without that arm in is_atom_boundary, (foo`bar`) would glue. This wasn't in the original bug report and turned out load-bearing.

An unterminated long string is refused as UnterminatedString, matching Janet's unexpected end of source. Reading to EOF as one atom is precisely the silent corruption this removes.

Evidence beyond the diff

Differential fuzz, 2500 cases, same seed both binaries: 1385 agree before → 2500 agree after, including all 332 cases Janet rejects. The first generator was thrown away — it mixed in tokens whose paredit/Janet divergence predates this change, so "both reject" was coincidental agreement rather than evidence.

Round-trip idempotence passes 239/239, which proves nothing on its own (a consistently wrong parse is a fixed point), so Janet itself was the oracle for whether the value survives.

Other dialects unaffected

3167 files, 1414 of them containing backticks, zero differences between pre- and post-change binaries:

corpus files with backtick different
nix store .el/.scm/.lisp 2500 1291 0
nix store .clj .asd .rkt .carp .lfe .hy .fnl .ss 627 123 0
repo tests/fixtures 40 0

Plus a unit test pinning backtick as ReaderPrefix::Quasiquote for all ten other dialects, so a future edit to has_long_strings can't quietly widen.

A formatter bug this surfaced, not fixed here

stringend (:355) does indent_col = top.column - 1a long string's value depends on the column of its opening backticks. edit format preserves the literal's bytes exactly and moves that column, so 30 files still change meaning:

(defn f

line one
indented

[x] x)

Janet reads the source string as "line one\n indented"; after edit format it reads " line one\n indented\n ".

Net this is still an improvement — 199 → 209 files round-trip value-identically, and exactly one file (jpm/cc.janet) regressed from value-preserving to value-changing. Three that look new simply didn't parse before. And the pre-change formatter failed worse in kind on the same input, collapsing the docstring's newlines outright. It's a formatter defect with its own blast radius and belongs in its own change.

Cost

The dialect test sits before the byte test in is_atom_boundary deliberately: self.dialect is loop-invariant across the per-byte calls made for every atom in the document, so for the other nine dialects it folds away rather than costing a comparison per byte. Nothing new allocates. This is a code argument, not a measurement — load average was 73 from parallel agents, so no local benchmark would have meant anything.

Verification

cargo build --workspace, cargo test --workspace, cargo test --test cli (3083 passed), cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings — all exit 0.

No pinned count or golden moved, consistent with the fix being confined to a dialect no fixture exercises with backticks. The repo's only .janet fixture contains no backticks and formats byte-identically under both binaries — part of why this went unnoticed.

`classify_janet` had no backtick arm, so a backtick was neither a
delimiter nor whitespace and got absorbed into an atom -- which meant
**a long string's contents were parsed as code**. Triple-backtick
docstrings are the dominant Janet idiom, so this was not an edge case.

Over 241 real .janet files (janet, spork, jpm, circlet, janet-sh), all
241 of which Janet's own reader accepts:

    parse failures  9 -> 2      (7 fixed, 0 newly broken)
    trees changed        42     (all reductions; 9880 phantom atoms gone)
    trees identical     190
    top-level form counts vs Janet's own reader: 230/232 -> 239/239

The two remaining failures are a *different* defect, left alone --
`'` (quote) is missing from `classify_janet`'s prefix table, so it glues
onto a following `"`. Identical on the pre-change binary, so it predates
this and deserves its own review; it silently mis-shapes every `'foo`
in every Janet file.

What parse.c actually specifies, verified against a 1.41.3-dev build
rather than assumed:

- A **single** backtick opens one; `root` (:643) has no run-length test.
- The close is **exactly N**, not at least N. `stringend` returns 0, so
  the revealing character is re-dispatched -- ```` ```ab```` x` ```` is
  two values, not one.
- The opener is the whole run, so an empty long string is unwritable.
- Newlines yes, escapes no -- the in-string branch is `push_buf` with no
  backslash case at all.
- `@` shares the extent, as a buffer literal.
- Backtick is **not** a symbol character (`symchars[3] = 0x07fffffe`
  leaves bit 0 clear), so it also ends the token before it. Without that
  arm in `is_atom_boundary`, `(foo`bar`)` would glue.

An unterminated long string is refused as `UnterminatedString`, matching
Janet's own `unexpected end of source`. Reading to EOF as one atom is
exactly the silent corruption this removes.

Differential fuzz, 2500 cases, same seed both binaries: 1385 agree
before, **2500 agree after**, including all 332 cases Janet rejects.
The first generator was thrown away -- it mixed in tokens whose
divergence predates this change, so "both reject" was coincidental
agreement rather than evidence.

Other dialects: 3167 files (1414 containing backticks) across .el .scm
.lisp .clj .asd .rkt .carp .lfe .hy .fnl .ss, **zero** differences. A
unit test pins backtick as `ReaderPrefix::Quasiquote` for all ten other
dialects so a future edit to `has_long_strings` cannot quietly widen.

One finding not fixed, and it is a formatter bug rather than a parser
one: `stringend` sets `indent_col = top.column - 1`, so a long string's
*value* depends on the column of its opening backticks. `edit format`
preserves the bytes and moves the column, so 30 files still change
meaning. Net this is an improvement -- 199 -> 209 files round-trip
value-identically under Janet as oracle, and exactly one file regressed;
the pre-change formatter failed worse in kind on the same input,
collapsing a docstring's newlines outright.

The repo's only .janet fixture contains no backticks and formats
byte-identically under both binaries, which is part of why this went
unnoticed.
@takeokunn
takeokunn merged commit dcd776a into main Aug 3, 2026
10 checks passed
@takeokunn
takeokunn deleted the fix/janet-long-strings branch August 3, 2026 08:33
takeokunn added a commit that referenced this pull request Aug 3, 2026
RULE_COUNT 316 -> 320, registry-only, no new commands. This completes
dialect coverage: every one of the ten dialects now has at least one
rule written for it specifically.

lint-hy-lfe-idiom: hy-mutable-default-argument (Error),
hy-identity-comparison-with-literal, hy-bare-except, and
lfe-catch-swallows-exit (pedantic).

Both interpreters were available, so every premise was run rather than
argued. Hy 1.3.1 and LFE 2.2.0 / Erlang 27.3.4.15.

Hy's mutable default is the mirror image of a rule this project already
refuted for Common Lisp: CL re-evaluates an `&optional` init form per
call, but Python evaluates a default **once**, at definition time, so
the same shape that is fine in CL is a real bug here. Confirmed: three
calls return [1], [1 1], [1 1 1]; `{}` and `#{}` behave identically and
`None` does not.

Also confirmed: CPython emits `SyntaxWarning: "is" with 'int' literal`
through Hy; `(except [])` catches KeyboardInterrupt and SystemExit while
`(except [e Exception])` catches neither; and `(catch (exit 'boom))`,
`(tuple 'EXIT 'boom)` and `(catch (tuple 'EXIT 'boom))` are the
**identical term**, so an LFE caller cannot tell failure from success.

Refuted and dropped: `(if 'false 'a)` returns false in LFE rather than
raising, so if-without-else is not a defect; `!` to a dead pid returns
the message with no error and is not statically detectable; and
case/receive without a catch-all is "let it crash", which would be mass
false positives.

The corpus audit -- 2825 .hy across 284 repos, 2701 .lfe across 195 --
changed three of the five candidates:

- `hy-mutable-class-attribute` was **killed**: 33 findings, 251
  candidates, 0 genuine defects. The premise is true and measured, but
  in real Hy a mutable class attribute is a *declaration* (`__slots__`,
  Django ModelAdmin fields, Textual BINDINGS), not accidental sharing.
- `hy-mutable-default-argument` was narrowed from "mutable default" to
  "mutable default the body mutates", taking it from 88 findings to 1.
  The survivor is a genuine bug whose own docstring shows the author
  expected a fresh set per call.
- `lfe-catch-swallows-exit` was tagged pedantic rather than killed:
  146 findings but only 9 of ~143 repos, and two of the three heaviest
  are LFE's own implementation.

Both dialect gates were proven on the corpus, not just in unit tests:
38 `catch` candidates in Hy files produced 0 findings, and 5 `is`
candidates in LFE files produced 0.

The wiring exposed a latent bug in feature_dependency_contract, flagged
not fixed: it scans manifests as whole text, so the package's *comment*
explaining a removed dev-dependency read as two declared feature edges.
Rewording it to name the packages by directory fixed the build -- but an
intermediate wording containing the bare marker `paredit-feature-` made
the scanner emit an empty-string dependency name and fail differently.
It cannot tolerate the marker appearing without a name after it.

Reader limitations found while building this are recorded in the
package README rather than worked around silently: Hy's interpolated
f-strings are unimplemented (390 of 2825 files fail to parse), `#!`
shebangs are not stripped (393 files, exit 0 with junk atoms), Hy
bracket strings `#[[...]]` are parsed as *code* (33 files -- the package
defends against this explicitly, since a rule could otherwise fire on
text that is not code), Hy's `~` is not a ReaderPrefix so QuoteState
never counts down, and LFE `#B(...)`/`#M(...)` are orphaned from their
list in 243 files, inflating arity at exit 0. Same class as the Janet
backtick defect fixed in #91, and a repair belongs in core/syntax.
takeokunn added a commit that referenced this pull request Aug 3, 2026
RULE_COUNT 316 -> 320, registry-only, no new commands. This completes
dialect coverage: every one of the ten dialects now has at least one
rule written for it specifically.

lint-hy-lfe-idiom: hy-mutable-default-argument (Error),
hy-identity-comparison-with-literal, hy-bare-except, and
lfe-catch-swallows-exit (pedantic).

Both interpreters were available, so every premise was run rather than
argued. Hy 1.3.1 and LFE 2.2.0 / Erlang 27.3.4.15.

Hy's mutable default is the mirror image of a rule this project already
refuted for Common Lisp: CL re-evaluates an `&optional` init form per
call, but Python evaluates a default **once**, at definition time, so
the same shape that is fine in CL is a real bug here. Confirmed: three
calls return [1], [1 1], [1 1 1]; `{}` and `#{}` behave identically and
`None` does not.

Also confirmed: CPython emits `SyntaxWarning: "is" with 'int' literal`
through Hy; `(except [])` catches KeyboardInterrupt and SystemExit while
`(except [e Exception])` catches neither; and `(catch (exit 'boom))`,
`(tuple 'EXIT 'boom)` and `(catch (tuple 'EXIT 'boom))` are the
**identical term**, so an LFE caller cannot tell failure from success.

Refuted and dropped: `(if 'false 'a)` returns false in LFE rather than
raising, so if-without-else is not a defect; `!` to a dead pid returns
the message with no error and is not statically detectable; and
case/receive without a catch-all is "let it crash", which would be mass
false positives.

The corpus audit -- 2825 .hy across 284 repos, 2701 .lfe across 195 --
changed three of the five candidates:

- `hy-mutable-class-attribute` was **killed**: 33 findings, 251
  candidates, 0 genuine defects. The premise is true and measured, but
  in real Hy a mutable class attribute is a *declaration* (`__slots__`,
  Django ModelAdmin fields, Textual BINDINGS), not accidental sharing.
- `hy-mutable-default-argument` was narrowed from "mutable default" to
  "mutable default the body mutates", taking it from 88 findings to 1.
  The survivor is a genuine bug whose own docstring shows the author
  expected a fresh set per call.
- `lfe-catch-swallows-exit` was tagged pedantic rather than killed:
  146 findings but only 9 of ~143 repos, and two of the three heaviest
  are LFE's own implementation.

Both dialect gates were proven on the corpus, not just in unit tests:
38 `catch` candidates in Hy files produced 0 findings, and 5 `is`
candidates in LFE files produced 0.

The wiring exposed a latent bug in feature_dependency_contract, flagged
not fixed: it scans manifests as whole text, so the package's *comment*
explaining a removed dev-dependency read as two declared feature edges.
Rewording it to name the packages by directory fixed the build -- but an
intermediate wording containing the bare marker `paredit-feature-` made
the scanner emit an empty-string dependency name and fail differently.
It cannot tolerate the marker appearing without a name after it.

Reader limitations found while building this are recorded in the
package README rather than worked around silently: Hy's interpolated
f-strings are unimplemented (390 of 2825 files fail to parse), `#!`
shebangs are not stripped (393 files, exit 0 with junk atoms), Hy
bracket strings `#[[...]]` are parsed as *code* (33 files -- the package
defends against this explicitly, since a rule could otherwise fire on
text that is not code), Hy's `~` is not a ReaderPrefix so QuoteState
never counts down, and LFE `#B(...)`/`#M(...)` are orphaned from their
list in 243 files, inflating arity at exit 0. Same class as the Janet
backtick defect fixed in #91, and a repair belongs in core/syntax.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant