fix(eslint-plugin): [prefer-optional-chain] include mixed "nullish comparison style" chains in checks (#11533)

Author: mdm317

packages/eslint-plugin/src/rules/no-base-to-string.ts

@@ -317,6 +317,7 @@ export default createRule<Options, MessageIds>({
 
       const declarations = toString.getDeclarations();
 
+      // eslint-disable-next-line @typescript-eslint/prefer-optional-chain
       if (declarations == null || declarations.length !== 1) {
         // If there are multiple declarations, at least one of them must not be
         // the default object toString.

packages/eslint-plugin/src/rules/prefer-includes.ts

@@ -39,7 +39,7 @@ export default createRule({
 
     function isNumber(node: TSESTree.Node, value: number): boolean {
       const evaluated = getStaticValue(node, globalScope);
-      return evaluated != null && evaluated.value === value;
+      return evaluated?.value === value;
     }
 
     function isPositiveCheck(node: TSESTree.BinaryExpression): boolean {

packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts

@@ -10,10 +10,10 @@ import type {
 } from '@typescript-eslint/utils/ts-eslint';
 
 import { AST_NODE_TYPES } from '@typescript-eslint/utils';
-import { unionConstituents } from 'ts-api-utils';
+import { isFalsyType, unionConstituents } from 'ts-api-utils';
 import * as ts from 'typescript';
 
-import type { ValidOperand } from './gatherLogicalOperands';
+import type { LastChainOperand, ValidOperand } from './gatherLogicalOperands';
 import type {
   PreferOptionalChainMessageIds,
   PreferOptionalChainOptions,
@@ -31,7 +31,7 @@ import {
 } from '../../util';
 import { checkNullishAndReport } from './checkNullishAndReport';
 import { compareNodes, NodeComparisonResult } from './compareNodes';
-import { NullishComparisonType } from './gatherLogicalOperands';
+import { ComparisonType, NullishComparisonType } from './gatherLogicalOperands';
 
 function includesType(
   parserServices: ParserServicesWithTypeInformation,
@@ -48,6 +48,109 @@ function includesType(
   return false;
 }
 
+function isAlwaysTruthyOperand(
+  comparedName: TSESTree.Node,
+  nullishComparisonType: NullishComparisonType,
+  parserServices: ParserServicesWithTypeInformation,
+): boolean {
+  const ANY_UNKNOWN_FLAGS = ts.TypeFlags.Any | ts.TypeFlags.Unknown;
+  const comparedNameType = parserServices.getTypeAtLocation(comparedName);
+
+  if (isTypeFlagSet(comparedNameType, ANY_UNKNOWN_FLAGS)) {
+    return false;
+  }
+  switch (nullishComparisonType) {
+    case NullishComparisonType.Boolean:
+    case NullishComparisonType.NotBoolean: {
+      const types = unionConstituents(comparedNameType);
+      return types.every(type => !isFalsyType(type));
+    }
+    case NullishComparisonType.NotStrictEqualUndefined:
+    case NullishComparisonType.NotStrictEqualNull:
+    case NullishComparisonType.StrictEqualNull:
+    case NullishComparisonType.StrictEqualUndefined:
+      return !isTypeFlagSet(
+        comparedNameType,
+        ts.TypeFlags.Null | ts.TypeFlags.Undefined,
+      );
+    case NullishComparisonType.NotEqualNullOrUndefined:
+    case NullishComparisonType.EqualNullOrUndefined:
+      return !isTypeFlagSet(
+        comparedNameType,
+        ts.TypeFlags.Null | ts.TypeFlags.Undefined,
+      );
+  }
+}
+
+function isValidAndLastChainOperand(
+  ComparisonValueType: TSESTree.Node,
+  comparisonType: ComparisonType,
+  parserServices: ParserServicesWithTypeInformation,
+) {
+  const type = parserServices.getTypeAtLocation(ComparisonValueType);
+  const ANY_UNKNOWN_FLAGS = ts.TypeFlags.Any | ts.TypeFlags.Unknown;
+
+  const types = unionConstituents(type);
+  switch (comparisonType) {
+    case ComparisonType.Equal: {
+      const isNullish = types.some(t =>
+        isTypeFlagSet(
+          t,
+          ANY_UNKNOWN_FLAGS | ts.TypeFlags.Null | ts.TypeFlags.Undefined,
+        ),
+      );
+      return !isNullish;
+    }
+    case ComparisonType.StrictEqual: {
+      const isUndefined = types.some(t =>
+        isTypeFlagSet(t, ANY_UNKNOWN_FLAGS | ts.TypeFlags.Undefined),
+      );
+      return !isUndefined;
+    }
+    case ComparisonType.NotStrictEqual: {
+      return types.every(t => isTypeFlagSet(t, ts.TypeFlags.Undefined));
+    }
+    case ComparisonType.NotEqual: {
+      return types.every(t =>
+        isTypeFlagSet(t, ts.TypeFlags.Undefined | ts.TypeFlags.Null),
+      );
+    }
+  }
+}
+function isValidOrLastChainOperand(
+  ComparisonValueType: TSESTree.Node,
+  comparisonType: ComparisonType,
+  parserServices: ParserServicesWithTypeInformation,
+) {
+  const type = parserServices.getTypeAtLocation(ComparisonValueType);
+  const ANY_UNKNOWN_FLAGS = ts.TypeFlags.Any | ts.TypeFlags.Unknown;
+
+  const types = unionConstituents(type);
+  switch (comparisonType) {
+    case ComparisonType.NotEqual: {
+      const isNullish = types.some(t =>
+        isTypeFlagSet(
+          t,
+          ANY_UNKNOWN_FLAGS | ts.TypeFlags.Null | ts.TypeFlags.Undefined,
+        ),
+      );
+      return !isNullish;
+    }
+    case ComparisonType.NotStrictEqual: {
+      const isUndefined = types.some(t =>
+        isTypeFlagSet(t, ANY_UNKNOWN_FLAGS | ts.TypeFlags.Undefined),
+      );
+      return !isUndefined;
+    }
+    case ComparisonType.Equal:
+      return types.every(t =>
+        isTypeFlagSet(t, ts.TypeFlags.Undefined | ts.TypeFlags.Null),
+      );
+    case ComparisonType.StrictEqual:
+      return types.every(t => isTypeFlagSet(t, ts.TypeFlags.Undefined));
+  }
+}
+
 // I hate that these functions are identical aside from the enum values used
 // I can't think of a good way to reuse the code here in a way that will preserve
 // the type safety and simplicity.
@@ -65,18 +168,7 @@ const analyzeAndChainOperand: OperandAnalyzer = (
   chain,
 ) => {
   switch (operand.comparisonType) {
-    case NullishComparisonType.Boolean: {
-      const nextOperand = chain.at(index + 1);
-      if (
-        nextOperand?.comparisonType ===
-          NullishComparisonType.NotStrictEqualNull &&
-        operand.comparedName.type === AST_NODE_TYPES.Identifier
-      ) {
-        return null;
-      }
-      return [operand];
-    }
-
+    case NullishComparisonType.Boolean:
     case NullishComparisonType.NotEqualNullOrUndefined:
       return [operand];
 
@@ -92,7 +184,8 @@ const analyzeAndChainOperand: OperandAnalyzer = (
         return [operand, nextOperand];
       }
       if (
-        includesType(
+        nextOperand &&
+        !includesType(
           parserServices,
           operand.comparedName,
           ts.TypeFlags.Undefined,
@@ -101,10 +194,9 @@ const analyzeAndChainOperand: OperandAnalyzer = (
         // we know the next operand is not an `undefined` check and that this
         // operand includes `undefined` - which means that making this an
         // optional chain would change the runtime behavior of the expression
-        return null;
+        return [operand];
       }
-
-      return [operand];
+      return null;
     }
 
     case NullishComparisonType.NotStrictEqualUndefined: {
@@ -156,6 +248,7 @@ const analyzeOrChainOperand: OperandAnalyzer = (
       ) {
         return [operand, nextOperand];
       }
+
       if (
         includesType(
           parserServices,
@@ -168,7 +261,6 @@ const analyzeOrChainOperand: OperandAnalyzer = (
         // optional chain would change the runtime behavior of the expression
         return null;
       }
-
       return [operand];
     }
 
@@ -207,7 +299,7 @@ const analyzeOrChainOperand: OperandAnalyzer = (
  * @returns The range to report.
  */
 function getReportRange(
-  chain: ValidOperand[],
+  chain: { node: TSESTree.Expression }[],
   boundary: TSESTree.Range,
   sourceCode: SourceCode,
 ): TSESTree.Range {
@@ -247,8 +339,10 @@ function getReportDescriptor(
   node: TSESTree.Node,
   operator: '&&' | '||',
   options: PreferOptionalChainOptions,
-  chain: ValidOperand[],
+  subChain: ValidOperand[],
+  lastChain: (LastChainOperand | ValidOperand) | undefined,
 ): ReportDescriptor<PreferOptionalChainMessageIds> {
+  const chain = lastChain ? [...subChain, lastChain] : subChain;
   const lastOperand = chain[chain.length - 1];
 
   let useSuggestionFixer: boolean;
@@ -264,6 +358,7 @@ function getReportDescriptor(
   // `undefined`, or else we're going to change the final type - which is
   // unsafe and might cause downstream type errors.
   else if (
+    lastChain ||
     lastOperand.comparisonType === NullishComparisonType.EqualNullOrUndefined ||
     lastOperand.comparisonType ===
       NullishComparisonType.NotEqualNullOrUndefined ||
@@ -521,10 +616,11 @@ export function analyzeChain(
   node: TSESTree.Node,
   operator: TSESTree.LogicalExpression['operator'],
   chain: ValidOperand[],
+  lastChainOperand?: LastChainOperand,
 ): void {
   // need at least 2 operands in a chain for it to be a chain
   if (
-    chain.length <= 1 ||
+    chain.length + (lastChainOperand ? 1 : 0) <= 1 ||
     /* istanbul ignore next -- previous checks make this unreachable, but keep it for exhaustiveness check */
     operator === '??'
   ) {
@@ -544,23 +640,28 @@ export function analyzeChain(
   // Things like x !== null && x !== undefined have two nodes, but they are
   // one logical unit here, so we'll allow them to be grouped.
   let subChain: (readonly ValidOperand[] | ValidOperand)[] = [];
+  let lastChain: LastChainOperand | ValidOperand | undefined = undefined;
   const maybeReportThenReset = (
     newChainSeed?: readonly [ValidOperand, ...ValidOperand[]],
   ): void => {
-    if (subChain.length > 1) {
+    if (subChain.length + (lastChain ? 1 : 0) > 1) {
       const subChainFlat = subChain.flat();
+      const maybeNullishNodes = lastChain
+        ? subChainFlat.map(({ node }) => node)
+        : subChainFlat.slice(0, -1).map(({ node }) => node);
       checkNullishAndReport(
         context,
         parserServices,
         options,
-        subChainFlat.slice(0, -1).map(({ node }) => node),
+        maybeNullishNodes,
         getReportDescriptor(
           context.sourceCode,
           parserServices,
           node,
           operator,
           options,
           subChainFlat,
+          lastChain,
         ),
       );
     }
@@ -578,6 +679,7 @@ export function analyzeChain(
     //                          ^^^^^^^^^^^ newChainSeed
     //                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second chain
     subChain = newChainSeed ? [newChainSeed] : [];
+    lastChain = undefined;
   };
 
   for (let i = 0; i < chain.length; i += 1) {
@@ -595,6 +697,35 @@ export function analyzeChain(
       //                    ^^^^^^^ invalid OR chain logical, but still part of
       //                            the chain for combination purposes
 
+      if (lastOperand) {
+        const comparisonResult = compareNodes(
+          lastOperand.comparedName,
+          operand.comparedName,
+        );
+        switch (operand.comparisonType) {
+          case NullishComparisonType.StrictEqualUndefined:
+          case NullishComparisonType.NotStrictEqualUndefined: {
+            if (comparisonResult === NodeComparisonResult.Subset) {
+              lastChain = operand;
+            }
+            break;
+          }
+          case NullishComparisonType.StrictEqualNull:
+          case NullishComparisonType.NotStrictEqualNull: {
+            if (
+              comparisonResult === NodeComparisonResult.Subset &&
+              isAlwaysTruthyOperand(
+                lastOperand.comparedName,
+                lastOperand.comparisonType,
+                parserServices,
+              )
+            ) {
+              lastChain = operand;
+            }
+            break;
+          }
+        }
+      }
       maybeReportThenReset();
       continue;
     }
@@ -624,7 +755,33 @@ export function analyzeChain(
       subChain.push(currentOperand);
     }
   }
+  const lastOperand = subChain.flat().at(-1);
 
+  if (lastOperand && lastChainOperand) {
+    const comparisonResult = compareNodes(
+      lastOperand.comparedName,
+      lastChainOperand.comparedName,
+    );
+    const isValidLastChainOperand =
+      operator === '&&'
+        ? isValidAndLastChainOperand
+        : isValidOrLastChainOperand;
+    if (
+      comparisonResult === NodeComparisonResult.Subset &&
+      (isAlwaysTruthyOperand(
+        lastOperand.comparedName,
+        lastOperand.comparisonType,
+        parserServices,
+      ) ||
+        isValidLastChainOperand(
+          lastChainOperand.comparisonValue,
+          lastChainOperand.comparisonType,
+          parserServices,
+        ))
+    ) {
+      lastChain = lastChainOperand;
+    }
+  }
   // check the leftovers
   maybeReportThenReset();
 }