Skip to content

Commit 977e67e

Browse files
ota-meshimdjermanovicnzakas
authored
feat: logical-assignment-operators to report expressions with 3 operands (#17600)
* fix: logical-assignment-operators to report expressions with 3 operands * Apply suggestions from code review Co-authored-by: Milos Djermanovic <[email protected]> * test: add more `??` test cases * docs: add explanation of always option * Update docs/src/rules/logical-assignment-operators.md Co-authored-by: Nicholas C. Zakas <[email protected]> --------- Co-authored-by: Milos Djermanovic <[email protected]> Co-authored-by: Nicholas C. Zakas <[email protected]>
1 parent bc77c9a commit 977e67e

3 files changed

Lines changed: 207 additions & 4 deletions

File tree

‎docs/src/rules/logical-assignment-operators.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ For example `a = a || b` can be shortened to `a ||= b`.
1010

1111
## Rule Details
1212

13-
This rule requires or disallows logical assignment operator shorthand.
13+
This rule requires or disallows logical assignment operator shorthand.
1414

1515
### Options
1616

@@ -27,6 +27,9 @@ Object option (only available if string option is set to `"always"`):
2727

2828
#### always
2929

30+
This option checks for expressions that can be shortened using logical assignment operator. For example, `a = a || b` can be shortened to `a ||= b`.
31+
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`.
32+
3033
Examples of **incorrect** code for this rule with the default `"always"` option:
3134

3235
::: incorrect
@@ -40,6 +43,9 @@ a = a ?? b
4043
a || (a = b)
4144
a && (a = b)
4245
a ?? (a = b)
46+
a = a || b || c
47+
a = a && b && c
48+
a = a ?? b ?? c
4349
```
4450
4551
:::
@@ -58,6 +64,8 @@ a = b || c
5864
a || (b = c)
5965

6066
if (a) a = b
67+
68+
a = (a || b) || c
6169
```
6270
6371
:::

‎lib/rules/logical-assignment-operators.js‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,31 @@ function isInsideWithBlock(node) {
150150
return node.parent.type === "WithStatement" && node.parent.body === node ? true : isInsideWithBlock(node.parent);
151151
}
152152

153+
/**
154+
* Gets the leftmost operand of a consecutive logical expression.
155+
* @param {SourceCode} sourceCode The ESLint source code object
156+
* @param {LogicalExpression} node LogicalExpression
157+
* @returns {Expression} Leftmost operand
158+
*/
159+
function getLeftmostOperand(sourceCode, node) {
160+
let left = node.left;
161+
162+
while (left.type === "LogicalExpression" && left.operator === node.operator) {
163+
164+
if (astUtils.isParenthesised(sourceCode, left)) {
165+
166+
/*
167+
* It should have associativity,
168+
* but ignore it if use parentheses to make the evaluation order clear.
169+
*/
170+
return left;
171+
}
172+
left = left.left;
173+
}
174+
return left;
175+
176+
}
177+
153178
//------------------------------------------------------------------------------
154179
// Rule Definition
155180
//------------------------------------------------------------------------------
@@ -318,7 +343,10 @@ module.exports = {
318343

319344
// foo = foo || bar
320345
"AssignmentExpression[operator='='][right.type='LogicalExpression']"(assignment) {
321-
if (!astUtils.isSameReference(assignment.left, assignment.right.left)) {
346+
const leftOperand = getLeftmostOperand(sourceCode, assignment.right);
347+
348+
if (!astUtils.isSameReference(assignment.left, leftOperand)
349+
) {
322350
return;
323351
}
324352

@@ -342,10 +370,10 @@ module.exports = {
342370
yield ruleFixer.insertTextBefore(assignmentOperatorToken, assignment.right.operator);
343371

344372
// -> foo ||= bar
345-
const logicalOperatorToken = getOperatorToken(assignment.right);
373+
const logicalOperatorToken = getOperatorToken(leftOperand.parent);
346374
const firstRightOperandToken = sourceCode.getTokenAfter(logicalOperatorToken);
347375

348-
yield ruleFixer.removeRange([assignment.right.range[0], firstRightOperandToken.range[0]]);
376+
yield ruleFixer.removeRange([leftOperand.parent.range[0], firstRightOperandToken.range[0]]);
349377
}
350378
};
351379

‎tests/lib/rules/logical-assignment-operators.js‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,28 @@ ruleTester.run("logical-assignment-operators", rule, {
354354
}, {
355355
code: "a.b = a.b || c",
356356
options: ["never"]
357+
},
358+
359+
// 3 or more operands
360+
{
361+
code: "a = a && b || c",
362+
options: ["always"]
363+
},
364+
{
365+
code: "a = a && b && c || d",
366+
options: ["always"]
367+
},
368+
{
369+
code: "a = (a || b) || c", // Allow if parentheses are used.
370+
options: ["always"]
371+
},
372+
{
373+
code: "a = (a && b) && c", // Allow if parentheses are used.
374+
options: ["always"]
375+
},
376+
{
377+
code: "a = (a ?? b) ?? c", // Allow if parentheses are used.
378+
options: ["always"]
357379
}
358380
],
359381
invalid: [
@@ -1511,6 +1533,151 @@ ruleTester.run("logical-assignment-operators", rule, {
15111533
output: "(a.b.c ||= d) as number"
15121534
}]
15131535
}]
1536+
},
1537+
1538+
// 3 or more operands
1539+
{
1540+
code: "a = a || b || c",
1541+
output: "a ||= b || c",
1542+
options: ["always"],
1543+
errors: [{
1544+
messageId: "assignment",
1545+
type: "AssignmentExpression",
1546+
data: { operator: "||=" },
1547+
suggestions: []
1548+
}]
1549+
},
1550+
{
1551+
code: "a = a && b && c",
1552+
output: "a &&= b && c",
1553+
options: ["always"],
1554+
errors: [{
1555+
messageId: "assignment",
1556+
type: "AssignmentExpression",
1557+
data: { operator: "&&=" },
1558+
suggestions: []
1559+
}]
1560+
},
1561+
{
1562+
code: "a = a ?? b ?? c",
1563+
output: "a ??= b ?? c",
1564+
options: ["always"],
1565+
errors: [{
1566+
messageId: "assignment",
1567+
type: "AssignmentExpression",
1568+
data: { operator: "??=" },
1569+
suggestions: []
1570+
}]
1571+
},
1572+
{
1573+
code: "a = a || b && c",
1574+
output: "a ||= b && c",
1575+
options: ["always"],
1576+
errors: [{
1577+
messageId: "assignment",
1578+
type: "AssignmentExpression",
1579+
data: { operator: "||=" },
1580+
suggestions: []
1581+
}]
1582+
},
1583+
{
1584+
code: "a = a || b || c || d",
1585+
output: "a ||= b || c || d",
1586+
options: ["always"],
1587+
errors: [{
1588+
messageId: "assignment",
1589+
type: "AssignmentExpression",
1590+
data: { operator: "||=" },
1591+
suggestions: []
1592+
}]
1593+
},
1594+
{
1595+
code: "a = a && b && c && d",
1596+
output: "a &&= b && c && d",
1597+
options: ["always"],
1598+
errors: [{
1599+
messageId: "assignment",
1600+
type: "AssignmentExpression",
1601+
data: { operator: "&&=" },
1602+
suggestions: []
1603+
}]
1604+
},
1605+
{
1606+
code: "a = a ?? b ?? c ?? d",
1607+
output: "a ??= b ?? c ?? d",
1608+
options: ["always"],
1609+
errors: [{
1610+
messageId: "assignment",
1611+
type: "AssignmentExpression",
1612+
data: { operator: "??=" },
1613+
suggestions: []
1614+
}]
1615+
},
1616+
{
1617+
code: "a = a || b || c && d",
1618+
output: "a ||= b || c && d",
1619+
options: ["always"],
1620+
errors: [{
1621+
messageId: "assignment",
1622+
type: "AssignmentExpression",
1623+
data: { operator: "||=" },
1624+
suggestions: []
1625+
}]
1626+
},
1627+
{
1628+
code: "a = a || b && c || d",
1629+
output: "a ||= b && c || d",
1630+
options: ["always"],
1631+
errors: [{
1632+
messageId: "assignment",
1633+
type: "AssignmentExpression",
1634+
data: { operator: "||=" },
1635+
suggestions: []
1636+
}]
1637+
},
1638+
{
1639+
code: "a = (a) || b || c",
1640+
output: "a ||= b || c",
1641+
options: ["always"],
1642+
errors: [{
1643+
messageId: "assignment",
1644+
type: "AssignmentExpression",
1645+
data: { operator: "||=" },
1646+
suggestions: []
1647+
}]
1648+
},
1649+
{
1650+
code: "a = a || (b || c) || d",
1651+
output: "a ||= (b || c) || d",
1652+
options: ["always"],
1653+
errors: [{
1654+
messageId: "assignment",
1655+
type: "AssignmentExpression",
1656+
data: { operator: "||=" },
1657+
suggestions: []
1658+
}]
1659+
},
1660+
{
1661+
code: "a = (a || b || c)",
1662+
output: "a ||= (b || c)",
1663+
options: ["always"],
1664+
errors: [{
1665+
messageId: "assignment",
1666+
type: "AssignmentExpression",
1667+
data: { operator: "||=" },
1668+
suggestions: []
1669+
}]
1670+
},
1671+
{
1672+
code: "a = ((a) || (b || c) || d)",
1673+
output: "a ||= ((b || c) || d)",
1674+
options: ["always"],
1675+
errors: [{
1676+
messageId: "assignment",
1677+
type: "AssignmentExpression",
1678+
data: { operator: "||=" },
1679+
suggestions: []
1680+
}]
15141681
}
15151682
]
15161683
});

0 commit comments

Comments
 (0)