tools: Implement automatic fix for no-let-in-for-declaration#16642
tools: Implement automatic fix for no-let-in-for-declaration#16642starkwang wants to merge 1 commit intonodejs:masterfrom
Conversation
apapirovski
left a comment
apapirovski
left a comment
There was a problem hiding this comment.
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
starkwang
force-pushed
the
autofix-no-let
branch
from
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: #16642 Refs: #16636 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: nodejs#16642 Refs: nodejs#16636 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: #16642 Refs: #16636 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
PR-URL: #16642 Refs: #16636 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
7 participants
Refs: #16636
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
tools