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

Breaking: Check assignment targets in no-extra-parens #12490

Merged
merged 2 commits into from Jan 17, 2020
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
44 changes: 43 additions & 1 deletion lib/rules/no-extra-parens.js
Expand Up @@ -342,6 +342,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
Expand Down Expand Up @@ -679,6 +690,12 @@ module.exports = {
.forEach(report);
},

ArrayPattern(node) {
node.elements
.filter(e => canBeAssignmentTarget(e) && hasExcessParens(e))
.forEach(report);
},

ArrowFunctionExpression(node) {
if (isReturnAssignException(node)) {
return;
Expand All @@ -705,6 +722,10 @@ module.exports = {
},

AssignmentExpression(node) {
if (canBeAssignmentTarget(node.left) && hasExcessParens(node.left)) {
report(node.left);
}

if (!isReturnAssignException(node) && hasExcessParensWithPrecedence(node.right, precedence(node))) {
report(node.right);
}
Expand Down Expand Up @@ -947,6 +968,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;
Expand All @@ -957,6 +987,14 @@ module.exports = {
}
},

RestElement(node) {
const argument = node.argument;

if (canBeAssignmentTarget(argument) && hasExcessParens(argument)) {
report(argument);
}
},

ReturnStatement(node) {
const returnToken = sourceCode.getFirstToken(node);

Expand Down Expand Up @@ -1054,7 +1092,11 @@ module.exports = {
},

AssignmentPattern(node) {
const { right } = node;
const { left, right } = node;

if (canBeAssignmentTarget(left) && hasExcessParens(left)) {
report(left);
}

if (right && hasExcessParensWithPrecedence(right, PRECEDENCE_OF_ASSIGNMENT_EXPR)) {
report(right);
Expand Down
24 changes: 24 additions & 0 deletions tests/lib/rules/no-extra-parens.js
Expand Up @@ -1403,6 +1403,30 @@ 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 these tests for RestElement's parenthesized arguments in object patterns when that becomes supported by Espree.
*
* invalid("({ ...(a) } = {})", "({ ...a } = {})", "Identifier"),
* invalid("({ ...(a.b) } = {})", "({ ...a.b } = {})", "MemberExpression")
*/

// https://github.com/eslint/eslint/issues/11706 (also in valid[])
{
code: "for ((a = (b in c)); ;);",
Expand Down