Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): add rule strict-boolean-expressions #579

Merged
merged 7 commits into from Jul 1, 2019
Merged
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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 | | | |
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/ROADMAP.md
Expand Up @@ -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] |
Expand Down
57 changes: 57 additions & 0 deletions 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)
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
101 changes: 101 additions & 0 deletions 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';
jonathanrdelgado marked this conversation as resolved.
Show resolved Hide resolved

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<ts.ExpressionStatement>(
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);
}
}
j-f1 marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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,
};
},
});