-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Enhancement: Have RuleCreator rules throw an error if used with an incompatible parser #8150
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
Comments
This wouldn't work as this is currently an Invariant within eslint itself. ESLint makes no assumptions about the AST other than the root node being a
Not all those that use our utils build specifically for our parser. Some people just want to use a nice type-safe API that allows them to work on an AST across both JS and TS codebases. So we can't hard-enforce it here without providing mechanisms for people to opt-out. The exception ofc is type-aware rules which hard require our parser as we're the only ones that can provide types.
This could work as we're the only parser that provide these props to the parser services. This also means that we could enforce that parsers like Vue that manually call us properly pass on the required bits. The one danger here is that this line of enforcement would hard-break usecases where people run our rules on JS code under eg the esprima parser (the default one) and expect them to silently do nothing. This can be a pretty common case - so if we do step into this domain we would want to do it really consciously and clearly communicate what's happening, why, and how to fix things. Though there are rules of ours that will work regardless of the parser or base file language (js vs TS) - eg The biggest danger for us is when someone uses a parser like babel that can understand TS but produces the wrong AST. All other cases should be fine? |
I do have a question: Babel and TS-ESLint diverge on some ASTs. Does that mean every ESLint rule out there that handles TS syntax already has an implicit dependency on either parser, or has to build compatibility for both? This is more about 3rd party rules, not us. |
Yes - if they want to support babel as the TS eslint parser. In the beginning our stance was "lets keep the AST the same" |
What if people build rules that handle TS syntax but don't use ts-eslint tooling? That doesn't sound uncommon, right? Rules always have a dependency on the parser used, to different extents. |
That's the same as the case I mentioned above - "If you're building a plugin without types then you're likely going to rely on whatever parser's AST that you're looking at." We're the only ones that provide TS types for the TS-ESTree AST. Babel only provides types for the babel AST - which isn't the same as the ESTree AST. There are going to be cases of people building rules for TS code that don't use our tooling - sure - some people are going to write untyped code. In that case as I said - they'll use whatever AST they find. In some cases that may be babel's - but mostly it'll be ours because, as I mentioned, we're the only ones who show up on Google for the space! So yes, it's not going to be uncommon that people write untyped rules for TS code - but it is uncommon (non-existent even) that people will write rules of any kind to target the babel TS estree ast. |
This will block people from using espree with our rules which I mentioned in the latter part of my comment above We don't want to make config harder by hard forcing everyone to turn off all of our rules on any file not parsed by our parser. On pure JS files it doesn't matter what parser is used - our non-type-aware rules will either silently do nothing (as there's nothing to report on) or will work as expected (eg It's specifically just the TS syntax usecase where we care about what parser was used. |
Makes sense. I'll drop this line of inquiry - seems fruitless. Thanks! |
Before You File a Proposal Please Confirm You Have Done The Following...
Relevant Package
utils
My proposal is suitable for this project
Description
#6814 -> #8146 and the discussion in #8149 are making me think that we really, really don't want users to use a parser that isn't at the very least running
@typescript-eslint/parser
internally. This is for two reasons:#8146 tries enforcing the exact parser being used - but that won't work for all users because some languages require wrapping parsing with their own (https://github.com/typescript-eslint/typescript-eslint/pull/8146/files#r1437273011). So checking the exact parser isn't likely feasible.
Instead, what if we augment
RuleCreator
so that rules verify some basic assumptions about the parsed AST / context beforecreate(context)
?Program
AST nodegetParserServices
on theesTreeNodeToTSNodeMap
andtsNodeToESTreeNodeMap
node maps existinggetParserServices
on theprogram
existing if!allowWithoutFullTypeInformation
Additional Info
No response
The text was updated successfully, but these errors were encountered: