Skip to content
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

Update: fix let logic in for-in and for-of loops in no-extra-parens #14011

Merged
merged 1 commit into from Jan 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 27 additions & 23 deletions lib/rules/no-extra-parens.js
Expand Up @@ -844,45 +844,49 @@ module.exports = {
ExportDefaultDeclaration: node => checkExpressionOrExportStatement(node.declaration),
ExpressionStatement: node => checkExpressionOrExportStatement(node.expression),

"ForInStatement, ForOfStatement"(node) {
if (node.left.type !== "VariableDeclarator") {
ForInStatement(node) {
if (node.left.type !== "VariableDeclaration") {
const firstLeftToken = sourceCode.getFirstToken(node.left, astUtils.isNotOpeningParenToken);

if (
firstLeftToken.value === "let" && (

/*
* If `let` is the only thing on the left side of the loop, it's the loop variable: `for ((let) of foo);`
* Removing it will cause a syntax error, because it will be parsed as the start of a VariableDeclarator.
*/
(firstLeftToken.range[1] === node.left.range[1] || /*
* If `let` is followed by a `[` token, it's a property access on the `let` value: `for ((let[foo]) of bar);`
* Removing it will cause the property access to be parsed as a destructuring declaration of `foo` instead.
*/
astUtils.isOpeningBracketToken(
sourceCode.getTokenAfter(firstLeftToken, astUtils.isNotClosingParenToken)
))
firstLeftToken.value === "let" &&
astUtils.isOpeningBracketToken(
sourceCode.getTokenAfter(firstLeftToken, astUtils.isNotClosingParenToken)
)
) {

// ForInStatement#left expression cannot start with `let[`.
tokensToIgnore.add(firstLeftToken);
}
}

if (node.type === "ForOfStatement") {
const hasExtraParens = node.right.type === "SequenceExpression"
? hasDoubleExcessParens(node.right)
: hasExcessParens(node.right);
if (hasExcessParens(node.left)) {
report(node.left);
}

if (hasExtraParens) {
report(node.right);
}
} else if (hasExcessParens(node.right)) {
if (hasExcessParens(node.right)) {
report(node.right);
}
},

ForOfStatement(node) {
if (node.left.type !== "VariableDeclaration") {
const firstLeftToken = sourceCode.getFirstToken(node.left, astUtils.isNotOpeningParenToken);

if (firstLeftToken.value === "let") {

// ForOfStatement#left expression cannot start with `let`.
tokensToIgnore.add(firstLeftToken);
}
}

if (hasExcessParens(node.left)) {
report(node.left);
}

if (hasExcessParensWithPrecedence(node.right, PRECEDENCE_OF_ASSIGNMENT_EXPR)) {
report(node.right);
}
},

ForStatement(node) {
Expand Down
194 changes: 189 additions & 5 deletions tests/lib/rules/no-extra-parens.js
Expand Up @@ -615,10 +615,28 @@ ruleTester.run("no-extra-parens", rule, {
"for ((let)[a]();;);",
"for ((let[a]) + b;;);",

"for ((let) in foo);",
// ForInStatement#left expression cannot start with `let[`. It would be parsed as a `let` declaration with array pattern, or a syntax error.
"for ((let[foo]) in bar);",
"for ((let)[foo] in bar);",
"for ((let[foo].bar) in baz);",
"for ((let[foo]).bar in baz);",
"for ((let)[foo].bar in baz);",

// ForOfStatement#left expression cannot start with `let`. It's explicitly forbidden by the specification.
"for ((let) of foo);",
"for ((let).foo of bar);",
"for ((let.foo) of bar);",
"for ((let[foo]) of bar);",
"for ((let)[foo] of bar);",
"for ((let.foo.bar) of baz);",
"for ((let.foo).bar of baz);",
"for ((let).foo.bar of baz);",
"for ((let[foo].bar) of baz);",
"for ((let[foo]).bar of baz);",
"for ((let)[foo].bar of baz);",
"for ((let)().foo of bar);",
"for ((let()).foo of bar);",
"for ((let().foo) of bar);",

// https://github.com/eslint/eslint/issues/11706 (also in invalid[])
"for (let a = (b in c); ;);",
Expand Down Expand Up @@ -1866,6 +1884,18 @@ ruleTester.run("no-extra-parens", rule, {
"Identifier",
1
),
invalid(
"for (foo of (baz = bar));",
"for (foo of baz = bar);",
"AssignmentExpression",
1
),
invalid(
"function* f() { for (foo of (yield bar)); }",
"function* f() { for (foo of yield bar); }",
"YieldExpression",
1
),
invalid(
"for (foo of ((bar, baz)));",
"for (foo of (bar, baz));",
Expand Down Expand Up @@ -2055,18 +2085,172 @@ ruleTester.run("no-extra-parens", rule, {
1
),

// ForInStatement#left expression cannot start with `let[`, but it can start with `let` if it isn't followed by `[`
invalid(
"for ((let) in foo);",
"for (let in foo);",
"Identifier",
1
),
invalid(
"for ((let())[a] in foo);",
"for (let()[a] in foo);",
"CallExpression",
1
),
invalid(
"for ((let.a) in foo);",
"for (let.a in foo);",
"MemberExpression",
1
),
invalid(
"for ((let).a in foo);",
"for (let.a in foo);",
"Identifier",
1
),
invalid(
"for ((let).a.b in foo);",
"for (let.a.b in foo);",
"Identifier",
1
),
invalid(
"for ((let).a[b] in foo);",
"for (let.a[b] in foo);",
"Identifier",
1
),
invalid(
"for ((let.a)[b] in foo);",
"for (let.a[b] in foo);",
"MemberExpression",
1
),
invalid(
"for ((let.a[b]) in foo);",
"for (let.a[b] in foo);",
"MemberExpression",
1
),
invalid(
"for (((let[a])) in foo);",
"for ((let[a]) in foo);",
"MemberExpression",
1
),
invalid(
"for (((let))[a] in foo);",
"for ((let)[a] in foo);",
"Identifier",
1
),
invalid(
"for (((let[a])).b in foo);",
"for ((let[a]).b in foo);",
"MemberExpression",
1
),
invalid(
"for (((let))[a].b in foo);",
"for ((let)[a].b in foo);",
"Identifier",
1
),
invalid(
"for (((let)[a]).b in foo);",
"for ((let)[a].b in foo);",
"MemberExpression",
1
),
invalid(
"for (((let[a]).b) in foo);",
"for ((let[a]).b in foo);",
"MemberExpression",
1
),
invalid(
"for ((Let[a]) in foo);",
"for (Let[a] in foo);",
"MemberExpression",
1
),
invalid(
"for ((lett)[a] in foo);",
"for (lett[a] in foo);",
"Identifier",
1
),

// ForOfStatement#left expression cannot start with `let`
invalid(
"for (((let)) of foo);",
"for ((let) of foo);",
"Identifier",
1
),
invalid(
"for ((let.foo) in bar);",
"for (let.foo in bar);",
"for (((let)).a of foo);",
"for ((let).a of foo);",
"Identifier",
1
),
invalid(
"for (((let))[a] of foo);",
"for ((let)[a] of foo);",
"Identifier",
1
),
invalid(
"for (((let).a) of foo);",
"for ((let).a of foo);",
"MemberExpression",
1
),
invalid(
"for (((let[a]).b) of foo);",
"for ((let[a]).b of foo);",
"MemberExpression",
1
),
invalid(
"for (((let).a).b of foo);",
"for ((let).a.b of foo);",
"MemberExpression",
1
),
invalid(
"for (((let).a.b) of foo);",
"for ((let).a.b of foo);",
"MemberExpression",
1
),
invalid(
"for ((let).foo.bar in baz);",
"for (let.foo.bar in baz);",
"for (((let.a).b) of foo);",
"for ((let.a).b of foo);",
"MemberExpression",
1
),
invalid(
"for (((let()).a) of foo);",
"for ((let()).a of foo);",
"MemberExpression",
1
),
invalid(
"for ((Let) of foo);",
"for (Let of foo);",
"Identifier",
1
),
invalid(
"for ((lett) of foo);",
"for (lett of foo);",
"Identifier",
1
),

invalid("for (a in (b, c));", "for (a in b, c);", "SequenceExpression", null),
invalid(
"(let)",
Expand Down