From 59cb6399697d9b3965519fa0e7c6150675071cc3 Mon Sep 17 00:00:00 2001 From: rajin9601 Date: Fri, 27 Dec 2019 12:06:35 +0900 Subject: [PATCH 1/3] feat(eslint-plugin): add allowSafe option to strict-boolean-expressions --- .../docs/rules/strict-boolean-expressions.md | 1 + .../src/rules/strict-boolean-expressions.ts | 40 ++++--- .../rules/strict-boolean-expressions.test.ts | 100 ++++++++++++++++++ 3 files changed, 129 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..c31c64b4124 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: [ @@ -965,5 +994,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; + `, + }, ], }); From ebc9addfc4781b39ce7ee979baca65f304a0539d Mon Sep 17 00:00:00 2001 From: rajin9601 Date: Fri, 27 Dec 2019 14:55:35 +0900 Subject: [PATCH 2/3] fix(eslint-plugin): add test for coverage --- .../rules/strict-boolean-expressions.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 c31c64b4124..064fd59543c 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -954,6 +954,24 @@ ruleTester.run('strict-boolean-expressions', rule, { }, ], }, + { + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 55, + }, + { + messageId: 'strictBooleanExpression', + line: 3, + column: 37, + }, + ], + code: ` + const f1 = (x: boolean | null | undefined) => x ? 1 : 0; + const f2 = (x?: boolean) => x ? 1 : 0; + `, + }, { options: [{ ignoreRhs: true }], errors: [ From cb3e8b575a179d06fa6129106201df3bdff4c741 Mon Sep 17 00:00:00 2001 From: rajin9601 Date: Mon, 30 Dec 2019 12:39:22 +0900 Subject: [PATCH 3/3] test(eslint-plugin): add test for coverage (apply review) --- .../tests/rules/strict-boolean-expressions.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) 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 064fd59543c..244d8511dba 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -966,10 +966,16 @@ ruleTester.run('strict-boolean-expressions', rule, { 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; `, }, {