Repository navigation
Change tree-sitter for Luau with updates - #28
karl-police wants to merge 35 commits into
Conversation
appellation
left a comment
There was a problem hiding this comment.
Note: this review was written by Claude (an AI assistant) at the maintainer's request, not by the maintainer personally. The findings below were checked by building this branch with the repo's own
./scripts/arborium-rt build wasm grammar group-hazel luaupipeline, parsing ~80 Luau snippets with both this branch andmain, and running a sample file through the wasm highlighter.
Thanks for working on this. const declarations and a basic class statement now parse and highlight, which is a real improvement. But as it stands the PR breaks several common Luau constructs that work on main, and it drops required license notices, so it needs changes before it can merge.
Attribution / licensing (blocking)
About 54% of the grammar's lines come from tree-sitter-lua (Munif Tanjim, MIT), about 19% from tree-sitter-luau (Amaan Qureshi, MIT), and about 90% of scanner.c is tree-sitter-lua's scanner. The rest looks like your own work, and the PR description already says this is where the code came from. All of that is fine under MIT, but MIT requires keeping the original copyright notices.
arborium.yaml now points at karl-police/tree-sitter-luau-new, and our THIRD_PARTY_NOTICES step clones that repo at the pinned commit and ships its LICENSE. That LICENSE says only Copyright (c) 2026 karl-police, so the Munif Tanjim and Amaan Qureshi notices would be dropped from what we ship (they're listed as authors in that repo's tree-sitter.json, but that isn't a license notice). Please add both original copyright lines to the upstream repo's LICENSE, for example:
Copyright (c) 2021 Munif Tanjim
Copyright (c) 2023 Amaan Qureshi
Copyright (c) 2026 karl-police
and bump commit: here once that's done.
Regressions compared with main
| Problem | Example | Cause |
|---|---|---|
| Interpolated strings starting with a letter misparse | `Hello {name}` is parsed as a call to an identifier named `Hello with a table argument |
The identifier regex allows ` |
| Optional types are lexed as one identifier | local x: string? = nil, type B = Foo? |
The identifier regex allows ? |
Typed generic for loops error |
for k: string, v in pairs(t) do end |
for_generic_clause is defined twice and the untyped one wins |
| Ordinary identifiers break at the start of a statement | open("f"), class.isinstance(p, Point), class = 1, const = 1 |
open / extends / class / const became hard keywords |
| Parser is about 5x larger | 585 → 2852 parse states; luau wasm 100 KB → 424 KB raw, 16.7 KB → 48.8 KB after brotli |
Details are in the inline comments.
Luau correctness
- The
constRFC saysconstis a contextual keyword and "fully backwards compatible with existing code", solocal const = 5; const = 1must keep working. - The merged classes RFC uses
public x: numberfields in class bodies and allowsexport class X. The RFC's ownPointexample doesn't parse with this grammar, and neither doesexport class X end.classis also contextual there (it's a library name too:class.isinstance).openandextendsaren't in any accepted RFC, so I'd suggest leaving them out rather than reserving them. - The grammar still carries Lua 5.4/5.5-only syntax that isn't Luau:
globaldeclarations,<attrib>(local x <const>), and the bitwise operators| & ~ << >>. These grow the parse table for no benefit. - Still unsupported (same as
main, so not a regression, just noting):@native/@[...]function attributes,type function, generic defaults (<T = string>), leading|/&in unions,read/writeproperties,;separators in table types,typeof(...)inside annotations, typed numericfor, and...: T....
Suggested changes
- Restore the previous Luau identifier regex (excludes
`and?, uses the/uflag). - Delete the duplicate
for_generic_clauseand the unused rules (binding_list,_vararg_parameter,special_self). - Make
constandclasscontextual, and dropopen/extends. - Support
publicfields (andexport class) in class bodies. - Remove the Lua 5.4/5.5-only syntax.
- Fix the upstream LICENSE as described above.
🤖 Generated with Claude Code
|
I think I should have covered most things now except New output: |
|
I have to remove the "global" stuff another time. I actually added that in my comments somewhere. |
well yup that prob Claude funny comment, it is not wrong though simce this grammar.js does not require lua anymore it is larger but I want to remove all of the non-Luau things today I replaced a lot of functions, I think it even got rid of most of the previous luau tree-sitter... I primarly used Luau grammar for guidance, so the structure is similar to how they define it. Fun fact is that they haven't updated it in a while and are missing a few things 🙃 |
|
ugh I put scanner back to something and it may be missing |
|
epic it is green https://github.com/karl-police/arborium-rt/actions/runs/43370335596 that's good it means this time the files are in a good state |
What is it trying to say? I don't use git CLI often. |
|
Okay yeah gotta add public function and export to class but that also future update theory 🤔 before there was no class at all, so I was actually (for some reason) worrying about the names of the nodes. Not only, but they're important for the as_exp being my biggest enemy, until I solved it... well extra node for So The unused functions and comments... yes... they came from my repository so. I gotta remove them manually, unless commenting them out is fine. I keep them there for reference until they can go. The name list function from Lua is interesting. I had to create a function by myself, because when I added "binding" rule to classes. Everything exploded, but not if I added the function, which returns a cloned copy of the rule, I think. So probably Lua had the same issue. Solving grammar.js errors are simple. Solving the For instance, grammar.js alone can make the most random errors appear in If I replace binding_func with binding Ah yes, now it also supports Their grammar defines structure and where to set the "loops" at. A few features they have are missing in their grammar, so they're not included here. Except classes though. It may had the highlights, but that's due to the bitwise operators. Which is funny to Luau because when it was being developed, they talked about how it would kill Also Similar to Point is, it has its highlight. I don't know if |
## Problem `build wasm grammars` builds grammars in parallel, so their streamed output interleaves. When one fails, the final error only names it: ``` Error: 1 of 12 grammar(s) failed: luau ``` The reason is buried mid-log. In the #28 CI run it was an `emcc: error: undefined exported symbol: "_tree_sitter_luau_external_scanner_create"` line surrounded by other grammars' output. ## Change - **`cli/src/lib/util.ts`:** `run`/`runCapture` now reject with a `CommandError` carrying the child's full stderr. The error message includes the last 20 lines, so the per-step `✖ … [FAILED: …]` lines show the reason without getting long. - **`cli/src/commands/build/wasm/grammars.ts`:** the end-of-run summary lists each failed grammar's failing step, command, and **complete** stderr: ``` 1 of 12 grammar(s) failed: luau --- luau --- linking grammars into wasm: emcc -O2 … exited with code 1 emcc: error: undefined exported symbol: "_tree_sitter_luau_external_scanner_create" [-Wundefined] [-Werror] ``` - `collectErrors` moves to the root list. Nested listr2 lists share the root's `errors` array, which is `null` unless the root collects, so the per-grammar list's own `collectErrors: true` never did anything. Adding the options argument made Biome re-indent `buildAll`, so the diff looks bigger than it is. ## Testing - `tsc --noEmit` and Biome pass for the CLI. - I couldn't run a real grammar build locally (no submodules in this checkout). Instead a small script reproduced the same four levels of nested lists, used the real `run()` and the `failureReason` source, and had one grammar's step write 30 lines of stderr and exit 1. Its step lines showed lines 11–30, and the final summary showed all 30. - This PR's own CI only exercises the success path, since every grammar builds on main. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
appellation
left a comment
There was a problem hiding this comment.
Note: this follow-up review was written by Claude (an AI assistant) at the maintainer's request, not by the maintainer personally. I generated the parser from
2c72a68and parsed the same snippets with this branch and withmain.
Thanks for the updates. const, contextual class/public, export class, attributes, type function, typeof in types and read/write all parse now, and the Lua 5.4 syntax is gone.
One blocker remains: a < b comparisons no longer parse (see the inline comment on function_call). There are also a few smaller regressions compared with main, and the pinned commit: in arborium.yaml is still out of date (see my reply on that thread). Note that CI only builds the grammar; it doesn't parse any Luau, so it can't catch these.
🤖 Generated with Claude Code
|
May I note that it allows function f(): (number, string, ...string) return; return; endhmmm, if I fix it the highlighting doesn't break, so lemme apply the fix |
now is fix |
I made a tree-sitter https://github.com/karl-police/tree-sitter-luau-new/blob/main/out.html
Well, actually I took the lua ones and the luau ones, read it a little bit to learn about tree-sitter. Then made some fixes until the highlights looked okay.
I even added comments in some places :)
I also added
constand yup.Just the highlight support, not the internal node naming. 🤔Since the first thing someone tried was to see if
constwas added too, and then they realized it wasn't. :(And now this thing is not lua dependent anymore, its in its own file, which makes editing it less complicated and it's easier to install and run and test.
What is missing in both still are Function Attributes, I think.Also the?for optional types, maybe.This also adds the Experimental
classsyntax, which is just a copy paste of thestat_dorule, thedo ... endsyntax. It can easily be disabled and removed. Given that it's also removed from thequeriesfolder.Their RFC for classes is merged anyways, so.
Except the inheritance one. Syntax wom't be visible unless used. And if someone that doesn't know about the class syntax yet, it might invite them to the Luau RFC discussion, so why not.
Now, there can't be complains about
constmissing noropen class. 😁constGitHub has it already.
For instance:
class A endRFC approvedopen class A extends Base endnot RFC approved yet but the syntax highlighting supports it anywaysWhat might also not work yet could be
type functionNow to support the highlight for it, change would be really really easy, but the node name would interally be treated as a regular function so... would need extra lines here.
It might also not support something even more new which is "if local".
Which is