From 36b55d07539050080c64eea9198b1ba2adfc834e Mon Sep 17 00:00:00 2001 From: ota-meshi Date: Sat, 23 Sep 2023 09:06:05 +0900 Subject: [PATCH 1/5] fix: logical-assignment-operators to report expressions with 3 operands --- lib/rules/logical-assignment-operators.js | 34 ++++++- .../lib/rules/logical-assignment-operators.js | 97 +++++++++++++++++++ 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/lib/rules/logical-assignment-operators.js b/lib/rules/logical-assignment-operators.js index 27ca585e995..c084c04c8ed 100644 --- a/lib/rules/logical-assignment-operators.js +++ b/lib/rules/logical-assignment-operators.js @@ -150,6 +150,31 @@ function isInsideWithBlock(node) { return node.parent.type === "WithStatement" && node.parent.body === node ? true : isInsideWithBlock(node.parent); } +/** + * Gets the leftmost operand of a consecutive logical expression. + * @param {SourceCode} sourceCode The ESLint source code object + * @param {LogicalExpression} node LogicalExpression + * @returns {Expression} Leftmost operand + */ +function getLeftmostOperand(sourceCode, node) { + let left = node.left; + + while (left.type === "LogicalExpression" && left.operator === node.operator) { + + if (astUtils.isParenthesised(sourceCode, left)) { + + /* + * It should have associativity, + * but ignore it if use parentheses to make the evaluation order clear. + */ + return left; + } + left = left.left; + } + return left; + +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -318,7 +343,10 @@ module.exports = { // foo = foo || bar "AssignmentExpression[operator='='][right.type='LogicalExpression']"(assignment) { - if (!astUtils.isSameReference(assignment.left, assignment.right.left)) { + const leftOperand = getLeftmostOperand(sourceCode, assignment.right); + + if (!astUtils.isSameReference(assignment.left, leftOperand) + ) { return; } @@ -342,10 +370,10 @@ module.exports = { yield ruleFixer.insertTextBefore(assignmentOperatorToken, assignment.right.operator); // -> foo ||= bar - const logicalOperatorToken = getOperatorToken(assignment.right); + const logicalOperatorToken = getOperatorToken(leftOperand.parent); const firstRightOperandToken = sourceCode.getTokenAfter(logicalOperatorToken); - yield ruleFixer.removeRange([assignment.right.range[0], firstRightOperandToken.range[0]]); + yield ruleFixer.removeRange([leftOperand.parent.range[0], firstRightOperandToken.range[0]]); } }; diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 36756815a90..e5caafbeacf 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -354,6 +354,24 @@ ruleTester.run("logical-assignment-operators", rule, { }, { code: "a.b = a.b || c", options: ["never"] + }, + + // 3 or more operands + { + code: "a = a && b || c", + options: ["always"] + }, + { + code: "a = a && b && c || d", + options: ["always"] + }, + { + code: "a = (a || b) || c", // Ignore parentheses if used. + options: ["always"] + }, + { + code: "a = (a && b) && c", // Ignore parentheses if used. + options: ["always"] } ], invalid: [ @@ -1511,6 +1529,85 @@ ruleTester.run("logical-assignment-operators", rule, { output: "(a.b.c ||= d) as number" }] }] + }, + + // 3 or more operands + { + code: "a = a || b || c", + output: "a ||= b || c", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = a && b && c", + output: "a &&= b && c", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "&&=" }, + suggestions: [] + }] + }, + { + code: "a = a || b && c", + output: "a ||= b && c", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = a || b || c || d", + output: "a ||= b || c || d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = a && b && c && d", + output: "a &&= b && c && d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "&&=" }, + suggestions: [] + }] + }, + { + code: "a = a || b || c && d", + output: "a ||= b || c && d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = a || b && c || d", + output: "a ||= b && c || d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] } ] }); From 5f22fed5f8a9eeb7493017581db33bee6118bfea Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Tue, 26 Sep 2023 09:56:32 +0900 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Milos Djermanovic --- .../lib/rules/logical-assignment-operators.js | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index e5caafbeacf..3e86940a578 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -366,11 +366,11 @@ ruleTester.run("logical-assignment-operators", rule, { options: ["always"] }, { - code: "a = (a || b) || c", // Ignore parentheses if used. + code: "a = (a || b) || c", // Allow if parentheses are used. options: ["always"] }, { - code: "a = (a && b) && c", // Ignore parentheses if used. + code: "a = (a && b) && c", // Allow if parentheses are used. options: ["always"] } ], @@ -1554,6 +1554,17 @@ ruleTester.run("logical-assignment-operators", rule, { suggestions: [] }] }, + { + code: "a = a ?? b ?? c", + output: "a ??= b ?? c", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??=" }, + suggestions: [] + }] + }, { code: "a = a || b && c", output: "a ||= b && c", @@ -1608,6 +1619,50 @@ ruleTester.run("logical-assignment-operators", rule, { data: { operator: "||=" }, suggestions: [] }] + }, + { + code: "a = (a) || b || c", + output: "a ||= b || c", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = a || (b || c) || d", + output: "a ||= (b || c) || d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = (a || b || c)", + output: "a ||= (b || c)", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] + }, + { + code: "a = ((a) || (b || c) || d)", + output: "a ||= ((b || c) || d)", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "||=" }, + suggestions: [] + }] } ] }); From 7f0d53b56d55ee82c8fc8601b6d785c24144a3fa Mon Sep 17 00:00:00 2001 From: ota-meshi Date: Tue, 26 Sep 2023 10:01:13 +0900 Subject: [PATCH 3/5] test: add more `??` test cases --- tests/lib/rules/logical-assignment-operators.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/lib/rules/logical-assignment-operators.js b/tests/lib/rules/logical-assignment-operators.js index 3e86940a578..471416322d2 100644 --- a/tests/lib/rules/logical-assignment-operators.js +++ b/tests/lib/rules/logical-assignment-operators.js @@ -372,6 +372,10 @@ ruleTester.run("logical-assignment-operators", rule, { { code: "a = (a && b) && c", // Allow if parentheses are used. options: ["always"] + }, + { + code: "a = (a ?? b) ?? c", // Allow if parentheses are used. + options: ["always"] } ], invalid: [ @@ -1598,6 +1602,17 @@ ruleTester.run("logical-assignment-operators", rule, { suggestions: [] }] }, + { + code: "a = a ?? b ?? c ?? d", + output: "a ??= b ?? c ?? d", + options: ["always"], + errors: [{ + messageId: "assignment", + type: "AssignmentExpression", + data: { operator: "??=" }, + suggestions: [] + }] + }, { code: "a = a || b || c && d", output: "a ||= b || c && d", From 6ffe19af3a4c065017fb630bc4205cf1ec8f133d Mon Sep 17 00:00:00 2001 From: ota-meshi Date: Tue, 26 Sep 2023 10:21:37 +0900 Subject: [PATCH 4/5] docs: add explanation of always option --- docs/src/rules/logical-assignment-operators.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 058afcd3ad7..64aa81cd9be 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -10,7 +10,7 @@ For example `a = a || b` can be shortened to `a ||= b`. ## Rule Details -This rule requires or disallows logical assignment operator shorthand. +This rule requires or disallows logical assignment operator shorthand. ### Options @@ -27,6 +27,9 @@ Object option (only available if string option is set to `"always"`): #### always +This option checks for expressions that can be shortened using logical assignment operator. For example, `a = a || b` can be shortened to `a ||= b`. +Also, expressions with associativity such as `a = a || b || c` are reported as being able to be shortened to `a ||= b || c`. However, it is allowed if the coder makes the evaluation order explicit, such as `a = (a || b) || c`. + Examples of **incorrect** code for this rule with the default `"always"` option: ::: incorrect @@ -40,6 +43,9 @@ a = a ?? b a || (a = b) a && (a = b) a ?? (a = b) +a = a || b || c +a = a && b && c +a = a ?? b ?? c ``` ::: @@ -58,6 +64,8 @@ a = b || c a || (b = c) if (a) a = b + +a = (a || b) || c ``` ::: From 38e1f310a8c552a885ca61864a25f57607cc834c Mon Sep 17 00:00:00 2001 From: Yosuke Ota Date: Wed, 27 Sep 2023 22:03:03 +0900 Subject: [PATCH 5/5] Update docs/src/rules/logical-assignment-operators.md Co-authored-by: Nicholas C. Zakas --- docs/src/rules/logical-assignment-operators.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/rules/logical-assignment-operators.md b/docs/src/rules/logical-assignment-operators.md index 64aa81cd9be..1b59cfa70bb 100644 --- a/docs/src/rules/logical-assignment-operators.md +++ b/docs/src/rules/logical-assignment-operators.md @@ -28,7 +28,7 @@ Object option (only available if string option is set to `"always"`): #### always This option checks for expressions that can be shortened using logical assignment operator. For example, `a = a || b` can be shortened to `a ||= b`. -Also, expressions with associativity such as `a = a || b || c` are reported as being able to be shortened to `a ||= b || c`. However, it is allowed if the coder makes the evaluation order explicit, such as `a = (a || b) || c`. +Expressions with associativity such as `a = a || b || c` are reported as being able to be shortened to `a ||= b || c` unless the evaluation order is explicitly defined using parentheses, such as `a = (a || b) || c`. Examples of **incorrect** code for this rule with the default `"always"` option: