update spaceWithinParensRule to always allow empty parens #3513
Conversation
src/rules/spaceWithinParensRule.ts
Outdated
| } else if (token.kind === ts.SyntaxKind.CloseParenToken) { | ||
| this.checkCloseParenToken(token); | ||
| if (sourceFile.text.charAt(token.end - 2) !== ts.tokenToString(ts.SyntaxKind.OpenParenToken)) { |
There was a problem hiding this comment.
ts.tokenToString(ts.SyntaxKind.OpenParenToken) just returns (. Consider comparing with the string directly.
Same for CloseParenToken above
There was a problem hiding this comment.
Thanks for the feedback, I updated to compare directly with the corresponding strings.
src/rules/spaceWithinParensRule.ts
Outdated
| */ | ||
| if (!ts.isLineBreak(currentChar) && currentChar !== 40) { | ||
| if (!ts.isLineBreak(currentChar) && this.sourceFile.text.charAt(currentPos) !== ts.tokenToString(ts.SyntaxKind.OpenParenToken)) { |
There was a problem hiding this comment.
I don't see a reason to replace the char code. If you don't like the magic number, you can add a const enum (because they are inlined by the compiler) with the charcode.
There was a problem hiding this comment.
Thanks for the feedback, I put back the previous code.
| new Foo( ); | ||
|
|
||
| // empty parens are always allowed | ||
| new Foo(); |
There was a problem hiding this comment.
The current implementation adds an error for new Foo( ), because 1 space !== 2 spaces.
If that is intended, please add a test
There was a problem hiding this comment.
Thanks for the feedback. My understanding is that the existing test was checking for no spaces i.e. new Foo() and the corresponding error "Needs 2 whitespaces..." was appearing. Since the empty parens should no longer error I just originally removed that error expectation and added a test to make sure 2 spaces work as expected. There were existing tests covering the missing single and double space as well as extra spaces, so I didn't originally add more missing space checks. Based on the review feedback I have added a test for new Foo( ); that covers 1 missing space.
|
Thanks @jbsingh |
PR checklist
Overview of change:
Updated spaceWithinParensRule to always allow empty parentheses (). Also removed use of a hard coded character code.
Is there anything you'd like reviewers to focus on?
CHANGELOG.md entry:
[enhancement]
space-within-parensupdated to always allow empty parentheses ().