Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: false positive with assignment in no-extra-parens (#16872)
* fix: false positive with assignment in `no-extra-parens`

Fixes #16850

* Update lib/rules/no-extra-parens.js

Updated as suggested

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* check assignment operator

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
fasttime and mdjermanovic committed Feb 9, 2023
1 parent 9dbe06d commit 923f61d
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 2 deletions.
35 changes: 34 additions & 1 deletion lib/rules/no-extra-parens.js
Expand Up @@ -766,6 +766,38 @@ module.exports = {
return false;
}

/**
* Checks if the left-hand side of an assignment is an identifier, the operator is one of
* `=`, `&&=`, `||=` or `??=` and the right-hand side is an anonymous class or function.
*
* As per https://tc39.es/ecma262/#sec-assignment-operators-runtime-semantics-evaluation, an
* assignment involving one of the operators `=`, `&&=`, `||=` or `??=` where the right-hand
* side is an anonymous class or function and the left-hand side is an *unparenthesized*
* identifier has different semantics than other assignments.
* Specifically, when an expression like `foo = function () {}` is evaluated, `foo.name`
* will be set to the string "foo", i.e. the identifier name. The same thing does not happen
* when evaluating `(foo) = function () {}`.
* Since the parenthesizing of the identifier in the left-hand side is significant in this
* special case, the parentheses, if present, should not be flagged as unnecessary.
* @param {ASTNode} node an AssignmentExpression node.
* @returns {boolean} `true` if the left-hand side of the assignment is an identifier, the
* operator is one of `=`, `&&=`, `||=` or `??=` and the right-hand side is an anonymous
* class or function; otherwise, `false`.
*/
function isAnonymousFunctionAssignmentException({ left, operator, right }) {
if (left.type === "Identifier" && ["=", "&&=", "||=", "??="].includes(operator)) {
const rhsType = right.type;

if (rhsType === "ArrowFunctionExpression") {
return true;
}
if ((rhsType === "FunctionExpression" || rhsType === "ClassExpression") && !right.id) {
return true;
}
}
return false;
}

return {
ArrayExpression(node) {
node.elements
Expand Down Expand Up @@ -804,7 +836,8 @@ module.exports = {
},

AssignmentExpression(node) {
if (canBeAssignmentTarget(node.left) && hasExcessParens(node.left)) {
if (canBeAssignmentTarget(node.left) && hasExcessParens(node.left) &&
(!isAnonymousFunctionAssignmentException(node) || isParenthesisedTwice(node.left))) {
report(node.left);
}

Expand Down
36 changes: 35 additions & 1 deletion tests/lib/rules/no-extra-parens.js
Expand Up @@ -771,6 +771,18 @@ ruleTester.run("no-extra-parens", rule, {
{
code: "const net = ipaddr.parseCIDR(/** @type {string} */ (cidr));",
options: ["all", { allowParensAfterCommentPattern: "@type" }]
},

// https://github.com/eslint/eslint/issues/16850
"(a) = function () {};",
"(a) = () => {};",
"(a) = class {};",
"(a) ??= function () {};",
"(a) &&= class extends SuperClass {};",
"(a) ||= async () => {}",
{
code: "((a)) = function () {};",
options: ["functions"]
}
],

Expand Down Expand Up @@ -3410,6 +3422,28 @@ ruleTester.run("no-extra-parens", rule, {
options: ["all"],
parserOptions: { ecmaVersion: 2020 },
errors: [{ messageId: "unexpected" }]
}
},

// https://github.com/eslint/eslint/issues/16850
invalid("(a) = function foo() {};", "a = function foo() {};", "Identifier"),
invalid("(a) = class Bar {};", "a = class Bar {};", "Identifier"),
invalid("(a.b) = function () {};", "a.b = function () {};", "MemberExpression"),
{
code: "(newClass) = [(one)] = class { static * [Symbol.iterator]() { yield 1; } };",
output: "newClass = [one] = class { static * [Symbol.iterator]() { yield 1; } };",
errors: [
{ messageId: "unexpected", type: "Identifier" },
{ messageId: "unexpected", type: "Identifier" }
]
},
invalid("((a)) = () => {};", "(a) = () => {};", "Identifier"),
invalid("(a) = (function () {})();", "a = (function () {})();", "Identifier"),
...["**=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", ">>>=", "&=", "^=", "|="].map(
operator => invalid(
`(a) ${operator} function () {};`,
`a ${operator} function () {};`,
"Identifier"
)
)
]
});

0 comments on commit 923f61d

Please sign in to comment.