diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index a6e9a3c60a2..875f89bbebe 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -178,6 +178,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 8a527cd3313..60cdbe7de24 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -92,7 +92,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 00000000000..699af9baa43 --- /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 5d6fc99aa48..f93cb788cf7 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -53,6 +53,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'; @@ -113,6 +114,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 00000000000..5118b46a248 --- /dev/null +++ b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts @@ -0,0 +1,101 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import ts from 'typescript'; +import * as tsutils from 'tsutils'; +import * as util 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. + */ + function isBooleanType(node: TSESTree.Node): boolean { + const tsNode = service.esTreeNodeToTSNodeMap.get( + node, + ); + const type = util.getConstrainedTypeAtLocation(checker, tsNode); + return tsutils.isTypeFlagSet(type, ts.TypeFlags.BooleanLike); + } + + /** + * Asserts that a testable expression contains a boolean, reports otherwise. + * Filters all LogicalExpressions to prevent some duplicate reports. + */ + function assertTestExpressionContainsBoolean( + node: ExpressionWithTest, + ): void { + if ( + node.test !== null && + node.test.type !== AST_NODE_TYPES.LogicalExpression && + !isBooleanType(node.test) + ) { + reportNode(node.test); + } + } + + /** + * Asserts that a logical expression contains a boolean, reports otherwise. + */ + 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: 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 new file mode 100644 index 00000000000..030bade0f48 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/strict-boolean-expressions.test.ts @@ -0,0 +1,907 @@ +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; + `, + ` + let a = 0; + let u1 = typeof a; + let u2 = -a; + let u3 = ~a; + `, + ` + 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)); + `, + ` + function foo(arg: T) { return !arg; } + `, + ], + + 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, + }, + ], + }, + { + code: ` + function foo(arg: T) { return !arg; } + `, + errors: [ + { + messageId: 'strictBooleanExpression', + line: 2, + column: 58, + }, + ], + }, + ], +});