feat(estlint-plugin): check BooleanExpressions in no-constant-condition

If both sides of a boolean expression are literals, the condition is unnecessary

packages/eslint-plugin/src/rules/no-unnecessary-condition.ts

@@ -30,6 +30,15 @@ const isPossiblyFalsy = (type: ts.Type): boolean =>
 
 const isPossiblyTruthy = (type: ts.Type): boolean =>
   unionTypeParts(type).some(type => !isFalsyType(type));
+
+// isLiteralType only covers numbers and strings, this is a more exhaustive check.
+const isLiteral = (type: ts.Type): boolean =>
+  isBooleanLiteralType(type, true) ||
+  isBooleanLiteralType(type, false) ||
+  type.flags === ts.TypeFlags.Undefined ||
+  type.flags === ts.TypeFlags.Null ||
+  type.flags === ts.TypeFlags.Void ||
+  isLiteralType(type);
 // #endregion
 
 type ExpressionWithTest =
@@ -45,7 +54,12 @@ export type Options = [
   },
 ];
 
-export default createRule<Options, 'alwaysTruthy' | 'alwaysFalsy' | 'never'>({
+export type MessageId =
+  | 'alwaysTruthy'
+  | 'alwaysFalsy'
+  | 'literalBooleanExpression'
+  | 'never';
+export default createRule<Options, MessageId>({
   name: 'no-unnecessary-conditionals',
   meta: {
     type: 'suggestion',
@@ -70,6 +84,8 @@ export default createRule<Options, 'alwaysTruthy' | 'alwaysFalsy' | 'never'>({
     messages: {
       alwaysTruthy: 'Unnecessary conditional, value is always truthy.',
       alwaysFalsy: 'Unnecessary conditional, value is always falsy.',
+      literalBooleanExpression:
+        'Unnecessary conditional, both sides of the expression are literal values',
       never: 'Unnecessary conditional, value is `never`',
     },
   },
@@ -82,15 +98,17 @@ export default createRule<Options, 'alwaysTruthy' | 'alwaysFalsy' | 'never'>({
     const service = getParserServices(context);
     const checker = service.program.getTypeChecker();
 
+    function getNodeType(node: TSESTree.Node): ts.Type {
+      const tsNode = service.esTreeNodeToTSNodeMap.get(node);
+      return getConstrainedTypeAtLocation(checker, tsNode);
+    }
+
     /**
      * Checks if a conditional node is necessary:
      * if the type of the node is always true or always false, it's not necessary.
      */
     function checkNode(node: TSESTree.Node): void {
-      const tsNode = service.esTreeNodeToTSNodeMap.get<ts.ExpressionStatement>(
-        node,
-      );
-      const type = getConstrainedTypeAtLocation(checker, tsNode);
+      const type = getNodeType(node);
 
       // Conditional is always necessary if it involves `any` or `unknown`
       if (isTypeFlagSet(type, TypeFlags.Any | TypeFlags.Unknown)) {
@@ -105,6 +123,26 @@ export default createRule<Options, 'alwaysTruthy' | 'alwaysFalsy' | 'never'>({
       }
     }
 
+    /**
+     * Checks that a binary expression is necessarily conditoinal, reports otherwise.
+     * If both sides of the binary expression are literal values, it's not a necessary condition.
+     *
+     * NOTE: It's also unnecessary if the types that don't overlap at all
+     *    but that case is handled by the Typescript compiler itself.
+     */
+    const BOOL_OPERATORS = ['<', '>', '<=', '>=', '==', '===', '!=', '!=='];
+    function checkIfBinaryExpressionIsNecessaryConditional(
+      node: TSESTree.BinaryExpression,
+    ): void {
+      if (
+        BOOL_OPERATORS.includes(node.operator) &&
+        isLiteral(getNodeType(node.left)) &&
+        isLiteral(getNodeType(node.right))
+      ) {
+        context.report({ node, messageId: 'literalBooleanExpression' });
+      }
+    }
+
     /**
      * Checks that a testable expression is necessarily conditional, reports otherwise.
      * Filters all LogicalExpressions to prevent some duplicate reports.
@@ -133,6 +171,7 @@ export default createRule<Options, 'alwaysTruthy' | 'alwaysFalsy' | 'never'>({
     }
 
     return {
+      BinaryExpression: checkIfBinaryExpressionIsNecessaryConditional,
       ConditionalExpression: checkIfTestExpressionIsNecessaryConditional,
       DoWhileStatement: checkIfTestExpressionIsNecessaryConditional,
       ForStatement: checkIfTestExpressionIsNecessaryConditional,

packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts

@@ -1,5 +1,8 @@
 import path from 'path';
-import rule, { Options } from '../../src/rules/no-unnecessary-condition';
+import rule, {
+  Options,
+  MessageId,
+} from '../../src/rules/no-unnecessary-condition';
 import { RuleTester } from '../RuleTester';
 import {
   TestCaseError,
@@ -16,7 +19,6 @@ const ruleTester = new RuleTester({
   },
 });
 
-type MessageId = 'alwaysTruthy' | 'alwaysFalsy' | 'never';
 const ruleError = (
   line: number,
   column: number,
@@ -71,6 +73,12 @@ function test<T extends string>(t: T) {
   return t ? 'yes' : 'no'
 }`,
 
+    // Boolean expressions
+    `
+function test(a: string) {
+  return a === "a"
+}`,
+
     // Supports ignoring the RHS
     {
       code: `
@@ -134,6 +142,34 @@ function test<T extends 'a' | 'b'>(t: T) {
       errors: [ruleError(3, 10, 'alwaysTruthy')],
     },
 
+    // Boolean expressions
+    {
+      code: `
+function test(a: "a") {
+  return a === "a"
+}`,
+      errors: [ruleError(3, 10, 'literalBooleanExpression')],
+    },
+    {
+      code: `
+const y = 1;
+if (y === 0) {}
+`,
+      errors: [ruleError(3, 5, 'literalBooleanExpression')],
+    },
+    {
+      code: `
+enum Foo {
+  a = 1,
+  b = 2
+}
+
+const x = Foo.a;
+if (x === Foo.a) {}
+`,
+      errors: [ruleError(8, 5, 'literalBooleanExpression')],
+    },
+
     // Still errors on in the expected locations when ignoring RHS
     {
       options: [{ ignoreRhs: true }],