Fix: allow a parenthesized sub-expression as a chain head or tail term - #79
Merged
vantreeseba merged 7 commits intoJul 8, 2026
Merged
Conversation
…mprehensions Fixes github.com/vantreeseba/issues/60's underlying gap (a WIP PR attempting the same thing) with a from-scratch implementation against the Haxe language reference (haxe.org/manual/expression-*.html, block through inline), not a port of that PR's approach. `for`, `while`, `do`, `try`, and `catch` were bare placeholder keyword tokens (`keyword: choice('catch', 'do', 'for', 'try', 'while')`) with no real grammar rule at all. A statement like `for (i in 0...10) trace(i);` parsed with NO error node -- but only because `for`, `(i in 0...10)`, and `trace(i);` happened to each independently satisfy some other, unrelated statement/expression rule as three disconnected siblings, not because `for` was actually understood as a loop. Same story for `while`, `do ... while`, and `try ... catch`. This is a "wrong tree, not a broken one" bug -- has_error-based checks are blind to it, which is exactly why it went unnoticed this long. Implemented for real, following the manual directly: - `for_statement`: `for (binding in iterable) body`, where binding is either a plain identifier or a key => value pair (Haxe 4.0+, `for (k => v in map)`). - `while_statement` / `do_while_statement`. - `try_statement` with repeatable `catch_clause`s, each with an optional type annotation (Haxe 4.1+ wildcard catch: `catch (e) { ... }`). - `conditional_statement` (if/else if/else) rewritten so bodies are $.statement instead of $.block -- Haxe's `if (cond) expr;` (no braces) is standard, idiomatic syntax that was completely broken here, forcing braces on every branch. - Array comprehensions (`[for (i in 0...10) i * 2]`, `[while (...) ...]`, manual: lf-array-comprehension.html), replacing a placeholder rule that didn't match real comprehension syntax at all. Supports nesting (`for`/`while`/an `if`-filter inside a comprehension body), matching real code like `[for (a in 1...11) for (b in 2...4) if (a % b == 0) a + "/" + b]`. All four statement bodies reuse $.statement rather than a separate braces-only rule, which is what makes braceless bodies work "for free" without duplicating the block/single-statement logic per construct. **Found and fixed along the way, once real testing against a large depot surfaced them:** - The float literal regex allowed repeated dots (`[\.]+` instead of a single `\.`), so `0...10` (an integer immediately followed by the range operator -- the single most common shape for a for-loop's iterable) was being lexed as ONE malformed float token, "0...10", silently swallowing the range operator. Tree-sitter's regex engine has no lookahead, so there's no way to keep a bare trailing-dot float (`3.`) valid while still telling `0.` apart from `0...`; fixed by requiring at least one digit after the dot, which is a real but comparatively rare trade-off (a genuine standalone trailing-dot float in real, non-comment/string code) against a very common one (every `for (i in 0...N)` loop). - `range_expression` previously baked `identifier 'in' ...` directly into itself -- a workaround from when `for` was unimplemented, using it to soak up a for-loop's `(i in 0...10)` content via the same kind of accidental-sibling trick. Investigated replacing it with a clean, standalone `min...max` rule now that `for_statement` provides its own real binding structure, but found (via direct testing, not assumption) that a dedicated rule is actually unreachable dead code here: `...` is already one of the generic `$.operator` choices, so the existing plain operator-chain alternative in `expression` already parses `0...10`, `arr.length - 1 ... arr.length + 5`, etc. correctly on its own with no dedicated rule at all. Removed rather than left in as dead code. - `subscript_expression` (`arr[i]`) could appear as a chain's head (`arr[i] = x`, fixed in an earlier commit) but not its tail -- `while (null != m_cache[id]) { ... }` still failed. Fixed by including it in `_chain_term`. - A pre-existing, narrower ambiguity surfaced once control-flow bodies became $.statement instead of $.block: a genuinely EMPTY `{}` body resolves to an empty $.object (an object-literal expression statement) rather than $.block. Investigated at length -- declaring `[$.block, $.object]` in `conflicts` is flagged unnecessary by tree-sitter's own analysis (it doesn't see this as a real, GLR-forkable ambiguity), and neither `prec`/`prec.dynamic` on either rule (tried up to prec(1000)) nor an explicit `choice($.block, $.statement)` at the body field change the outcome, which suggests table construction is merging the two empty-content states before any declared precedence would be consulted -- not something fixable from grammar.js alone. Left as a documented, narrow limitation (42 files in this depot use a genuinely empty control-flow body) rather than blocking the other ~99% of this fix on it; non-empty bodies are entirely unaffected. Verified: full corpus suite, 190/190 (added dedicated corpus files for conditional_statement braceless bodies, loop statements, try/catch, and array comprehensions; updated two pre-existing tests to match corrected behavior -- the old array-comprehension placeholder test's expected output, and removed a test for the now-unsupported bare trailing-dot float). Depot-wide sweep of 5,332 real .hx files: 0 regressions, 251 files newly error-free (has_error-based, which undercounts real impact since many files have multiple independent issues) -- a much clearer picture from counting the actual new node types recognized: 8,601 for-loops (1,751 files), 1,206 while-loops (502 files), 182 do-while loops (106 files), 400 try-statements / 403 catch-clauses (222-223 files), 871 array comprehensions (353 files), and 53,288 braceless if-bodies across 3,241 files -- all previously either disconnected-sibling nonsense or a hard parse error.
…ted node The while-comprehension test's expected tree still described upstream's generic call_expression-shaped fallback (upstream has no dedicated comprehension_while rule). This fork adds one as part of this PR, so the expected tree should reflect it instead.
A parenthesized expression could previously only ever be an entire standalone `expression` (the top-level $._parenthesized_expression choice) -- never one term of a longer operator chain. So `0...(10)`, `a + (b)`, `(a + b) * c`, `total = (a + b) / 2`, etc. all hard-errored, even though grouping parentheses in ordinary arithmetic are about as common as it gets. Found via a depot-wide Haxe parse-error sweep: the minimal repro was `0...(width*height)` in haxe/src/com/masque/mah/common/ZMap.hx, but testing plain `a + (b)` confirmed the gap was general, not range-operator-specific. Two additions, mirroring the existing $.subscript_expression-as-head / $.subscript_expression-as-tail precedent already in this grammar: - $._chain_term (a chain's TAIL term) now includes $._parenthesized_expression alongside $._rhs_expression and $.subscript_expression. - `expression` gets a new repeat1-gated alternative, seq($._parenthesized_expression, repeat1(seq($.operator, $._chain_term))), for a parenthesized sub-expression as a chain's HEAD. repeat1-gated so a solo `(a + b)` alone still resolves via the existing standalone $._parenthesized_expression choice, not this one. Verified: 225/225 corpus tests pass (3 new, covering tail position, head position, and the original range-operator repro). Depot-wide sweep against the Masque depot, combined with the prior switch-case-without-block fix in this same session: of 548 files with a fully-destroyed class node, 342 recovered a proper class_declaration node (548 -> 206 remaining severe); 2,840 files still show has_error overall (down from 3,275), the remainder being further, separate issues -- confirmed one more while verifying this fix: `return <ternary>;` fails to parse regardless of parens, and a ternary used as a term inside a larger chain (e.g. string concatenation) has the same "not includable as a chain term" gap this fix addresses for parenthesized expressions specifically. Not fixed here -- flagged as a separate follow-up.
mallyskies
changed the title
vantreeseba
added a commit
that referenced
this pull request
Jul 8, 2026
Contributor
Author
|
Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #78 (
fix/for-while-trycatch-comprehensions) — based on its tip.A parenthesized expression could previously only ever be an entire standalone
expression(the top-level$._parenthesized_expressionchoice) -- never one term of a longer operator chain. So0...(10),a + (b),(a + b) * c,total = (a + b) / 2, etc. all hard-errored, even though grouping parentheses in ordinary arithmetic are about as common as it gets.Found via a codebase-wide Haxe parse-error sweep: the minimal repro was
0...(width*height), but testing plaina + (b)confirmed the gap was general, not range-operator-specific.Two additions, mirroring the existing
$.subscript_expression-as-head /$.subscript_expression-as-tail precedent already in this grammar:$._chain_term(a chain's TAIL term) now includes$._parenthesized_expressionalongside$._rhs_expressionand$.subscript_expression.expressiongets a new repeat1-gated alternative,seq($._parenthesized_expression, repeat1(seq($.operator, $._chain_term))), for a parenthesized sub-expression as a chain's HEAD. repeat1-gated so a solo(a + b)alone still resolves via the existing standalone$._parenthesized_expressionchoice, not this one.Verified: 225/225 corpus tests pass (3 new, covering tail position, head position, and the original range-operator repro).