From f0a8739f2cde18f8660d08d60554da55a10d53af Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Thu, 24 Oct 2019 22:52:33 +0200 Subject: [PATCH] Update: Check assignment targets in no-extra-parens --- lib/rules/no-extra-parens.js | 46 +++++++++++++++++++++++++++--- tests/lib/rules/no-extra-parens.js | 19 ++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index f96e572bfee..ae04169a9c2 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -320,6 +320,17 @@ module.exports = { return node.type === "CallExpression" && node.callee.type === "FunctionExpression"; } + /** + * Determines if the given node can be the assignment target in destructuring or the LHS of an assignment. + * This is to avoid an autofix that could change behavior because parsers mistakenly allow invalid syntax, + * such as `(a = b) = c` and `[(a = b) = c] = []`. Ideally, this function shouldn't be necessary. + * @param {ASTNode} [node] The node to check + * @returns {boolean} `true` if the given node can be a valid assignment target + */ + function canBeAssignmentTarget(node) { + return node && (node.type === "Identifier" || node.type === "MemberExpression"); + } + /** * Report the node * @param {ASTNode} node node to evaluate @@ -655,6 +666,12 @@ module.exports = { .forEach(report); }, + ArrayPattern(node) { + node.elements + .filter(e => canBeAssignmentTarget(e) && hasExcessParens(e)) + .forEach(report); + }, + ArrowFunctionExpression(node) { if (isReturnAssignException(node)) { return; @@ -681,11 +698,11 @@ module.exports = { }, AssignmentExpression(node) { - if (isReturnAssignException(node)) { - return; + if (canBeAssignmentTarget(node.left) && hasExcessParens(node.left)) { + report(node.left); } - if (hasExcessParens(node.right) && precedence(node.right) >= precedence(node)) { + if (!isReturnAssignException(node) && hasExcessParens(node.right) && precedence(node.right) >= precedence(node)) { report(node.right); } }, @@ -918,6 +935,15 @@ module.exports = { }).forEach(property => report(property.value)); }, + ObjectPattern(node) { + node.properties + .filter(property => { + const value = property.value; + + return canBeAssignmentTarget(value) && hasExcessParens(value); + }).forEach(property => report(property.value)); + }, + Property(node) { if (node.computed) { const { key } = node; @@ -928,6 +954,14 @@ module.exports = { } }, + RestElement(node) { + const argument = node.argument; + + if (canBeAssignmentTarget(argument) && hasExcessParens(argument)) { + report(argument); + } + }, + ReturnStatement(node) { const returnToken = sourceCode.getFirstToken(node); @@ -1022,7 +1056,11 @@ module.exports = { }, AssignmentPattern(node) { - const { right } = node; + const { left, right } = node; + + if (canBeAssignmentTarget(left) && hasExcessParens(left)) { + report(left); + } if (right && hasExcessParens(right) && precedence(right) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { report(right); diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 2cda39104d2..b7239f9c99b 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -1362,6 +1362,25 @@ ruleTester.run("no-extra-parens", rule, { invalid("const [a = (b)] = []", "const [a = b] = []", "Identifier"), invalid("const {a = (b)} = {}", "const {a = b} = {}", "Identifier"), + // LHS of assigments/Assignment targets + invalid("(a) = b", "a = b", "Identifier"), + invalid("(a.b) = c", "a.b = c", "MemberExpression"), + invalid("(a) += b", "a += b", "Identifier"), + invalid("(a.b) >>= c", "a.b >>= c", "MemberExpression"), + invalid("[(a) = b] = []", "[a = b] = []", "Identifier"), + invalid("[(a.b) = c] = []", "[a.b = c] = []", "MemberExpression"), + invalid("({ a: (b) = c } = {})", "({ a: b = c } = {})", "Identifier"), + invalid("({ a: (b.c) = d } = {})", "({ a: b.c = d } = {})", "MemberExpression"), + invalid("[(a)] = []", "[a] = []", "Identifier"), + invalid("[(a.b)] = []", "[a.b] = []", "MemberExpression"), + invalid("[,(a),,] = []", "[,a,,] = []", "Identifier"), + invalid("[...(a)] = []", "[...a] = []", "Identifier"), + invalid("[...(a.b)] = []", "[...a.b] = []", "MemberExpression"), + invalid("({ a: (b) } = {})", "({ a: b } = {})", "Identifier"), + invalid("({ a: (b.c) } = {})", "({ a: b.c } = {})", "MemberExpression"), + + // TODO: add tests for the RestElement in object patterns when it becomes supported in espree + // https://github.com/eslint/eslint/issues/11706 (also in valid[]) { code: "for ((a = (b in c)); ;);",