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 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
10 changes: 9 additions & 1 deletion docs/src/rules/logical-assignment-operators.md
Original file line number Diff line number Diff line change
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`.
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:

::: 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
]
});