Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[STAL-2005] Update the javascript grammar #343

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

[STAL-2005] Update the javascript grammar #343

wants to merge 2 commits into from

Conversation

amaanq
Copy link
Member

@amaanq amaanq commented May 2, 2024

What problem are you trying to solve?

Being able to query for supertypes in the JS grammar, namely the statement node, so that we don't have to write an alternation of several possible child nodes of the statement node. This will help with the ESLint rule implementations

What is your solution?

In tree-sitter-javascript, the statement node was inlined as well as being a supertype, which doesn't make a lot of sense since inlining it will make it unqueryable. Supertyping a node has a similar effect that inlining does where a node is not present in the syntax tree, but it can still be queried for. This is especially useful in cases where the supertype contains a choice of children, such as the statement node, expression node, and others. Removing statement from the inline list in the grammar allows us to query for it again.

Alternatives considered

Hacky workaround with alternations (not fun)

What the reviewer should know

This just updates the JS grammar, however, I will update the TS/TSX grammar later today for this change as well. I also removed the cpp field since upstream has deprecated c++ scanners and no maintained grammar uses them anymore

amaanq added 2 commits May 2, 2024 15:17
It's also deprecated upstream and there are no longer any *maintained*
grammars that have a C++ scanner.
@amaanq amaanq requested review from jasonforal and juli1 May 2, 2024 19:38
@jasonforal
Copy link
Collaborator

Moving this to a draft for the time-being, as this change has unintended behavior regressions that we should review first.

@jasonforal jasonforal marked this pull request as draft May 3, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants