From cb53d0a0acade50b02a748fb0ef55ccaa98e2239 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Tue, 7 Feb 2023 22:34:40 +0100 Subject: [PATCH 1/3] fix: false positive with assignment in `no-extra-parens` Fixes #16850 --- lib/rules/no-extra-parens.js | 33 +++++++++++++++++++++++++++++- tests/lib/rules/no-extra-parens.js | 29 +++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 75ac606ea74..deaabd2e2ed 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -766,6 +766,36 @@ module.exports = { return false; } + /** + * Checks if the left-hand side of an assignment is an identifier 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 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) = () => {}`. + * 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 and + * the right-hand side is an anonymous class or function; otherwise, `false`. + */ + function isAnonymousFunctionAssignmentException({ left, right }) { + if (left.type === "Identifier") { + 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 @@ -804,7 +834,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); } diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 3f57e5b2958..48a7ec26465 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -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"] } ], @@ -3410,6 +3422,21 @@ 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") ] }); From ef6d178ebdc52da551f12ca4116918581bd70efc Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Wed, 8 Feb 2023 12:39:09 +0100 Subject: [PATCH 2/3] Update lib/rules/no-extra-parens.js Updated as suggested Co-authored-by: Milos Djermanovic --- lib/rules/no-extra-parens.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index deaabd2e2ed..64979dd47ab 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -775,7 +775,7 @@ module.exports = { * 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) = () => {}`. + * 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. From ce3951af9d015f212f34a8815d4b62facca1fb81 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Thu, 9 Feb 2023 00:51:07 +0100 Subject: [PATCH 3/3] check assignment operator --- lib/rules/no-extra-parens.js | 18 ++++++++++-------- tests/lib/rules/no-extra-parens.js | 9 ++++++++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 64979dd47ab..d3ea7d33415 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -767,23 +767,25 @@ module.exports = { } /** - * Checks if the left-hand side of an assignment is an identifier and the right-hand side is - * an anonymous class or function. + * 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 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. + * 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 and - * the right-hand side is an anonymous class or function; otherwise, `false`. + * @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, right }) { - if (left.type === "Identifier") { + function isAnonymousFunctionAssignmentException({ left, operator, right }) { + if (left.type === "Identifier" && ["=", "&&=", "||=", "??="].includes(operator)) { const rhsType = right.type; if (rhsType === "ArrowFunctionExpression") { diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 48a7ec26465..6c14cc0ab61 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -3437,6 +3437,13 @@ ruleTester.run("no-extra-parens", rule, { ] }, invalid("((a)) = () => {};", "(a) = () => {};", "Identifier"), - invalid("(a) = (function () {})();", "a = (function () {})();", "Identifier") + invalid("(a) = (function () {})();", "a = (function () {})();", "Identifier"), + ...["**=", "*=", "/=", "%=", "+=", "-=", "<<=", ">>=", ">>>=", "&=", "^=", "|="].map( + operator => invalid( + `(a) ${operator} function () {};`, + `a ${operator} function () {};`, + "Identifier" + ) + ) ] });