-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Refactor queries for ecma based languages #7207
Refactor queries for ecma based languages #7207
Conversation
ca1634f
to
c381649
Compare
@the-mikedavis Sorry for tagging you directly. You were the author of #3301, so I think it'll be good to know your opinion on this proposal. I know it touches quite many files, but it's mostly moving existing queries to new files. Let me know your thoughts. |
bdc639a
to
62020e3
Compare
This proposal is mostly finished. @kirawi I was wondering if you can add the "S-waiting-on-review" label to this PR so that anyone from the core team can take a look when they have time available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense. It fragments the queries a little but I don't see another way around it without undoing all of the inherits
Ecma based languages share many traits. Because of this we want to share as many queries as possible while avoiding nested inheritance that can make query behavior unpredictable due to unexpected precedence. To achieve that, this PR implements "public" and "private" versions for javascript, jsx, and typescript, that share the same name, but the "private" version name starts with an underscore (ecma already exists as sort of the base "private" language). This allows the "private" versions to host the specific queries of the language excluding any "inherits" statement, in order to make them safe to be inherited by the "public" version of the same language and other languages as well. The tsx language does not have a "private" version given it doesn't need to be inherited by other languages. In addition to this, I've adjusted some highlights and locals queries to take advantage of this inheritance and fix existing problems. I've put special attention to queries related to parameters, given it is a place where javascript and typescript treesitter grammars have diverged.
62020e3
to
be1aba8
Compare
I've rebased this branch with master so we have the latest version. If there are no additional comments, it is safe to merge this with master. |
Intro
This PR builds on top of #3301, which created the "fake" ecma language to store queries that are common to both javascript and typescript.
The problem
Ecma-based languages share many traits, but it's not a linear and simple inheritance. Sometimes we want to inherit queries from one of those languages without inheriting the queries that that language inherits. The current state of inheritance is the following:
There are three main examples why this current model generates limitations and bugs:
<
and>
symbols are being highlighted as operators in the context of generic types likeMyGenericType<GenericArgument>
. I wanted to fix this (change it to punctuation), but I realized that tsx was inheriting highlight queries from both jsx and typescript (check here), and both of them were inheriting ecma. The problem is that the queries in ecma for<
and>
(that assigned them to operator highlight, here) inherited by jsx where overwriting the queries for generic types in typescript (here). If I then changed the order and added typescript first, then those same operator queries in ecma inherited by typescript overwrite the<
and>
symbols for jsx elements (here), highlighting them as operators.formal_parameters
and typescript addsrequired_parameter
andoptional_parameter
. This affects the way queries should be written in particular for highlights and locals. We then should put those specific queries under javascript and typescript (not under ecma) and those queries should be inherited by jsx and tsx respectively. However, tsx also benefits from inheriting things from jsx, but it shouldn't inherit javascript specific queries, just jsx queries.We can all agree that the javascript ecosystem is quite messy and that it presents some unique challenges for tooling developers.
So, as stated above, Ecma based languages share many traits, but it's not a linear and simple inheritance. Because of this we want to share as many queries as possible while avoiding nested inheritance in the queries that can make query behavior unpredictable due to unexpected precedence.
The solution
This PR proposes "public" and "private" versions for javascript, jsx, and typescript, that share the same name, but the "private" version name starts with an underscore (ecma already exists as a sort of base "private" language). This allows the "private" versions to host the specific queries of the language excluding any
; inherits
statement, in order to make them safe to be inherited by the "public" version of the same language and other languages as well. This helps fixing the problems above. The tsx language doesn't have a "private" version given it doesn't need to be inherited by other languages.In summary, I've...
_javascript
,_jsx
, and_typescript
languages (called "private" above).javascript
,jsx
,typescript
, andtsx
languages (called "public" above) to only inherit from "private" languages.In addition to this, I've adjusted some highlights and locals queries to take advantage of this inheritance and fix existing problems. I've put special attention to queries related to parameters, given it is a place where javascript and typescript treesitter grammars have diverged the most (you can use the following files to test that, just add the correct extensions:
test-js.txt, test-jsx.txt, test-ts.txt, test-tsx.txt)
Textobjects can be improved too, but I plan to address that in another PR. Let me know if this seems like a good strategy to tackle this issue.