Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add rule 'no-promise-as-boolean' #4790

Merged
merged 14 commits into from Jul 29, 2019
1 change: 1 addition & 0 deletions src/configs/all.ts
Expand Up @@ -129,6 +129,7 @@ export const rules = {
"no-null-keyword": true,
"no-null-undefined-union": true,
"no-object-literal-type-assertion": true,
"no-promise-as-boolean": true,
"no-return-await": true,
"no-shadowed-variable": true,
"no-string-literal": true,
Expand Down
128 changes: 128 additions & 0 deletions src/rules/noPromiseAsBooleanRule.ts
@@ -0,0 +1,128 @@
/**
* @license
* Copyright 2019 Palantir Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {
isBinaryExpression,
isConditionalExpression,
isDoStatement,
isForStatement,
isIfStatement,
isPrefixUnaryExpression,
isWhileStatement,
} from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";

export class Rule extends Lint.Rules.TypedRule {
/* tslint:disable:object-literal-sort-keys */
public static metadata: Lint.IRuleMetadata = {
ruleName: "no-promise-as-boolean",
description: "Warns for Promises that are used for boolean conditions.",
optionsDescription: Lint.Utils.dedent`
A list of 'string' names of any additional classes that should also be treated as Promises.
For example, if you are using a class called 'Future' that implements the Thenable interface,
you might tell the rule to consider type references with the name 'Future' as valid Promise-like
types. Note that this rule doesn't check for type assignability or compatibility; it just checks
type reference names.
`,
options: {
type: "list",
listType: {
type: "array",
items: { type: "string" },
},
},
optionExamples: [true, [true, "Thenable"]],
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
rationale: Lint.Utils.dedent`
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
There are no situations where one would like to check if a variable's value is truthy if it's type
guidsdo marked this conversation as resolved.
Show resolved Hide resolved
only is Promise.
This may only occur when the typings are incorrect or the variable has a union type
(like Promise | undefined), of which the latter is allowed.

This rule prevents common bugs, where the user most of the times wants to 'await' a Promise first.
guidsdo marked this conversation as resolved.
Show resolved Hide resolved
`,
type: "functionality",
typescriptOnly: true,
requiresTypeInfo: true,
};
/* tslint:enable:object-literal-sort-keys */

public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
return this.applyWithFunction(
sourceFile,
walk,
["Promise", ...(this.ruleArguments as string[])],
program.getTypeChecker(),
);
}
}

const RULE_MESSAGE = "Promises are not allowed as boolean.";

function walk(context: Lint.WalkContext<string[]>, checker: ts.TypeChecker): void {
const { sourceFile } = context;
return ts.forEachChild(sourceFile, cb);

function cb(node: ts.Node): void {
if (isBooleanBinaryExpression(node)) {
const { left, right } = node;
if (!isBooleanBinaryExpression(left)) {
checkExpression(left);
}

if (!isBooleanBinaryExpression(right)) {
checkExpression(right);
}
} else if (isPrefixUnaryExpression(node)) {
const { operator, operand } = node;
if (operator === ts.SyntaxKind.ExclamationToken) {
checkExpression(operand);
}
} else if (isIfStatement(node) || isWhileStatement(node) || isDoStatement(node)) {
// If it's a boolean binary expression, we'll check it when recursing.
if (!isBooleanBinaryExpression(node.expression)) {
checkExpression(node.expression);
}
} else if (isConditionalExpression(node)) {
checkExpression(node.condition);
} else if (isForStatement(node)) {
const { condition } = node;
if (condition !== undefined) {
checkExpression(condition);
}
}

return ts.forEachChild(node, cb);
}

function checkExpression(expression: ts.Expression): void {
const { symbol } = checker.getTypeAtLocation(expression);
if (symbol !== undefined && context.options.indexOf(symbol.name) !== -1) {
context.addFailureAtNode(expression, RULE_MESSAGE);
}
}
}

/** Matches `&&` and `||` operators. */
function isBooleanBinaryExpression(expression: ts.Node): expression is ts.BinaryExpression {
return (
isBinaryExpression(expression) &&
(expression.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken ||
expression.operatorToken.kind === ts.SyntaxKind.BarBarToken)
);
}
54 changes: 54 additions & 0 deletions test/rules/no-promise-as-boolean/test.ts.lint
@@ -0,0 +1,54 @@
[typescript]: >= 2.4.0

async function promiseFunc() {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

normalExpression = promiseFunc();
const stringLiteral = "text" + promiseFunc();

const globalUnaryExpression = !!promiseFunc();
~~~~~~~~~~~~~ [0]

const globalBinaryExpression = "text" && promiseFunc();
~~~~~~~~~~~~~ [0]

promiseFunc() && console.log("topLevelBinaryExpression");
~~~~~~~~~~~~~ [0]

function funky(maybePromise?: Promise<number>) {
while (promiseFunc() && maybePromise) {
~~~~~~~~~~~~~ [0]

const binaryExpression = (promiseFunc() && "1") || ("1" && !promiseFunc()) ? ("1" && promiseFunc() ? "1" : "1") : maybePromise;
~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~ [0]

// For-loop
for (let index = 0; promiseFunc(); index++) {
~~~~~~~~~~~~~ [0]

// a non-promise
if ("just some text" + promiseFunc()) {

// Promise literal
} else if (new Promise(() => { })) {
~~~~~~~~~~~~~~~~~~~~~~ [0]

// Nested Promise
if (promiseFunc()) {
~~~~~~~~~~~~~ [0]
}

// Parenthesized Expression + Exclamation Tokens
} else if (promiseFunc() && !!promiseFunc()) {
~~~~~~~~~~~~~ [0]
~~~~~~~~~~~~~ [0]
}
}
}
}

[0]: Promises are not allowed as boolean.
8 changes: 8 additions & 0 deletions test/rules/no-promise-as-boolean/tsconfig.json
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"module": "commonjs",
"sourceMap": true,
"strict": true,
"target": "es6"
}
}
5 changes: 5 additions & 0 deletions test/rules/no-promise-as-boolean/tslint.json
@@ -0,0 +1,5 @@
{
"rules": {
"no-promise-as-boolean": true
}
}