Skip to content

Commit

Permalink
Fix: no-extra-parens false positives for variables called "let" (#8808)
Browse files Browse the repository at this point in the history
"let" is unusual because it's sometimes parsed as a variable declaration keyword, and sometimes as an identifier for a variable. This commit fixes some bugs in the `no-extra-parens` rule where parentheses are unnecessary for most variable names, but are necessary when the variable is called "let".
  • Loading branch information
not-an-aardvark authored and gyandeeps committed Jun 27, 2017
1 parent 616587f commit eac06f2
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 11 deletions.
31 changes: 21 additions & 10 deletions lib/rules/no-extra-parens.js
Expand Up @@ -426,7 +426,7 @@ module.exports = {
secondToken.type === "Keyword" && (
secondToken.value === "function" ||
secondToken.value === "class" ||
secondToken.value === "let" && astUtils.isOpeningBracketToken(sourceCode.getTokenAfter(secondToken))
secondToken.value === "let" && astUtils.isOpeningBracketToken(sourceCode.getTokenAfter(secondToken, astUtils.isNotClosingParenToken))
)
)
) {
Expand Down Expand Up @@ -512,16 +512,27 @@ module.exports = {
ExportDefaultDeclaration: node => checkExpressionOrExportStatement(node.declaration),
ExpressionStatement: node => checkExpressionOrExportStatement(node.expression),

ForInStatement(node) {
if (hasExcessParens(node.right)) {
report(node.right);
}
if (hasExcessParens(node.left)) {
report(node.left);
}
},
"ForInStatement, ForOfStatement"(node) {
if (node.left.type !== "VariableDeclarator") {
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] ||

ForOfStatement(node) {
// 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)
)
)
) {
tokensToIgnore.add(firstLeftToken);
}
}
if (hasExcessParens(node.right)) {
report(node.right);
}
Expand Down
25 changes: 24 additions & 1 deletion tests/lib/rules/no-extra-parens.js
Expand Up @@ -450,7 +450,12 @@ ruleTester.run("no-extra-parens", rule, {
{
code: "((function(){}).foo)();",
options: ["functions"]
}
},
"(let)[foo]",
"for ((let) in foo);",
"for ((let[foo]) in bar);",
"for ((let)[foo] in bar);",
"for ((let[foo].bar) in baz);"
],

invalid: [
Expand Down Expand Up @@ -1052,6 +1057,24 @@ ruleTester.run("no-extra-parens", rule, {
"MemberExpression",
1,
{ parserOptions: { ecmaVersion: 2015 } }
),
invalid(
"(let).foo",
"let.foo",
"Identifier",
1
),
invalid(
"for ((let.foo) in bar);",
"for (let.foo in bar);",
"MemberExpression",
1
),
invalid(
"for ((let).foo.bar in baz);",
"for (let.foo.bar in baz);",
"Identifier",
1
)
]
});

0 comments on commit eac06f2

Please sign in to comment.