-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
tools: Implement automatic fix for no-let-in-for-declaration #16642
Conversation
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.
Hi @starkwang, thanks for working on this! Could you expand the test in test/parallel/test-eslint-no-let-in-for-declaration.js
to test the fixer (output
prop)?
See a sample of how it works here https://github.com/eslint/eslint/blob/master/tests/lib/rules/arrow-body-style.js
ebda2cb
to
ae9b2c6
Compare
@apapirovski I've just added the test for fixer |
Basically LGTM, just note that automatic fixing can introduce bugs if another variable of the same name was declared within the function. |
@tniessen I think we can rely on code reviews for that. If the author does not notice it themselves after running |
Landed in 04ffa36 |
PR-URL: nodejs#16642 Refs: nodejs#16636 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Refs: #16636
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
tools