refactor: split large grammar.js into modules

[vergenzt]

I tried to make no semantic changes to anything, but that could use some verifying. I did make substantial editorial decisions about what "belongs" where -- totally open to feedback on any of it.

I may try to do some sort of automated analysis to confirm that all of the rules expressions from before and after are the same. We'll see.

Would you be open to merging something like this?

[vergenzt]

Okay I was able to figure out a good way to do that validation. I used an ast-grep rule to fetch all relevant grammar rule JS key/value pairs, normalized whitespace for each (since I dedented when extracting to the grammar/ folder), then sorted and wrote to a file and was able to diff the files.

On both main and this branch, I ran the following:

ast-grep scan --json --inline-rules='id: _
language: js
rule:
  kind: pair
  inside:
    kind: object
    inside:
      any:
        - pattern: module.exports = { $$$ }
        - kind: pair
          has:
            field: key
            regex: rules
' grammar* | jq '.[].text | gsub("\\\\s+"; " ")' | sort > rules-$(git rev-parse HEAD)

(edit 2025-12-29 to remove indent which arose from copy/paste)

This then produced the following files:

• rules-224f3a49757b4997fc89b89fe31b84c635002534 (all rules detected at 224f3a4, tip of this branch)
• rules-fe77f6868d6cdea593052a6af390116495093dc1 (all rules detected at fe77f68, current tip of main)

Then, git diff --no-index rules-fe77f6868d6cdea593052a6af390116495093dc1 rules-224f3a49757b4997fc89b89fe31b84c635002534 for me yields:

diff --git a/rules-fe77f6868d6cdea593052a6af390116495093dc1 b/rules-224f3a49757b4997fc89b89fe31b84c635002534
index c025468..320e991 100644
--- a/rules-fe77f6868d6cdea593052a6af390116495093dc1
+++ b/rules-224f3a49757b4997fc89b89fe31b84c635002534
@@ -148,7 +148,7 @@
 "direction: $ => choice($.keyword_desc, $.keyword_asc)"
 "distinct_from: $ => seq($.keyword_is, $.keyword_distinct, $.keyword_from)"
 "dollar_quote: () => /\\$[^\\$]*\\$/"
-"double: $ => choice( make_keyword(\"float8\"), unsigned_type($, parametric_type($, $.keyword_double, ['precision', 'scale'])), unsigned_type($, parametric_type($, seq($.keyword_double, $.keyword_precision), ['precision', 'scale'])), unsigned_type($, parametric_type($, $.keyword_real, ['precision', 'scale'])), )"
+"double: $ => choice( $.keyword_float8, unsigned_type($, parametric_type($, $.keyword_double, ['precision', 'scale'])), unsigned_type($, parametric_type($, seq($.keyword_double, $.keyword_precision), ['precision', 'scale'])), unsigned_type($, parametric_type($, $.keyword_real, ['precision', 'scale'])), )"
 "drop_column: $ => seq( $.keyword_drop, optional( $.keyword_column, ), optional($._if_exists), field('name', $.identifier), )"
 "drop_constraint: $ => seq( $.keyword_drop, $.keyword_constraint, optional($._if_exists), $.identifier, optional($._drop_behavior), )"
 "drop_database: $ => prec.left(seq( $.keyword_drop, $.keyword_database, optional($._if_exists), $.identifier, optional($.keyword_with), optional($.keyword_force), ))"
@@ -306,6 +306,7 @@
 "keyword_filter: _ => make_keyword(\"filter\")"
 "keyword_first: _ => make_keyword(\"first\")"
 "keyword_float: _ => make_keyword(\"float\")"
+"keyword_float8: _ => make_keyword(\"float8\")"
 "keyword_following: _ => make_keyword(\"following\")"
 "keyword_follows: _ => make_keyword(\"follows\")"
 "keyword_for: _ => make_keyword(\"for\")"

... which of course now reminds me that I did change one thing, which was to extract a separate keyword_float8 rule instead of the previous inline call to make_keyword. 😄

Looks like that's confirmed to be the only change though!!

[DerekStride]

I like the idea here, splitting everything out into separate modules seems nice, are you aware of other projects that make use of this pattern? We're already quite different in that we don't commit the generated code so I worry about adding more friction.

[vergenzt]

I like the idea here, splitting everything out into separate modules seems nice, are you aware of other projects that make use of this pattern? We're already quite different in that we don't commit the generated code so I worry about adding more friction.

From some searching, the following appear to be tree-sitter grammar definitions which require pieces of their definitions from other files:

• https://github.com/DeNA/DeClang/blob/419618d25f9c209dc3ca3c49f72947b06f3d5fa1/mlir/utils/tree-sitter-mlir/grammar.js#L4
• https://github.com/tweakoz/ork.build/blob/71f174cdd73708268dc499c49ada4559c01dcc0b/docs/tree-sitter-cpp-grammar.js#L13
• https://github.com/quarto-dev/quarto-markdown/blob/c925e444df03c1f7b7b4cccb5f0a2e72fc130885/crates/tree-sitter-qmd/tree-sitter-markdown/old-grammar.js#L7
• https://github.com/nurulhudaapon/zx/blob/a80f92e20098a4778db839f063707d8487eb6c0d/packages/tree-sitter-zx/grammar.js#L1
• https://github.com/shackle-rs/shackle/blob/a1090067ad19e24fabc0afb751a057a1926a4b85/parsers/tree-sitter-datazinc/grammar.js#L1
• https://github.com/tree-sitter/tree-sitter-typescript/blob/75b3874edb2dc714fb1fd77a32013d0f8699989f/common/define-grammar.js#L1

To answer your question more directly though, no I'm not aware of specific other projects which split things out. I believe this is also the first major tree-sitter grammar I've attempted to make a contribution to -- but I'm pretty sure it's also the largest / most complex grammar I've encountered.

IMO this will substantially reduce friction for new contributors because potential contributions will have a clearer place to go.

[vergenzt]

FYI I pushed 8f8e5fd to revert the (admittedly minor) keyword change, and reran the diff script. Diff output from ebf08ba (now-current tip of main) to 8f8e5fd (new tip of this PR) is now empty.

(You can see full output here: https://gist.github.com/vergenzt/1614ef579a3ba452050b2338cfd7adcf)

[vergenzt]

If you're not interested in this would you at least be open to some sort of well-defined order for rules in grammar.js? Maybe a topological sort or something?

[DerekStride]

@matthias-Q Do you have any objections? If not, I'm good to merge these changes.

[matthias-Q]

No, I am totally fine with it.

[vergenzt]

Awesome! I'll try to get conflicts resolved asap and get this back to being ready to merge.

[clason]

PSA: upstream tree-sitter is moving away from CJS towards ESM (modules), mostly for improved compatibility with the new native JS runtime. You might wish to future-proof this while you're at it.

(I know you're pinning to tree-sitter 0.24 here (current is 0.26.3), but this will not be tenable forever -- you're missing out on useful features and bugfixes, and support for ABI 14 will eventually be removed.)

[vergenzt]

Hi all! Instead of rebasing this time I "merged" each of the PRs which landed in main since I branched off, and I ensured all changes to grammar.js in the original PR got re-applied into the split versions. I also then switched everything over to ES6 module syntax per #351.

It should be ready to go! Tests passed locally, so would love if someone could approve the CI jobs to run and confirm everything is still green.

[DerekStride]

This was a big refactor, thanks for sticking with it