-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(typescript-estree): pass jsDocParsingMode everywhere #7997
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
Conversation
Thanks for the PR, @bradzacher! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
⚡
One day you may want to export this as some sort of setting; if lint rules want to do anything with |
@jakebailey for sure - if we have some concrete usecases that would be an option!
AFAIK the parsed JSDoc AST isn't consumed by anyone (at least nobody's ever mentioned a usecase for it -- people always avoid touching the TS AST if they can).
What part of the picture is missing from the program? Isn't it just some scope analysis / usage information in terms of "this thing was used in JSDoc within a TS file"? |
Yep, this is precisely what I meant.
I'm still holding out hope to reuse info from TS's parser to make eslint-plugin-jsdoc faster; TS parses the same info way faster and it would have already been doing it unconditionally until the new option.
So concretely, you won't get any info about something being deprecated, because that is part of what's skipped (depending on the mode). That and a rule that could use any other kind of tag. |
We actually currently rely on However I just tested and re-tested this and it seems to be working fine even with this PR. I.e. if the rule was unable to get the JSDoc tag from the TS ast this file would have lint errors due to unused disable comments: typescript-eslint/packages/typescript-estree/src/getModifiers.ts Lines 16 to 19 in 39c437a
|
Right; Josh had told me that you used something that didn't use the TS info directly before I proposed/implemented the new option. Mainly that was just an easy example. The other case is see/link, which aren't parsed in the modes used after this PR, which would be needed to create an edge-case free version of no-unused-var, my main hope for the future. |
Okay now I see it. I had to push through all of the nx caching to see it properly. |
I'll fix this and we'll do an out-of-band release to get this out soon. |
Ugh, yeah, they are pulling it out of jsdoc nodes via I guess I really thought that this wasn't the case, but I never verified myself... On the TS side, there's no jsdoc parsing mode besides |
Yeah there was a user in discord that also mentioned that they were using a custom JSDoc tag to power a custom lint rule and it OFC broke with this change. So we can fit a lot of users in a neat box - but we also need to allow people to get out of the box. Worth noting that a lot of those use cases could probably be sorted with a substring match eg |
Overview
@fisker made me realise that we hadn't wired this new option up to every possible spot.
This PR ensures that all locations that we create a program or source file have the new option set to improve our perf.