8000 feat: add `enforceForInnerExpressions` option to `no-extra-boolean-ca… · eslint/eslint@db0b174 · GitHub
[go: up one dir, main page]

Skip to content

Commit db0b174

Browse files
feat: add enforceForInnerExpressions option to no-extra-boolean-cast (#18222)
* fix: [no-extra-boolean-cast] inspect comma expressions and ?? expressions. Fixes #18186 * check ternaries recursively too * pr feedback * change docs * add option for recursive expression checks * better wording * format * enforceForInnerExpressions * feedback * nice, anyOf is clever about additionalProperties * test cov * test coverage * docs
1 parent 0de0909 commit db0b174

File tree

3 files changed

+1457
-216
lines changed

3 files changed

+1457
-216
lines changed

docs/src/rules/no-extra-boolean-cast.md

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,6 @@ title: no-extra-boolean-cast
33
rule_type: suggestion
44
---
55

6-
7-
8-
9-
106
In contexts such as an `if` statement's test where the result of the expression will already be coerced to a Boolean, casting to a Boolean via double negation (`!!`) or a `Boolean` call is unnecessary. For example, these `if` statements are equivalent:
117

128
```js
@@ -88,16 +84,18 @@ var foo = bar ? !!baz : !!bat;
8884

8985
This rule has an object option:
9086

