Implement for/while/do-while/try-catch, braceless if-bodies, array comprehensions - #78
Merged
vantreeseba merged 5 commits intoJul 8, 2026
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.
mallyskies
changed the title
Owner
|
While I appreciate all the PRs, since these are all "stacked" (i.e. 80 contains 79 contains 78). I would appreciate if you do a chunk of work locally, even if on seperate branches, and create a single branch with all the PRs merge into it, and make a PR from that one to here. so workflow would look something like: local repo "dev" branch gets PR from each, you merge those into it. single PR from dev/whatever/some "merged branch that is a set of features together" to here. I'm going through these today and will get them all in, so this is just for the future. Lastly, I really prefer having semantic commit markers on each commit, I will update a contributing doc or something in the near future as well. i.e. commit looks like feat(grammar): added array parsing |
Contributor
Author
|
No problem at all. Thanks for the feedback and guidance! |
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.
Fixes the underlying gap #60 (closed, unmerged WIP) attempted to address, 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. Also fixes #62 (nested for loops sometimes fail to highlight) as a side effect of the underlying grammar fix -- the highlighter relies on real for_statement/while_statement/etc. nodes, which this PR adds for the first time.
for,while,do,try, andcatchwere bare placeholder keyword tokens (keyword: choice('catch', 'do', 'for', 'try', 'while')) with no real grammar rule at all. A statement likefor (i in 0...10) trace(i);parsed with no ERROR node — but only becausefor,(i in 0...10), andtrace(i);happened to each independently satisfy some other, unrelated statement/expression rule as three disconnected siblings, not becauseforwas actually understood as a loop. Same story forwhile,do ... while, andtry ... catch. This is a "wrong tree, not a broken one" bug —has_error-based checks are blind to it.This PR adds:
for_statement,while_statement,do_while_statement,try_statement(with typed/untyped/multiplecatchclauses) rules.[for (...) ...],[while (...) ...]), including a dedicatedcomprehension_whilenode (upstream's own later while-comprehension test — merged via Include the tests from pr_60 to make sure we can close that. #77 — expected the old disconnected-siblings fallback shape; this PR's comprehension_while test corrects that expectation to match the real node).if (cond) expr;, no{}) —if/else/else ifbodies now use$.statement(which already covers both the{ ... }and bare-expr;shapes) instead of forcing$.blockeverywhere. This also folds in the already-merged multi-branchelse iffix (Fix bare 'this', prefix nullable params, and multi-branch else-if chains #71) as a strict subset — same per-branchrepeat(...)structure, just with wider body types._chain_term(chain tail positions only) to also accept$.subscript_expression, so comparisons likewhile (null != m_timer[id]) { ... }parse as a real chain instead of erroring — found via a real codebase-wide sweep, rather than synthesized.Verified against current
main:for/while/do-while/try-catch/both comprehension forms/bracelessif-else) against unmodified currentmainstill produces anERRORnode; the same snippet parses cleanly with this branch.