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

feat: logical-assignment-operators to report expressions with 3 operands #17600

Merged
merged 5 commits into from Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 9 additions & 1 deletion docs/src/rules/logical-assignment-operators.md
Expand Up @@ -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

Expand All @@ -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`.
ota-meshi marked this conversation as resolved.
Show resolved Hide resolved

Examples of **incorrect** code for this rule with the default `"always"` option:

::: incorrect
Expand All @@ -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
```

:::
Expand All @@ -58,6 +64,8 @@ a = b || c
a || (b = c)

if (a) a = b

a = (a || b) || c
```

:::
Expand Down
34 changes: 31 additions & 3 deletions lib/rules/logical-assignment-operators.js
Expand Up @@ -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
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -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;
}

Expand All @@ -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]]);
}
};

Expand Down
167 changes: 167 additions & 0 deletions tests/lib/rules/logical-assignment-operators.js
Expand Up @@ -354,6 +354,28 @@ 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", // Allow if parentheses are used.
options: ["always"]
},
{
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: [
Expand Down Expand Up @@ -1511,6 +1533,151 @@ 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: []
}]
},
ota-meshi marked this conversation as resolved.
Show resolved Hide resolved
{
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: []
}]
},
{
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: []
}]
},
{
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: []
}]
}
ota-meshi marked this conversation as resolved.
Show resolved Hide resolved
]
});