91-
* `"enforceForLogicalOperands"` when set to `true`, in addition to checking default contexts, checks whether the extra boolean cast is contained within a logical expression. Default is `false`, meaning that this rule by default does not warn about extra booleans cast inside logical expression.
87+
* `"enforceForInnerExpressions"` when set to `true`, in addition to checking default contexts, checks whether extra boolean casts are present in expressions whose result is used in a boolean context. See examples below. Default is `false`, meaning that this rule by default does not warn about extra booleans cast inside inner expressions.
88+
89+
**Deprecated:** The object property `enforceForLogicalOperands` is deprecated ([eslint#18222](https://github.com/eslint/eslint/pull/18222)). Please use `enforceForInnerExpressions` instead.
9290

93-
### enforceForLogicalOperands
91+
### enforceForInnerExpressions
9492

95-
Examples of **incorrect** code for this rule with `"enforceForLogicalOperands"` option set to `true`:
93+
Examples of **incorrect** code for this rule with `"enforceForInnerExpressions"` option set to `true`:
9694

9795
::: incorrect
9896

9997
```js
100-
/*eslint no-extra-boolean-cast: ["error", {"enforceForLogicalOperands": true}]*/
98+
/*eslint no-extra-boolean-cast: ["error", {"enforceForInnerExpressions": true}]*/
10199

102100
if (!!foo || bar) {
103101
//...
@@ -107,23 +105,38 @@ while (!!foo && bar) {
107105
//...
108106
}
109107

110-
if ((!!foo || bar) && baz) {
108+
if ((!!foo || bar) && !!baz) {
111109
//...
112110
}
113111

114-
foo && Boolean(bar) ? baz : bat
112+
var foo = new Boolean(!!bar || baz);
115113

116-
var foo = new Boolean(!!bar || baz)
114+
foo && Boolean(bar) ? baz : bat;
115+
116+
const ternaryBranches = Boolean(bar ? !!baz : bat);
117+
118+
const nullishCoalescingOperator = Boolean(bar ?? Boolean(baz));
119+
120+
const commaOperator = Boolean((bar, baz, !!bat));
121+
122+
// another comma operator example
123+
for (let i = 0; console.log(i), Boolean(i < 10); i++) {
124+
// ...
125+
}
117126
```
118127
119128
:::
120129
121-
Examples of **correct** code for this rule with `"enforceForLogicalOperands"` option set to `true`:
130+
Examples of **correct** code for this rule with `"enforceForInnerExpressions"` option set to `true`:
122131
123132
::: correct
124133
125134
```js
126-
/*eslint no-extra-boolean-cast: ["error", {"enforceForLogicalOperands": true}]*/
135+
/*eslint no-extra-boolean-cast: ["error", {"enforceForInnerExpressions": true}]*/
136+
137+
// Note that `||` and `&&` alone aren't a boolean context for either operand
138+
// since the resultant value need not be a boolean without casting.
139+
var foo = !!bar || baz;
127140

128141
if (foo || bar) {
129142
//...
@@ -137,11 +150,23 @@ if ((foo || bar) && baz) {
137150
//...
138151
}
139152

140-
foo && bar ? baz : bat
153+
var foo = new Boolean(bar || baz);
141154

142-
var foo = new Boolean(bar || baz)
155+
foo && bar ? baz : bat;
143156

144-
var foo = !!bar || baz;
157+
const ternaryBranches = Boolean(bar ? baz : bat);
158+
159+
const nullishCoalescingOperator = Boolean(bar ?? baz);
160+
161+
const commaOperator = Boolean((bar, baz, bat));
162+
163+
// another comma operator example
164+
for (let i = 0; console.log(i), i < 10; i++) {
165+
// ...
166+
}
167+
168+
// comma operator in non-final position
169+
Boolean((Boolean(bar), baz, bat));
145170
```
146171
147172
:::

lib/rules/no-extra-boolean-cast.js

Lines changed: 79 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,28 @@ module.exports = {
3030
},
3131

3232
schema: [{
33-
type: "object",
34-
properties: {
35-
enforceForLogicalOperands: {
36-
type: "boolean",
37-
default: false
33+
anyOf: [
34+
{
35+
type: "object",
36+
properties: {
37+
enforceForInnerExpressions: {
38+
type: "boolean"
39+
}
40+
},
41+
additionalProperties: false
42+
},
43+
44+
// deprecated
45+
{
46+
type: "object",
47+
properties: {
48+
enforceForLogicalOperands: {
49+
type: "boolean"
50+
}
51+
},
52+
additionalProperties: false
3853
}
39-
},
40-
additionalProperties: false
54+
]
4155
}],
4256
fixable: "code",
4357

@@ -49,6 +63,9 @@ module.exports = {
4963

5064
create(context) {
5165
const sourceCode = context.sourceCode;
66+
const enforceForLogicalOperands = context.options[0]?.enforceForLogicalOperands === true;
67+
const enforceForInnerExpressions = context.options[0]?.enforceForInnerExpressions === true;
68+
5269

5370
// Node types which have a test which will coerce values to booleans.
5471
const BOOLEAN_NODE_TYPES = new Set([
@@ -72,19 +89,6 @@ module.exports = {
7289
node.callee.name === "Boolean";
7390
}
7491

75-
/**
76-
* Checks whether the node is a logical expression and that the option is enabled
77-
* @param {ASTNode} node the node
78-
* @returns {boolean} if the node is a logical expression and option is enabled
79-
*/
80-
function isLogicalContext(node) {
81-
return node.type === "LogicalExpression" &&
82-
(node.operator === "||" || node.operator === "&&") &&
83-
(context.options.length && context.options[0].enforceForLogicalOperands === true);
84-
85-
}
86-
87-
8892
/**
8993
* Check if a node is in a context where its value would be coerced to a boolean at runtime.
9094
* @param {ASTNode} node The node
@@ -115,12 +119,51 @@ module.exports = {
115119
return isInFlaggedContext(node.parent);
116120
}
117121

118-
return isInBooleanContext(node) ||
119-
(isLogicalContext(node.parent) &&
122+
/*
123+
* legacy behavior - enforceForLogicalOperands will only recurse on
124+
* logical expressions, not on other contexts.
125+
* enforceForInnerExpressions will recurse on logical expressions
126+
* as well as the other recursive syntaxes.
127+
*/
128+
129+
if (enforceForLogicalOperands || enforceForInnerExpressions) {
130+
if (node.parent.type === "LogicalExpression") {
131+
if (node.parent.operator === "||" || node.parent.operator === "&&") {
132+
return isInFlaggedContext(node.parent);
133+
}
120134

121-
// For nested logical statements
122-
isInFlaggedContext(node.parent)
123-
);
135+
// Check the right hand side of a `??` operator.
136+
if (enforceForInnerExpressions &&
137+
node.parent.operator === "??" &&
138+
node.parent.right === node
139+
) {
140+
return isInFlaggedContext(node.parent);
141+
}
142+
}
143+
}
144+
145+
if (enforceForInnerExpressions) {
146+
if (
147+
node.parent.type === "ConditionalExpression" &&
148+
(node.parent.consequent === node || node.parent.alternate === node)
149+
) {
150+
return isInFlaggedContext(node.parent);
151+
}
152+
153+
/*
154+
* Check last expression only in a sequence, i.e. if ((1, 2, Boolean(3))) {}, since
155+
* the others don't affect the result of the expression.
156+
*/
157+
if (
158+
node.parent.type === "SequenceExpression" &&
159+
node.parent.expressions.at(-1) === node
160+
) {
161+
return isInFlaggedContext(node.parent);
162+
}
163+
164+
}
165+
166+
return isInBooleanContext(node);
124167
}
125168

126169

@@ -147,7 +190,6 @@ module.exports = {
147190
* Determines whether the given node needs to be parenthesized when replacing the previous node.
148191
* It assumes that `previousNode` is the node to be reported by this rule, so it has a limited list
149192
* of possible parent node types. By the same assumption, the node's role in a particular parent is already known.
150-
* For example, if the parent is `ConditionalExpression`, `previousNode` must be its `test` child.
151193
* @param {ASTNode} previousNode Previous node.
152194
* @param {ASTNode} node The node to check.
153195
* @throws {Error} (Unreachable.)
@@ -157,6 +199,7 @@ module.exports = {
157199
if (previousNode.parent.type === "ChainExpression") {
158200
return needsParens(previousNode.parent, node);
159201
}
202+
160203
if (isParenthesized(previousNode)) {
161204

162205
// parentheses around the previous node will stay, so there is no need for an additional pair
@@ -174,9 +217,18 @@ module.exports = {
174217
case "DoWhileStatement":
175218
case "WhileStatement":
176219
case "ForStatement":
220+
case "SequenceExpression":
177221
return false;
178222
case "ConditionalExpression":
179-
return precedence(node) <= precedence(parent);
223+
if (previousNode === parent.test) {
224+
return precedence(node) <= precedence(parent);
225+
}
226+
if (previousNode === parent.consequent || previousNode === parent.alternate) {
227+
return precedence(node) < precedence({ type: "AssignmentExpression" });
228+
}
229+
230+
/* c8 ignore next */
231+
throw new Error("Ternary child must be test, consequent, or alternate.");
180232
case "UnaryExpression":
181233
return precedence(node) < precedence(parent);
182234
case "LogicalExpression":

0 commit comments

Comments
 (0)
0