tools: auto fix custom eslint rule for no-unescaped-regexp-dot #16673
Conversation
1. Added fixer method for resolving lint error. 2. Added additional valid tests + Output for current and new invalid tests. Refs: nodejs#16636
Fixing some build errors + added correct output for invalid cases Refs: nodejs#16636
| }, | ||
| { | ||
| code: String.raw`/\\./`, | ||
| errors: [{ message: 'Unescaped dot character in regular expression' }] | ||
| errors: [{ message: 'Unescaped dot character in regular expression' }], | ||
| output: String.raw`/\./` |
There was a problem hiding this comment.
This seems wrong... ? I believe it should be more like:
String.raw`/\\\./`The \\ is part of the RegExp.
There was a problem hiding this comment.
Updated the logic and test file.
| const dotOffset = expression.indexOf('.'); | ||
| var correctedExpression = ''; | ||
|
|
||
| if (expression.charAt(dotOffset - 1) === '\\') { |
There was a problem hiding this comment.
I don't think this code should run unless a fixer is requested.
There was a problem hiding this comment.
@apapirovski Fixed.
1. Updated test with correct output strings + added additional invalid case. 2. Refractored the fixer method + logic correction. Refs: nodejs#16636
|
Hm, I am somewhat against landing this. The reason is that this is ambiguous. The user might indeed intend to use the dot as is. If a user would just run eslint with the fix flag the user would not notice the change at all and that is somewhat bad. So I would rather keep it as is. |
|
I am with @BridgeAR on this one. |
|
@apapirovski @joyeecheung @BridgeAR Would you like me to close this PR then ? |
|
@shobhitchittora Yes...I think so. Sorry! |
Refs: #16636
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
Tools