From f9cc27c0f1c0e0790a450e8cf428032602c42fc7 Mon Sep 17 00:00:00 2001 From: Jonathan Delgado Date: Fri, 31 May 2019 18:31:59 -0700 Subject: [PATCH 1/4] feat(eslint-plugin): strict-boolean-expressions --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/ROADMAP.md | 2 +- .../docs/rules/strict-boolean-expressions.md | 57 ++ packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/strict-boolean-expressions.ts | 92 ++ .../rules/strict-boolean-expressions.test.ts | 886 ++++++++++++++++++ 6 files changed, 1039 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/docs/rules/strict-boolean-expressions.md create mode 100644 packages/eslint-plugin/src/rules/strict-boolean-expressions.ts create mode 100644 packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 18dc3a6e6cda..f635c596f5ce 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -161,6 +161,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Enforce giving `compare` argument to `Array#sort` | | | :thought_balloon: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | | [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | | +| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions. | | | :thought_balloon: | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | :thought_balloon: | | [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index b2567afe070e..ef562efa0a65 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -90,7 +90,7 @@ | [`prefer-object-spread`] | 🌟 | [`prefer-object-spread`][prefer-object-spread] | | [`radix`] | 🌟 | [`radix`][radix] | | [`restrict-plus-operands`] | ✅ | [`@typescript-eslint/restrict-plus-operands`] | -| [`strict-boolean-expressions`] | 🛑 | N/A | +| [`strict-boolean-expressions`] | ✅ | [`@typescript-eslint/strict-boolean-expressions`] | | [`strict-type-predicates`] | 🛑 | N/A | | [`switch-default`] | 🌟 | [`default-case`][default-case] | | [`triple-equals`] | 🌟 | [`eqeqeq`][eqeqeq] | diff --git a/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md new file mode 100644 index 000000000000..699af9baa431 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/strict-boolean-expressions.md @@ -0,0 +1,57 @@ +# Boolean expressions are limited to booleans (strict-boolean-expressions) + +Requires that any boolean expression is limited to true booleans rather than +casting another primitive to a boolean at runtime. + +It is useful to be explicit, for example, if you were trying to check if a +number was defined. Doing `if (number)` would evaluate to `false` if `number` +was defined and `0`. This rule forces these expressions to be explicit and to +strictly use booleans. + +The following nodes are checked: + +- Arguments to the `!`, `&&`, and `||` operators +- The condition in a conditional expression `(cond ? x : y)` +- Conditions for `if`, `for`, `while`, and `do-while` statements. + +Examples of **incorrect** code for this rule: + +```ts +const number = 0; +if (number) { + return; +} + +let foo = bar || 'foobar'; + +let undefinedItem; +let foo = undefinedItem ? 'foo' : 'bar'; + +let str = 'foo'; +while (str) { + break; +} +``` + +Examples of **correct** code for this rule: + +```ts +const number = 0; +if (typeof number !== 'undefined') { + return; +} + +let foo = typeof bar !== 'undefined' ? bar : 'foobar'; + +let undefinedItem; +let foo = typeof undefinedItem !== 'undefined' ? 'foo' : 'bar'; + +let str = 'foo'; +while (typeof str !== 'undefined') { + break; +} +``` + +## Related To + +- TSLint: [strict-boolean-expressions](https://palantir.github.io/tslint/rules/strict-boolean-expressions) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index d62f58d6af3f..7b4e9c487eaa 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -49,6 +49,7 @@ import promiseFunctionAsync from './promise-function-async'; import requireArraySortCompare from './require-array-sort-compare'; import restrictPlusOperands from './restrict-plus-operands'; import semi from './semi'; +import strictBooleanExpressions from './strict-boolean-expressions'; import typeAnnotationSpacing from './type-annotation-spacing'; import unboundMethod from './unbound-method'; import unifiedSignatures from './unified-signatures'; @@ -105,6 +106,7 @@ export default { 'require-array-sort-compare': requireArraySortCompare, 'restrict-plus-operands': restrictPlusOperands, semi: semi, + 'strict-boolean-expressions': strictBooleanExpressions, 'type-annotation-spacing': typeAnnotationSpacing, 'unbound-method': unboundMethod, 'unified-signatures': unifiedSignatures, diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts new file mode 100644 index 000000000000..5ee7d14f29b8 --- /dev/null +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -0,0 +1,92 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import ts from 'typescript'; +import * as util from '../util'; +import { getTypeName } from '../util'; + +type ExpressionWithTest = + | TSESTree.ConditionalExpression + | TSESTree.DoWhileStatement + | TSESTree.ForStatement + | TSESTree.IfStatement + | TSESTree.WhileStatement; + +export default util.createRule({ + name: 'strict-boolean-expressions', + meta: { + type: 'suggestion', + docs: { + description: 'Restricts the types allowed in boolean expressions', + category: 'Best Practices', + recommended: false, + }, + schema: [], + messages: { + strictBooleanExpression: 'Unexpected non-boolean in conditional.', + }, + }, + defaultOptions: [], + create(context) { + const service = util.getParserServices(context); + const checker = service.program.getTypeChecker(); + + /** + * Determines if the node has a boolean type. Does recursion for nodes with + * left/right operators. + */ + function isBooleanType(node: TSESTree.Expression): boolean { + if (node.type === AST_NODE_TYPES.LogicalExpression) { + return isBooleanType(node.left) && isBooleanType(node.right); + } + + const tsNode = service.esTreeNodeToTSNodeMap.get( + node, + ); + const type = checker.getTypeAtLocation(tsNode); + const typeName = getTypeName(checker, type); + return ['true', 'false', 'boolean'].includes(typeName); + } + + /** + * Asserts that a node is a boolean, reports otherwise. + */ + function assertNodeIsBoolean(node: TSESTree.Expression): void { + if (!isBooleanType(node)) { + return context.report({ node, messageId: 'strictBooleanExpression' }); + } + } + + /** + * Asserts that an expression contains a boolean, reports otherwise. Filters + * all LogicalExpressions to prevent some duplicate reports. + */ + function assertExpressionContainsBoolean(node: ExpressionWithTest): void { + if ( + node.test !== null && + node.test.type !== AST_NODE_TYPES.LogicalExpression + ) { + assertNodeIsBoolean(node.test); + } + } + + return { + ConditionalExpression: assertExpressionContainsBoolean, + DoWhileStatement: assertExpressionContainsBoolean, + ForStatement: assertExpressionContainsBoolean, + IfStatement: assertExpressionContainsBoolean, + WhileStatement: assertExpressionContainsBoolean, + LogicalExpression(node: TSESTree.LogicalExpression) { + if (node.operator === '||' || node.operator === '&&') { + assertNodeIsBoolean(node); + } + }, + UnaryExpression(node) { + if (node.operator === '!') { + assertNodeIsBoolean(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 new file mode 100644 index 000000000000..0fab0e4eae0f --- /dev/null +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -0,0 +1,886 @@ +import path from 'path'; +import rule from '../../src/rules/strict-boolean-expressions'; +import { RuleTester } from '../RuleTester'; + +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('strict-boolean-expressions', rule, { + valid: [ + ` + let val = true; + let bool = !val; + let bool2 = true || val; + let bool3 = true && val; + `, + ` + const bool1 = true; + const bool2 = false; + if (true) { + return; + } + + if (bool1) { + return; + } + + if (bool1 && bool2) { + return; + } + + if (bool1 || bool2) { + return; + } + + if ((bool1 && bool2) || (bool1 || bool2)) { + return; + } + `, + ` + const bool1 = true; + const bool2 = false; + const res1 = true ? true : false; + const res2 = bool1 && bool2 ? true : false; + const res3 = bool1 || bool2 ? true : false; + const res4 = (bool1 && bool2) || (bool1 || bool2) ? true : false; + `, + ` + for (let i = 0; true; i++) { + break; + } + `, + ` + const bool = true; + for (let i = 0; bool; i++) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + for (let i = 0; bool1 && bool2; i++) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + for (let i = 0; bool1 || bool2; i++) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + for (let i = 0; (bool1 && bool2) || (bool1 || bool2); i++) { + break; + } + `, + ` + while (true) { + break; + } + `, + ` + const bool = true; + while (bool) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + while (bool1 && bool2) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + while (bool1 || bool2) { + break; + } + `, + ` + const bool1 = true; + const bool2 = false; + while ((bool1 && bool2) || (bool1 || bool2)) { + break; + } + `, + ` + do { + break; + } while (true); + `, + ` + const bool = true; + do { + break; + } while (bool); + `, + ` + const bool1 = true; + const bool2 = false; + do { + break; + } while (bool1 && bool2); + `, + ` + const bool1 = true; + const bool2 = false; + do { + break; + } while (bool1 || bool2); + `, + ` + const bool1 = true; + const bool2 = false; + do { + break; + } while ((bool1 && bool2) || (bool1 || bool2)); + `, + ], + + invalid: [ + { + code: ` + let val = 1; + let bool = !val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 21, + }, + ], + }, + { + code: ` + let val; + let bool = !val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 21, + }, + ], + }, + { + code: ` + let val = 1; + let bool = true && val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 20, + }, + ], + }, + { + code: ` + let val; + let bool = true && val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 20, + }, + ], + }, + { + code: ` + let val = 1; + let bool = true || val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 20, + }, + ], + }, + { + code: ` + let val; + let bool = true || val; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 20, + }, + ], + }, + { + code: ` + if (1) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 13, + }, + ], + }, + { + code: ` + if (undefined) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 13, + }, + ], + }, + { + code: ` + let item = 1; + if (item) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 13, + }, + ], + }, + { + code: ` + let item; + if (item) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 13, + }, + ], + }, + { + code: ` + let item1 = true; + let item2 = 1; + if (item1 && item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + let item1 = 1; + let item2 = true; + if (item1 && item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + let item1; + let item2 = true; + if (item1 && item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + let item1 = true; + let item2 = 1; + if (item1 || item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + let item1 = 1; + let item2 = true; + if (item1 || item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + let item1; + let item2 = true; + if (item1 || item2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 13, + }, + ], + }, + { + code: ` + const bool = 1 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 22, + }, + ], + }, + { + code: ` + const bool = undefined ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 22, + }, + ], + }, + { + code: ` + let item = 1; + const bool = item ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 22, + }, + ], + }, + { + code: ` + let item; + const bool = item ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 22, + }, + ], + }, + { + code: ` + let item1 = 1; + let item2 = false; + const bool = item1 && item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + let item1 = true; + let item2 = 1; + const bool = item1 && item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + let item1 = true; + let item2; + const bool = item1 && item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + let item1 = 1; + let item2 = false; + const bool = item1 || item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + let item1 = true; + let item2 = 1; + const bool = item1 || item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + let item1 = true; + let item2; + const bool = item1 || item2 ? true : false; + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 22, + }, + ], + }, + { + code: ` + for (let i = 0; 1; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 25, + }, + ], + }, + { + code: ` + for (let i = 0; undefined; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 25, + }, + ], + }, + { + code: ` + let bool = 1; + for (let i = 0; bool; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 25, + }, + ], + }, + { + code: ` + let bool; + for (let i = 0; bool; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 25, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + for (let i = 0; bool1 && bool2; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 25, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + for (let i = 0; bool1 && bool2; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 25, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + for (let i = 0; bool1 || bool2; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 25, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + for (let i = 0; bool1 || bool2; i++) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 25, + }, + ], + }, + { + code: ` + while (1) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 16, + }, + ], + }, + { + code: ` + while (undefined) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 16, + }, + ], + }, + { + code: ` + let bool = 1; + while (bool) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 16, + }, + ], + }, + { + code: ` + let bool; + while (bool) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 3, + column: 16, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + while (bool1 && bool2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 16, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + while (bool1 && bool2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 16, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + while (bool1 || bool2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 16, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + while (bool1 || bool2) { + return; + } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 16, + }, + ], + }, + { + code: ` + do { + return; + } while (1); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 18, + }, + ], + }, + { + code: ` + do { + return; + } while (undefined); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 4, + column: 18, + }, + ], + }, + { + code: ` + let bool = 1; + do { + return; + } while (bool); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 5, + column: 18, + }, + ], + }, + { + code: ` + let bool; + do { + return; + } while (bool); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 5, + column: 18, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + do { + return; + } while (bool1 && bool2); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 6, + column: 18, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + do { + return; + } while (bool1 && bool2); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 6, + column: 18, + }, + ], + }, + { + code: ` + let bool1 = 1; + let bool2 = true; + do { + return; + } while (bool1 || bool2); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 6, + column: 18, + }, + ], + }, + { + code: ` + let bool1; + let bool2 = true; + do { + return; + } while (bool1 || bool2); + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 6, + column: 18, + }, + ], + }, + ], +}); From f6d869bb876324909b51daa98f92711103a3954b Mon Sep 17 00:00:00 2001 From: Jonathan Delgado Date: Fri, 31 May 2019 18:42:55 -0700 Subject: [PATCH 2/4] chore(eslint-plugin): fix readme build --- packages/eslint-plugin/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f635c596f5ce..fa3cd79149a2 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -161,7 +161,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Enforce giving `compare` argument to `Array#sort` | | | :thought_balloon: | | [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | | [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | | -| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions. | | | :thought_balloon: | +| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: | | [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | :thought_balloon: | | [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | | From 43ba5be49174e170c11cc0bd06d49059ade56f04 Mon Sep 17 00:00:00 2001 From: Jonathan Delgado Date: Sat, 8 Jun 2019 08:39:58 -0700 Subject: [PATCH 3/4] chore(eslint-plugin): improve coverage --- .../eslint-plugin/src/rules/strict-boolean-expressions.ts | 7 ++----- .../tests/rules/strict-boolean-expressions.test.ts | 6 ++++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 5ee7d14f29b8..8af0163f1f00 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -4,7 +4,6 @@ import { } from '@typescript-eslint/experimental-utils'; import ts from 'typescript'; import * as util from '../util'; -import { getTypeName } from '../util'; type ExpressionWithTest = | TSESTree.ConditionalExpression @@ -45,7 +44,7 @@ export default util.createRule({ node, ); const type = checker.getTypeAtLocation(tsNode); - const typeName = getTypeName(checker, type); + const typeName = util.getTypeName(checker, type); return ['true', 'false', 'boolean'].includes(typeName); } @@ -78,9 +77,7 @@ export default util.createRule({ IfStatement: assertExpressionContainsBoolean, WhileStatement: assertExpressionContainsBoolean, LogicalExpression(node: TSESTree.LogicalExpression) { - if (node.operator === '||' || node.operator === '&&') { - assertNodeIsBoolean(node); - } + assertNodeIsBoolean(node); }, UnaryExpression(node) { if (node.operator === '!') { 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 0fab0e4eae0f..3c0c44222834 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -20,6 +20,12 @@ ruleTester.run('strict-boolean-expressions', rule, { let bool2 = true || val; let bool3 = true && val; `, + ` + let a = 0; + let u1 = typeof a; + let u2 = -a; + let u3 = ~a; + `, ` const bool1 = true; const bool2 = false; From f3b716189ad6b7d8361925d1ecae19e1a053e4a2 Mon Sep 17 00:00:00 2001 From: Jonathan Delgado Date: Sat, 29 Jun 2019 15:21:45 -0700 Subject: [PATCH 4/4] chore(eslint-plugin): refactor - Add support for generics - Abstract Logical/Unary expressions - Abstract reports for multiple calls - Filter UnaryExpressions at the walker --- .../src/rules/strict-boolean-expressions.ts | 82 +++++++++++-------- .../rules/strict-boolean-expressions.test.ts | 15 ++++ 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts index 8af0163f1f00..5118b46a2486 100644 --- a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -3,6 +3,7 @@ import { AST_NODE_TYPES, } from '@typescript-eslint/experimental-utils'; import ts from 'typescript'; +import * as tsutils from 'tsutils'; import * as util from '../util'; type ExpressionWithTest = @@ -32,58 +33,69 @@ export default util.createRule({ const checker = service.program.getTypeChecker(); /** - * Determines if the node has a boolean type. Does recursion for nodes with - * left/right operators. + * Determines if the node has a boolean type. */ - function isBooleanType(node: TSESTree.Expression): boolean { - if (node.type === AST_NODE_TYPES.LogicalExpression) { - return isBooleanType(node.left) && isBooleanType(node.right); - } - + function isBooleanType(node: TSESTree.Node): boolean { const tsNode = service.esTreeNodeToTSNodeMap.get( node, ); - const type = checker.getTypeAtLocation(tsNode); - const typeName = util.getTypeName(checker, type); - return ['true', 'false', 'boolean'].includes(typeName); + const type = util.getConstrainedTypeAtLocation(checker, tsNode); + return tsutils.isTypeFlagSet(type, ts.TypeFlags.BooleanLike); } /** - * Asserts that a node is a boolean, reports otherwise. + * Asserts that a testable expression contains a boolean, reports otherwise. + * Filters all LogicalExpressions to prevent some duplicate reports. */ - function assertNodeIsBoolean(node: TSESTree.Expression): void { - if (!isBooleanType(node)) { - return context.report({ node, messageId: 'strictBooleanExpression' }); + function assertTestExpressionContainsBoolean( + node: ExpressionWithTest, + ): void { + if ( + node.test !== null && + node.test.type !== AST_NODE_TYPES.LogicalExpression && + !isBooleanType(node.test) + ) { + reportNode(node.test); } } /** - * Asserts that an expression contains a boolean, reports otherwise. Filters - * all LogicalExpressions to prevent some duplicate reports. + * Asserts that a logical expression contains a boolean, reports otherwise. */ - function assertExpressionContainsBoolean(node: ExpressionWithTest): void { - if ( - node.test !== null && - node.test.type !== AST_NODE_TYPES.LogicalExpression - ) { - assertNodeIsBoolean(node.test); + function assertLocalExpressionContainsBoolean( + node: TSESTree.LogicalExpression, + ): void { + if (!isBooleanType(node.left) || !isBooleanType(node.right)) { + reportNode(node); } } + /** + * Asserts that a unary expression contains a boolean, reports otherwise. + */ + function assertUnaryExpressionContainsBoolean( + node: TSESTree.UnaryExpression, + ): void { + if (!isBooleanType(node.argument)) { + reportNode(node.argument); + } + } + + /** + * Reports an offending node in context. + */ + function reportNode(node: TSESTree.Node): void { + context.report({ node, messageId: 'strictBooleanExpression' }); + } + return { - ConditionalExpression: assertExpressionContainsBoolean, - DoWhileStatement: assertExpressionContainsBoolean, - ForStatement: assertExpressionContainsBoolean, - IfStatement: assertExpressionContainsBoolean, - WhileStatement: assertExpressionContainsBoolean, - LogicalExpression(node: TSESTree.LogicalExpression) { - assertNodeIsBoolean(node); - }, - UnaryExpression(node) { - if (node.operator === '!') { - assertNodeIsBoolean(node.argument); - } - }, + ConditionalExpression: assertTestExpressionContainsBoolean, + DoWhileStatement: assertTestExpressionContainsBoolean, + ForStatement: assertTestExpressionContainsBoolean, + IfStatement: assertTestExpressionContainsBoolean, + WhileStatement: assertTestExpressionContainsBoolean, + LogicalExpression: assertLocalExpressionContainsBoolean, + 'UnaryExpression[operator="!"]': assertUnaryExpressionContainsBoolean, }; }, }); 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 3c0c44222834..030bade0f480 100644 --- a/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -153,6 +153,9 @@ ruleTester.run('strict-boolean-expressions', rule, { break; } while ((bool1 && bool2) || (bool1 || bool2)); `, + ` + function foo(arg: T) { return !arg; } + `, ], invalid: [ @@ -888,5 +891,17 @@ ruleTester.run('strict-boolean-expressions', rule, { }, ], }, + { + code: ` + function foo(arg: T) { return !arg; } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 58, + }, + ], + }, ], });