From f3b716189ad6b7d8361925d1ecae19e1a053e4a2 Mon Sep 17 00:00:00 2001 From: Jonathan Delgado Date: Sat, 29 Jun 2019 15:21:45 -0700 Subject: [PATCH] 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 8af0163f1f0..5118b46a248 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 3c0c4422283..030bade0f48 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, + }, + ], + }, ], });