feat(eslint-plugin): [strict-bool-expr] add allowSafe option (#1385)

Rajin9601 · bradzacher · commit 93442333728a · 2019-12-30T16:26:32.000-08:00
diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md
@@ -57,6 +57,7 @@ while (typeof str !== 'undefined') {
 Options may be provided as an object with:
 
 - `allowNullable` to allow `undefined` and `null` in addition to `boolean` as a type of all boolean expressions. (`false` by default).
+- `allowSafe` to allow non-falsy types (i.e. non string / number / boolean) in addition to `boolean` as a type of all boolean expressions. (`false` by default).
 - `ignoreRhs` to skip the check on the right hand side of expressions like `a && b` or `a || b` - allows these operators to be used for their short-circuiting behavior. (`false` by default).
 
 ## Related To
diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts
@@ -17,6 +17,7 @@ type Options = [
   {
     ignoreRhs?: boolean;
     allowNullable?: boolean;
+    allowSafe?: boolean;
   },
 ];
 
@@ -40,6 +41,9 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
           allowNullable: {
             type: 'boolean',
           },
+          allowSafe: {
+            type: 'boolean',
+          },
         },
         additionalProperties: false,
       },
@@ -52,16 +56,17 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
     {
       ignoreRhs: false,
       allowNullable: false,
+      allowSafe: false,
     },
   ],
   create(context, [options]) {
     const service = util.getParserServices(context);
     const checker = service.program.getTypeChecker();
 
     /**
-     * Determines if the node has a boolean type.
+     * Determines if the node is safe for boolean type
      */
-    function isBooleanType(node: TSESTree.Node): boolean {
+    function isValidBooleanNode(node: TSESTree.Node): boolean {
       const tsNode = service.esTreeNodeToTSNodeMap.get<ts.ExpressionStatement>(
         node,
       );
@@ -79,20 +84,31 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
             hasBoolean = true;
             continue;
           }
-          if (options.allowNullable) {
-            if (tsutils.isTypeFlagSet(ty, ts.TypeFlags.Null)) {
-              continue;
+
+          if (
+            tsutils.isTypeFlagSet(ty, ts.TypeFlags.Null) ||
+            tsutils.isTypeFlagSet(ty, ts.TypeFlags.Undefined)
+          ) {
+            if (!options.allowNullable) {
+              return false;
             }
-            if (tsutils.isTypeFlagSet(ty, ts.TypeFlags.Undefined)) {
+            continue;
+          }
+
+          if (
+            !tsutils.isTypeFlagSet(ty, ts.TypeFlags.StringLike) &&
+            !tsutils.isTypeFlagSet(ty, ts.TypeFlags.NumberLike)
+          ) {
+            if (options.allowSafe) {
+              hasBoolean = true;
               continue;
             }
           }
-          // Union variant is something else
+
           return false;
         }
         return hasBoolean;
       }
-
       return false;
     }
 
@@ -106,7 +122,7 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
       if (
         node.test !== null &&
         node.test.type !== AST_NODE_TYPES.LogicalExpression &&
-        !isBooleanType(node.test)
+        !isValidBooleanNode(node.test)
       ) {
         reportNode(node.test);
       }
@@ -119,8 +135,8 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
       node: TSESTree.LogicalExpression,
     ): void {
       if (
-        !isBooleanType(node.left) ||
-        (!options.ignoreRhs && !isBooleanType(node.right))
+        !isValidBooleanNode(node.left) ||
+        (!options.ignoreRhs && !isValidBooleanNode(node.right))
       ) {
         reportNode(node);
       }
@@ -132,7 +148,7 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
     function assertUnaryExpressionContainsBoolean(
       node: TSESTree.UnaryExpression,
     ): void {
-      if (!isBooleanType(node.argument)) {
+      if (!isValidBooleanNode(node.argument)) {
         reportNode(node.argument);
       }
     }
diff --git a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts
@@ -178,6 +178,35 @@ ruleTester.run('strict-boolean-expressions', rule, {
       declare const x: string | null;
       y = x ?? 'foo';
     `,
+    {
+      options: [{ allowSafe: true }],
+      code: `
+        type TestType = { a: string; };
+        const f1 = (x: boolean | TestType) => x ? 1 : 0;
+        const f2 = (x: true | TestType) => x ? 1 : 0;
+        const f3 = (x: TestType | false) => x ? 1 : 0;
+      `,
+    },
+    {
+      options: [{ allowNullable: true, allowSafe: true }],
+      code: `
+        type TestType = { a: string; };
+        type TestType2 = { b: number; };
+        const f1 = (x?: boolean | TestType) => x ? 1 : 0;
+        const f2 = (x: TestType | TestType2 | null) => x ? 1 : 0;
+        const f3 = (x?: TestType | TestType2 | null) => x ? 1 : 0;
+        const f4 = (x?: TestType2 | true) => x ? 1 : 0;
+        const f5 = (g?: (x: number) => number) => g ? g(1) : 0;
+      `,
+    },
+    {
+      options: [{ allowNullable: true, allowSafe: true, ignoreRhs: true }],
+      code: `
+        type TestType = { foo? : { bar?: string; }; };
+        const f1 = (x?: TestType) => x && x.foo && x.foo.bar
+        const f2 = (g?: (x: number) => number) => g && g(1)
+      `,
+    },
   ],
 
   invalid: [
@@ -925,6 +954,30 @@ ruleTester.run('strict-boolean-expressions', rule, {
         },
       ],
     },
+    {
+      errors: [
+        {
+          messageId: 'strictBooleanExpression',
+          line: 2,
+          column: 55,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 3,
+          column: 37,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 4,
+          column: 41,
+        },
+      ],
+      code: `
+        const f1 = (x: boolean | null | undefined) => x ? 1 : 0;
+        const f2 = (x?: boolean) => x ? 1 : 0;
+        const f3 = (x: boolean | {}) => x ? 1 : 0;
+      `,
+    },
     {
       options: [{ ignoreRhs: true }],
       errors: [
@@ -965,5 +1018,76 @@ const objAndBool = obj && bool;
         const f = (x?: number) => x ? 1 : 0;
       `,
     },
+    {
+      options: [{ allowSafe: true }],
+      errors: [
+        {
+          messageId: 'strictBooleanExpression',
+          line: 3,
+          column: 42,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 4,
+          column: 42,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 5,
+          column: 44,
+        },
+      ],
+      code: `
+        type Type = { a: string; };
+        const f1 = (x: Type | string) => x ? 1 : 0;
+        const f2 = (x: Type | number) => x ? 1 : 0;
+        const f3 = (x: number | string) => x ? 1 : 0;
+      `,
+    },
+    {
+      options: [{ allowSafe: true }],
+      errors: [
+        {
+          messageId: 'strictBooleanExpression',
+          line: 8,
+          column: 34,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 9,
+          column: 34,
+        },
+      ],
+      code: `
+        enum Enum1 {
+          A, B, C
+        }
+        enum Enum2 {
+          A = 'A', B = 'B', C = 'C'
+        }
+        const f1 = (x: Enum1) => x ? 1 : 0;
+        const f2 = (x: Enum2) => x ? 1 : 0;
+      `,
+    },
+    {
+      options: [{ allowNullable: true, allowSafe: true }],
+      errors: [
+        {
+          messageId: 'strictBooleanExpression',
+          line: 3,
+          column: 43,
+        },
+        {
+          messageId: 'strictBooleanExpression',
+          line: 4,
+          column: 49,
+        },
+      ],
+      code: `
+        type Type = { a: string; };
+        const f1 = (x?: Type | string) => x ? 1 : 0;
+        const f2 = (x: Type | number | null) => x ? 1 : 0;
+      `,
+    },
   ],
 });
