feat: add enforceForInnerExpressions option to no-extra-boolean-cast

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

@@ -3,10 +3,6 @@ title: no-extra-boolean-cast
 rule_type: suggestion
 ---
 
-
-
-
-
 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:
 
 ```js
@@ -88,16 +84,18 @@ var foo = bar ? !!baz : !!bat;
 
 This rule has an object option:
 
-* `"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.
+* `"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.
+
+**Deprecated:** The object property `enforceForLogicalOperands` is deprecated ([eslint#18222](https://github.com/eslint/eslint/pull/18222)). Please use `enforceForInnerExpressions` instead.
 
-### enforceForLogicalOperands
+### enforceForInnerExpressions
 
-Examples of **incorrect** code for this rule with `"enforceForLogicalOperands"` option set to `true`:
+Examples of **incorrect** code for this rule with `"enforceForInnerExpressions"` option set to `true`:
 
 ::: incorrect
 
 ```js
-/*eslint no-extra-boolean-cast: ["error", {"enforceForLogicalOperands": true}]*/
+/*eslint no-extra-boolean-cast: ["error", {"enforceForInnerExpressions": true}]*/
 
 if (!!foo || bar) {
  //...
@@ -107,23 +105,38 @@ while (!!foo && bar) {
  //...
 }
 
-if ((!!foo || bar) && baz) {
+if ((!!foo || bar) && !!baz) {
  //...
 }
 
-foo && Boolean(bar) ? baz : bat
+var foo = new Boolean(!!bar || baz);
 
-var foo = new Boolean(!!bar || baz)
+foo && Boolean(bar) ? baz : bat;
+
+const ternaryBranches = Boolean(bar ? !!baz : bat);
+
+const nullishCoalescingOperator = Boolean(bar ?? Boolean(baz));
+
+const commaOperator = Boolean((bar, baz, !!bat));
+
+// another comma operator example
+for (let i = 0; console.log(i), Boolean(i < 10); i++) {
+ // ...
+}
 ```
 
 :::
 
-Examples of **correct** code for this rule with `"enforceForLogicalOperands"` option set to `true`:
+Examples of **correct** code for this rule with `"enforceForInnerExpressions"` option set to `true`:
 
 ::: correct
 
 ```js
-/*eslint no-extra-boolean-cast: ["error", {"enforceForLogicalOperands": true}]*/
+/*eslint no-extra-boolean-cast: ["error", {"enforceForInnerExpressions": true}]*/
+
+// Note that `||` and `&&` alone aren't a boolean context for either operand 
+// since the resultant value need not be a boolean without casting.
+var foo = !!bar || baz;
 
 if (foo || bar) {
  //...
@@ -137,11 +150,23 @@ if ((foo || bar) && baz) {
  //...
 }
 
-foo && bar ? baz : bat
+var foo = new Boolean(bar || baz);
 
-var foo = new Boolean(bar || baz)
+foo && bar ? baz : bat;
 
-var foo = !!bar || baz;
+const ternaryBranches = Boolean(bar ? baz : bat);
+
+const nullishCoalescingOperator = Boolean(bar ?? baz);
+
+const commaOperator = Boolean((bar, baz, bat));
+
+// another comma operator example
+for (let i = 0; console.log(i), i < 10; i++) {
+ // ...
+}
+
+// comma operator in non-final position
+Boolean((Boolean(bar), baz, bat));
 ```
 
 :::

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

@@ -30,14 +30,28 @@ module.exports = {
  },
 
  schema: [{
- type: "object",
- properties: {
- enforceForLogicalOperands: {
- type: "boolean",
- default: false
+ anyOf: [
+ {
+ type: "object",
+ properties: {
+ enforceForInnerExpressions: {
+ type: "boolean"
+ }
+ },
+ additionalProperties: false
+ },
+
+ // deprecated
+ {
+ type: "object",
+ properties: {
+ enforceForLogicalOperands: {
+ type: "boolean"
+ }
+ },
+ additionalProperties: false
  }
- },
- additionalProperties: false
+ ]
  }],
  fixable: "code",
 
@@ -49,6 +63,9 @@ module.exports = {
 
  create(context) {
  const sourceCode = context.sourceCode;
+ const enforceForLogicalOperands = context.options[0]?.enforceForLogicalOperands === true;
+ const enforceForInnerExpressions = context.options[0]?.enforceForInnerExpressions === true;
+
 
  // Node types which have a test which will coerce values to booleans.
  const BOOLEAN_NODE_TYPES = new Set([
@@ -72,19 +89,6 @@ module.exports = {
  node.callee.name === "Boolean";
  }
 
- /**
- * Checks whether the node is a logical expression and that the option is enabled
- * @param {ASTNode} node the node
- * @returns {boolean} if the node is a logical expression and option is enabled
- */
- function isLogicalContext(node) {
- return node.type === "LogicalExpression" &&
- (node.operator === "||" || node.operator === "&&") &&
- (context.options.length && context.options[0].enforceForLogicalOperands === true);
-
- }
-
-
  /**
  * Check if a node is in a context where its value would be coerced to a boolean at runtime.
  * @param {ASTNode} node The node
@@ -115,12 +119,51 @@ module.exports = {
  return isInFlaggedContext(node.parent);
  }
 
- return isInBooleanContext(node) ||
- (isLogicalContext(node.parent) &&
+ /*
+ * legacy behavior - enforceForLogicalOperands will only recurse on
+ * logical expressions, not on other contexts.
+ * enforceForInnerExpressions will recurse on logical expressions
+ * as well as the other recursive syntaxes.
+ */
+
+ if (enforceForLogicalOperands || enforceForInnerExpressions) {
+ if (node.parent.type === "LogicalExpression") {
+ if (node.parent.operator === "||" || node.parent.operator === "&&") {
+ return isInFlaggedContext(node.parent);
+ }
 
- // For nested logical statements
- isInFlaggedContext(node.parent)
- );
+ // Check the right hand side of a `??` operator.
+ if (enforceForInnerExpressions &&
+ node.parent.operator === "??" &&
+ node.parent.right === node
+ ) {
+ return isInFlaggedContext(node.parent);
+ }
+ }
+ }
+
+ if (enforceForInnerExpressions) {
+ if (
+ node.parent.type === "ConditionalExpression" &&
+ (node.parent.consequent === node || node.parent.alternate === node)
+ ) {
+ return isInFlaggedContext(node.parent);
+ }
+
+ /*
+ * Check last expression only in a sequence, i.e. if ((1, 2, Boolean(3))) {}, since
+ * the others don't affect the result of the expression.
+ */
+ if (
+ node.parent.type === "SequenceExpression" &&
+ node.parent.expressions.at(-1) === node
+ ) {
+ return isInFlaggedContext(node.parent);
+ }
+
+ }
+
+ return isInBooleanContext(node);
  }
 
 
@@ -147,7 +190,6 @@ module.exports = {
  * Determines whether the given node needs to be parenthesized when replacing the previous node.
  * It assumes that `previousNode` is the node to be reported by this rule, so it has a limited list
  * of possible parent node types. By the same assumption, the node's role in a particular parent is already known.
- * For example, if the parent is `ConditionalExpression`, `previousNode` must be its `test` child.
  * @param {ASTNode} previousNode Previous node.
  * @param {ASTNode} node The node to check.
  * @throws {Error} (Unreachable.)
@@ -157,6 +199,7 @@ module.exports = {
  if (previousNode.parent.type === "ChainExpression") {
  return needsParens(previousNode.parent, node);
  }
+
  if (isParenthesized(previousNode)) {
 
  // parentheses around the previous node will stay, so there is no need for an additional pair
@@ -174,9 +217,18 @@ module.exports = {
  case "DoWhileStatement":
  case "WhileStatement":
  case "ForStatement":
+ case "SequenceExpression":
  return false;
  case "ConditionalExpression":
- return precedence(node) <= precedence(parent);
+ if (previousNode === parent.test) {
+ return precedence(node) <= precedence(parent);
+ }
+ if (previousNode === parent.consequent || previousNode === parent.alternate) {
+ return precedence(node) < precedence({ type: "AssignmentExpression" });
+ }
+
+ /* c8 ignore next */
+ throw new Error("Ternary child must be test, consequent, or alternate.");
  case "UnaryExpression":
  return precedence(node) < precedence(parent);
  case "LogicalExpression":