New: rest-spread-spacing rule (fixes #5391) #6278
Conversation
|
LGTM |
|
By analyzing the blame information on this pull request, we identified @nzakas, @scriptdaemon and @pedrottimark to be potential reviewers |
kaicataldo
force-pushed
the
fixes5391
branch
2 times, most recently
from
e321743
to
8bf1df9
Compare
| return { | ||
| SpreadElement: function(node) { | ||
| var args = node.argument, | ||
| operator = sourceCode.getTokenBefore(args); |
There was a problem hiding this comment.
I'd like to see a test for ...(foo).
There was a problem hiding this comment.
Great catch, will update with the fix
|
As this comment mentioned, I think we should support rest elements and experimentalObjectRestSpread in this rule. |
|
Missed that. Thanks - will update and report back. |
|
LGTM |
kaicataldo
changed the title
|
Thanks for pointing that stuff out, @mysticatea :) Updated |
| operator = sourceCode.getTokenBefore(operator); | ||
| } | ||
|
|
||
| if (alwaysSpace && !sourceCode.isSpaceBetweenTokens(operator, args)) { |
There was a problem hiding this comment.
Maybe "never" option of this rule is conflicting with the "always" option of space-in-parens rule: e.g. ...( foo ).
I think this should check spacing between operator and the next token of operator, as same as fix functions.
There was a problem hiding this comment.
Makes sense - updated once more!
kaicataldo
force-pushed
the
fixes5391
branch
2 times, most recently
from
a2b7155
to
4b17cc3
Compare
|
Thank you for great works! (Though I cannot check documents!) |
| }] | ||
| }, | ||
| { | ||
| code: "fn(... ( args ))", |
There was a problem hiding this comment.
Added tests to ensure compatibility with space-in-parens
kaicataldo
force-pushed
the
fixes5391
branch
2 times, most recently
from
4532fe3
to
8ffe15e
Compare
|
LGTM |
|
LGTM |
|
@mysticatea Refactored and simplified this logic after changing to account for parens. Thanks again for reviewing 👍 |
|
Lgtm. Nice job! |
eslint-deprecated
bot
added
the
archived due to age
No description provided.