From 9bb8308cc527e66cc2de56c362e3d020b5088523 Mon Sep 17 00:00:00 2001 From: Teddy Katz Date: Mon, 26 Jun 2017 01:19:07 -0700 Subject: [PATCH] Fix: no-extra-parens false positives for variables called "let" "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". --- lib/rules/no-extra-parens.js | 31 ++++++++++++++++++++---------- tests/lib/rules/no-extra-parens.js | 25 +++++++++++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 9f48f1d81a9..fd6fd0b78b6 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -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)) ) ) ) { @@ -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); } diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 54990b96e3a..be32041c8ac 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -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: [ @@ -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 ) ] });