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) {
  //...
@@ -111,19 +109,30 @@ if ((!!foo || bar) && baz) {
  //...
 }
 
-foo && Boolean(bar) ? baz : bat
+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));
 
-var foo = new Boolean(!!bar || baz)
+// 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}]*/
 
 if (foo || bar) {
  //...
@@ -137,11 +146,23 @@ if ((foo || bar) && baz) {
  //...
 }
 
-foo && bar ? baz : bat
+var foo = new Boolean(bar || baz);
+
+foo && bar ? baz : bat;
+
+const ternaryBranches = Boolean(bar ? baz : bat);
 
-var foo = new Boolean(bar || baz)
+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++) {
+ // ...
+}
 
-var foo = !!bar || baz;
+// comma operator in non-final position
+Boolean((Boolean(bar), baz, bat));
 ```
 
 :::

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

@@ -35,6 +35,11 @@ module.exports = {
  enforceForLogicalOperands: {
  type: "boolean",
  default: false
+ },
+
+ enforceForInnerExpressions: {
+ type: "boolean",
+ default: false
  }
  },
  additionalProperties: false
@@ -49,6 +54,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([
@@ -79,9 +87,7 @@ module.exports = {
  */
  function isLogicalContext(node) {
  return node.type === "LogicalExpression" &&
- (node.operator === "||" || node.operator === "&&") &&
- (context.options.length && context.options[0].enforceForLogicalOperands === true);
-
+ (node.operator === "||" || node.operator === "&&");
  }
 
 
@@ -115,12 +121,49 @@ 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 the rest.
+ */
 
- // For nested logical statements
- isInFlaggedContext(node.parent)
- );
+ if (enforceForLogicalOperands || enforceForInnerExpressions) {
+ if (isLogicalContext(node.parent)) {
+ 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);
+ }
+
+ // Check the right hand side of a `??` operator.
+ if (
+ node.parent.type === "LogicalExpression" &&
+ node.parent.operator === "??" &&
+ node.parent.right === 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":