tree-sitter-typescript

tree-sitter-typescript copied to clipboard

Add stack graph rules and corresponding tests. The tests are executed against the rules using the tree-sitter-stack-graphs CLI. Stack graph tests are added to CI runs as well, so that the grammar changes do not accidentally break the stack graph rules.

Repository Layout

The tree-sitter-stack-graphs CLI finds stack graph rules automatically in queries/stack-graphs.tsg, similar to tags, locals, etc. Optionally a file with predefined symbols, such as builtins or standard library definitions are automatically loaded from queries/builtins.ts.

Tests are currently located in typescript/test/stack-graphs. This is not prescribed by tree-sitter-stack-graphs so it is easy to move them if another locations is better.

CI

The stack graph tests are executed during CI. For this, the tree-sitter-stack-graphs CLI is installed using cargo, which means that is a dependency to be able to execute npm run test.

To manage the tension between having stack graph rules included and tested in CI, and grammar contributions from contributors that may not have the knowledge to update stack graph rules, I propose that the Semantic Code team gets involved for PRs where this is a problem. They can either provide direct help, or decide the PR may be merged with stack graph tests temporarily disabled, so that they can update them at a more convenient time, without blocking the grammar contribution.

Checklist

• [ ] All tests pass in CI.
• [ ] There are sufficient tests for the new fix/feature.
• [ ] Grammar rules have not been renamed unless absolutely necessary.
• [ ] The conflicts section hasn't grown too much.
• [ ] The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).

This is amazing; it's very cool to see all of these passing tests showing TypeScript/JavaScript's complexities being successfully modeled with stack graphs! I think it's a great thing to make these stack graph rules open source and invite collaboration from other teams who may be interested in this functionality.

My only concerns are about how the maintenance of this repo is going to work. I saw the section that you added to the README about the contributing guidelines, about potentially disabling the stack graphs CI on an as-needed basis. I'm a little bit worried about the friction that this may add to the contribution procedure, given that it's already a bit challenging to be responsive to PRs from the community.

Would it be worth scheduling a zoom call sometime soon, so that we could check in about this stuff?

@maxbrunsfeld I think it's an excellent idea to schedule a zoom call to discuss the maintenance challenges.

It seems likely that adding stack graph definitions and their tests to the grammar repos would increase friction to grammar contributions. We decided it would be better to decouple the stack graph definitions, and have them depend on specific grammar versions. The TypeScript rules are now part of github/stack-graphs#142. A versioned release will hopefully follow soon as well.

Yeah, I like the idea of decoupling them and having them depend on specific versions, as opposed to keeping them in-sync in the repo. Thanks for the update @hendrikvanantwerpen.

I restored the branch for now because we're still pinning to one of its commit SHAs. Once we update our production job to point at something on master we'll delete the branch again.

🤦 I think I cleaned this up. So sorry about that! I didn't think about that dependency.

Another reason why it is good to depend on versioned releases, they are much less likely to suddenly disappear.