From 93442333728ae9a9b67bd2de95b38840d9145a8e Mon Sep 17 00:00:00 2001 From: rajin Date: Tue, 31 Dec 2019 09:26:32 +0900 Subject: [PATCH] feat(eslint-plugin): [strict-bool-expr] add allowSafe option (#1385) --- .../docs/rules/strict-boolean-expressions.md | 1 + .../src/rules/strict-boolean-expressions.ts | 40 ++++-- .../rules/strict-boolean-expressions.test.ts | 124 ++++++++++++++++++ 3 files changed, 153 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md index fd23d75cb5e..a46ac67a0af 100644 --- 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 index e33b9c8a39e..0d4235e2d7d 100644 --- 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({ allowNullable: { type: 'boolean', }, + allowSafe: { + type: 'boolean', + }, }, additionalProperties: false, }, @@ -52,6 +56,7 @@ export default util.createRule({ { ignoreRhs: false, allowNullable: false, + allowSafe: false, }, ], create(context, [options]) { @@ -59,9 +64,9 @@ export default util.createRule({ 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( node, ); @@ -79,20 +84,31 @@ export default util.createRule({ 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({ 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({ 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({ 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 index f1b98a1c941..244d8511dba 100644 --- 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; + `, + }, ], });