Skip to content

Commit 5d7e622

Browse files
fiskerfisker
authored
Add prefer-logical-operator-over-ternary rule (#1830)
1 parent 75c9214 commit 5d7e622

8 files changed

+565
-3
lines changed

‎configs/recommended.js

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ module.exports = {
8383
'unicorn/prefer-includes': 'error',
8484
'unicorn/prefer-json-parse-buffer': 'off',
8585
'unicorn/prefer-keyboard-event-key': 'error',
86+
'unicorn/prefer-logical-operator-over-ternary': 'error',
8687
'unicorn/prefer-math-trunc': 'error',
8788
'unicorn/prefer-modern-dom-apis': 'error',
8889
'unicorn/prefer-modern-math-apis': 'error',

‎docs/rules/prefer-logical-operator-over-ternary.md

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Prefer using a logical operator over a ternary
2+
3+
<!-- Do not manually modify RULE_NOTICE part. Run: `npm run generate-rule-notices` -->
4+
<!-- RULE_NOTICE -->
5+
✅ *This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.*
6+
7+
💡 *This rule provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).*
8+
<!-- /RULE_NOTICE -->
9+
10+
Disallow ternary operators when simpler logical operator alternatives exist.
11+
12+
Ideally, most reported cases have an equivalent [`Logical OR(||)`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_OR) expression. The rule intentionally provides suggestions instead of auto-fixes, because in many cases, the [nullish coalescing operator (`??`)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator) should be preferred.
13+
14+
## Fail
15+
16+
```js
17+
foo ? foo : bar;
18+
```
19+
20+
```js
21+
foo.bar ? foo.bar : foo.baz
22+
```
23+
24+
```js
25+
foo?.bar ? foo.bar : baz
26+
```
27+
28+
```js
29+
!bar ? foo : bar;
30+
```
31+
32+
## Pass
33+
34+
```js
35+
foo ?? bar;
36+
```
37+
38+
```js
39+
foo || bar;
40+
```
41+
42+
```js
43+
foo ? bar : baz;
44+
```
45+
46+
```js
47+
foo.bar ?? foo.baz
48+
```
49+
50+
```js
51+
foo?.bar ?? baz
52+
```

‎readme.md

+1
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ Each rule has emojis denoting:
123123
| [prefer-includes](docs/rules/prefer-includes.md) | Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence. | ✅ | 🔧 | 💡 |
124124
| [prefer-json-parse-buffer](docs/rules/prefer-json-parse-buffer.md) | Prefer reading a JSON file as a buffer. | | 🔧 | |
125125
| [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md) | Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. | ✅ | 🔧 | |
126+
| [prefer-logical-operator-over-ternary](docs/rules/prefer-logical-operator-over-ternary.md) | Prefer using a logical operator over a ternary. | ✅ | | 💡 |
126127
| [prefer-math-trunc](docs/rules/prefer-math-trunc.md) | Enforce the use of `Math.trunc` instead of bitwise operators. | ✅ | 🔧 | 💡 |
127128
| [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) | Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. | ✅ | 🔧 | |
128129
| [prefer-modern-math-apis](docs/rules/prefer-modern-math-apis.md) | Prefer modern `Math` APIs over legacy patterns. | ✅ | 🔧 | |

‎rules/prefer-logical-operator-over-ternary.js

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
'use strict';
2+
const {isParenthesized, getParenthesizedText} = require('./utils/parentheses.js');
3+
const isSameReference = require('./utils/is-same-reference.js');
4+
const shouldAddParenthesesToLogicalExpressionChild = require('./utils/should-add-parentheses-to-logical-expression-child.js');
5+
const needsSemicolon = require('./utils/needs-semicolon.js');
6+
7+
const MESSAGE_ID_ERROR = 'prefer-logical-operator-over-ternary/error';
8+
const MESSAGE_ID_SUGGESTION = 'prefer-logical-operator-over-ternary/suggestion';
9+
const messages = {
10+
[MESSAGE_ID_ERROR]: 'Prefer using a logical operator over a ternary.',
11+
[MESSAGE_ID_SUGGESTION]: 'Switch to `{{operator}}` operator.',
12+
};
13+
14+
function isSameNode(left, right, sourceCode) {
15+
if (isSameReference(left, right)) {
16+
return true;
17+
}
18+
19+
if (left.type !== right.type) {
20+
return false;
21+
}
22+
23+
switch (left.type) {
24+
case 'AwaitExpression':
25+
return isSameNode(left.argument, right.argument, sourceCode);
26+
27+
case 'LogicalExpression':
28+
return (
29+
left.operator === right.operator
30+
&& isSameNode(left.left, right.left, sourceCode)
31+
&& isSameNode(left.right, right.right, sourceCode)
32+
);
33+
34+
case 'UnaryExpression':
35+
return (
36+
left.operator === right.operator
37+
&& left.prefix === right.prefix
38+
&& isSameNode(left.argument, right.argument, sourceCode)
39+
);
40+
41+
case 'UpdateExpression':
42+
return false;
43+
44+
// No default
45+
}
46+
47+
return sourceCode.getText(left) === sourceCode.getText(right);
48+
}
49+
50+
function fix({
51+
fixer,
52+
sourceCode,
53+
conditionalExpression,
54+
left,
55+
right,
56+
operator,
57+
}) {
58+
let text = [left, right].map((node, index) => {
59+
const isNodeParenthesized = isParenthesized(node, sourceCode);
60+
let text = isNodeParenthesized ? getParenthesizedText(node, sourceCode) : sourceCode.getText(node);
61+
62+
if (
63+
!isNodeParenthesized
64+
&& shouldAddParenthesesToLogicalExpressionChild(node, {operator, property: index === 0 ? 'left' : 'right'})
65+
) {
66+
text = `(${text})`;
67+
}
68+
69+
return text;
70+
}).join(` ${operator} `);
71+
72+
// According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#table
73+
// There should be no cases need add parentheses when switching ternary to logical expression
74+
75+
// ASI
76+
if (needsSemicolon(sourceCode.getTokenBefore(conditionalExpression), sourceCode, text)) {
77+
text = `;${text}`;
78+
}
79+
80+
return fixer.replaceText(conditionalExpression, text);
81+
}
82+
83+
function getProblem({
84+
sourceCode,
85+
conditionalExpression,
86+
left,
87+
right,
88+
}) {
89+
return {
90+
node: conditionalExpression,
91+
messageId: MESSAGE_ID_ERROR,
92+
suggest: ['??', '||'].map(operator => ({
93+
messageId: MESSAGE_ID_SUGGESTION,
94+
data: {operator},
95+
fix: fixer => fix({
96+
fixer,
97+
sourceCode,
98+
conditionalExpression,
99+
left,
100+
right,
101+
operator,
102+
}),
103+
})),
104+
};
105+
}
106+
107+
/** @param {import('eslint').Rule.RuleContext} context */
108+
const create = context => {
109+
const sourceCode = context.getSourceCode();
110+
111+
return {
112+
ConditionalExpression(conditionalExpression) {
113+
const {test, consequent, alternate} = conditionalExpression;
114+
115+
// `foo ? foo : bar`
116+
if (isSameNode(test, consequent, sourceCode)) {
117+
return getProblem({
118+
sourceCode,
119+
conditionalExpression,
120+
left: test,
121+
right: alternate,
122+
});
123+
}
124+
125+
// `!bar ? foo : bar`
126+
if (
127+
test.type === 'UnaryExpression'
128+
&& test.operator === '!'
129+
&& test.prefix
130+
&& isSameNode(test.argument, alternate, sourceCode)
131+
) {
132+
return getProblem({
133+
sourceCode,
134+
conditionalExpression,
135+
left: test.argument,
136+
right: consequent,
137+
});
138+
}
139+
},
140+
};
141+
};
142+
143+
/** @type {import('eslint').Rule.RuleModule} */
144+
module.exports = {
145+
create,
146+
meta: {
147+
type: 'suggestion',
148+
docs: {
149+
description: 'Prefer using a logical operator over a ternary.',
150+
},
151+
152+
hasSuggestions: true,
153+
messages,
154+
},
155+
};

‎rules/utils/should-add-parentheses-to-logical-expression-child.js

+10-3
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,17 @@ Check if parentheses should be added to a `node` when it's used as child of `Log
77
@returns {boolean}
88
*/
99
function shouldAddParenthesesToLogicalExpressionChild(node, {operator, property}) {
10-
// When operator or property is different, need check `LogicalExpression` operator precedence, not implemented
10+
// We are not using this, but we can improve this function with it
1111
/* c8 ignore next 3 */
12-
if (operator !== '??' || property !== 'left') {
13-
throw new Error('Not supported.');
12+
if (!property) {
13+
throw new Error('`property` is required.');
14+
}
15+
16+
if (
17+
node.type === 'LogicalExpression'
18+
&& node.operator === operator
19+
) {
20+
return false;
1421
}
1522

1623
// Not really needed, but more readable

‎test/prefer-logical-operator-over-ternary.mjs

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import outdent from 'outdent';
2+
import {getTester} from './utils/test.mjs';
3+
4+
const {test} = getTester(import.meta);
5+
6+
test.snapshot({
7+
valid: [
8+
'foo ? foo1 : bar;',
9+
'foo.bar ? foo.bar1 : foo.baz',
10+
'foo.bar ? foo1.bar : foo.baz',
11+
'++foo ? ++foo : bar;',
12+
13+
// Not checking
14+
'!!bar ? foo : bar;',
15+
],
16+
invalid: [
17+
'foo ? foo : bar;',
18+
'foo.bar ? foo.bar : foo.baz',
19+
'foo?.bar ? foo.bar : baz',
20+
'!bar ? foo : bar;',
21+
'!!bar ? foo : !bar;',
22+
23+
'foo() ? foo() : bar',
24+
25+
// Children parentheses
26+
'foo ? foo : a && b',
27+
'foo ? foo : a || b',
28+
'foo ? foo : a ?? b',
29+
'a && b ? a && b : bar',
30+
'a || b ? a || b : bar',
31+
'a ?? b ? a ?? b : bar',
32+
'foo ? foo : await a',
33+
'await a ? await a : foo',
34+
35+
// ASI
36+
outdent`
37+
const foo = []
38+
!+a ? b : +a
39+
`,
40+
outdent`
41+
const foo = []
42+
a && b ? a && b : 1
43+
`,
44+
],
45+
});

0 commit comments

Comments
 (0)