-
Notifications
You must be signed in to change notification settings - Fork 49
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
Support named imports and destructured requires #232
Conversation
@@ -19,9 +19,66 @@ const avaImportDeclarationAst = { | |||
type: 'Literal', | |||
value: 'ava' | |||
} | |||
}; | |||
}, { | |||
type: 'ImportDeclaration', |
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.
Not happy with the wall of ASTs... any way to make this easier to comprehend?
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.
You might have been able to reduce the amount of AST code if you did the checks programmatically instead of comparing the AST tree, but that will also be more fragile and IMHO less readable, so I think this is fine.
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.
Some comments here and there to define what each AST match against could help.
Tested this locally in a random rule test. Should we create a dedicated test for |
Yes, this needs to be tested. |
@alexzherdev Would you be willing to help review some of the other PRs? 🙏 |
sure 👌 |
@alexzherdev You still need to add test for this. Maybe you can use what's already done to test rules to be in a "real life" scenario. |
@alexzherdev Still interested in finishing this? :) |
6e38a7d
to
74632ac
Compare
74632ac
to
470678f
Compare
☝️updated |
Fixes #121