Skip to content

Add internal lint rule to guard against raw strings instead of AST_NODE_TYPES #1503

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.

Sign up for GitHub

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

Jump to bottom
Closed
bradzacher opened this issue Jan 23, 2020 · 3 comments · Fixed by #1508
Closed

Add internal lint rule to guard against raw strings instead of AST_NODE_TYPES #1503

bradzacher opened this issue Jan 23, 2020 · 3 comments · Fixed by #1508
Labels
has pr there is a PR raised to close this tests anything to do with testing

Comments

@bradzacher
Copy link
Member

We've gotten most of the cases, but it'd be good to guard against this explicitly in the codebase.

There are a few times where there were things like the following, which aren't strictly false at runtime, but we prefer to use the enums in the codebase.

if (node.type === 'Identifier') {}

An internal lint rule would mean we can add a fixer, and would save us ever having to mention this in code reviews.
@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party tests anything to do with testing 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge and removed 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge tests anything to do with testing awaiting response Issues waiting for a reply from the OP or another party labels Jan 23, 2020
@bradzacher bradzacher mentioned this issue Jan 23, 2020
Merged
@G-Rath
Copy link
Contributor

G-Rath commented Jan 24, 2020

Would love to be able to use this in eslint-plugin-jest as well (and my own custom TypeScript rules).

I've created such a rule for my company's eslint configuration - I'd welcome a review on it :)

If you've happy with that, I'll make a PR into here too.

@bradzacher
Copy link
Member Author

bradzacher commented Jan 24, 2020 •
edited
Loading

hey man, that's literally the code that what I was going to write!
Happy to have it PR'd in here if you're willing. Thanks!!

This would eventually be caught by #315, but it needs to be picked up by someone, so a little interim rule like that will do wonders :)

@G-Rath
Copy link
Contributor

G-Rath commented Jan 24, 2020

@bradzacher No problem :)

I'll have a look at #315 for you as well, to see if I can get it across the line this weekend (or at least kick it a bit closer) :)

@G-Rath G-Rath mentioned this issue Jan 24, 2020
Merged
@armano2 armano2 added the has pr there is a PR raised to close this label Jan 24, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
has pr there is a PR raised to close this tests anything to do with testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(eslint-plugin-internal): add prefer-ast-types-enum
3 participants
@armano2 @G-Rath @bradzacher