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

* Add rule 'no-promise-as-boolean'

This commit fixes #3983

* Review: fix grammar in rule rationale

Co-Authored-By: Josh Goldberg <[email protected]>

* Review: add extra test cases for union types

* Review: use options object for no-promise-ad-boolean rule

* Move es6 promise tests to separate folder

* Review: add tests for third party promises

* Add support for union typed promises in rule

* Add extra test that awaits the promise

* Review: add example for new noPromiseAsBoolean rule

* Review: recommend other related rule

* Fix build by using tsutils

* Fix tslint issues

* Review: fix typo

Co-Authored-By: Josh Goldberg <[email protected]>

src/configs/all.ts

@@ -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,

src/rules/code-examples/noPromiseAsBoolean.examples.ts

@@ -0,0 +1,72 @@
+/**
+ * @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 * as Lint from "../../index";
+
+// tslint:disable: object-literal-sort-keys
+export const codeExamples = [
+    {
+        description: "Disallows usages of a non-awaited Promise as boolean.",
+        config: Lint.Utils.dedent`
+            "rules": { "no-promise-as-boolean": true }
+        `,
+        pass: Lint.Utils.dedent`
+            async function waiter(custumerDecisionPromise: Promise<any>) {
+                if (await custumerDecisionPromise) {
+                    console.log("Customer ready to take an order.")
+                }
+            }
+        `,
+        fail: Lint.Utils.dedent`
+            async function waiter(custumerDecisionPromise: Promise<any>) {
+                if (custumerDecisionPromise) {
+                    console.log("Customer ready to take an order.")
+                }
+            }
+        `,
+    },
+    {
+        description: "Disallows usages of a non-awaited third-party promise as boolean.",
+        config: Lint.Utils.dedent`
+            "rules": { "no-promise-as-boolean": [true, { "promise-classes": ["CustomPromise"] }] }
+        `,
+        pass: Lint.Utils.dedent`
+            function printOrdersPerLine(orderId: number, orderedFoodPromise: CustomPromise<string[]>) {
+                orderedFoodPromise.then(orderedFood => {
+                    console.log(\`\${orderId} contains the following items:\`);
+
+                    for (let index = 0; orderedFood; index++) {
+                        console.log("orderedFood[index]");
+                    }
+
+                    console.log("Done.");
+                })
+            }
+        `,
+        fail: Lint.Utils.dedent`
+            function printOrdersPerLine(orderId: number, orderedFoodPromise: CustomPromise<string[]>) {
+                console.log(\`\${orderId} contains the following items:\`);
+
+                for (let index = 0; orderedFoodPromise; index++) {
+                    console.log("orderedFoodPromise[index]");
+                }
+
+                console.log("Done.");
+            }
+        `,
+    },
+];

src/rules/noPromiseAsBooleanRule.ts

@@ -0,0 +1,163 @@
+/**
+ * @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,
+    isUnionType,
+    isWhileStatement,
+} from "tsutils";
+import * as ts from "typescript";
+
+import * as Lint from "../index";
+
+import { codeExamples } from "./code-examples/noPromiseAsBoolean.examples";
+
+const OPTION_PROMISE_CLASSES = "promise-classes";
+
+interface Options {
+    promiseClasses: string[];
+}
+
+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.",
+        descriptionDetails: Lint.Utils.dedent`
+            For the most accurate findings, set \`"strict": true\` in your \`tsconfig.json\`.
+
+            It's recommended to enable the following rules as well:
+            * [\`strict-boolean-expressions\`](https://palantir.github.io/tslint/rules/strict-boolean-expressions/)
+            * [\`strict-type-predicates\`](https://palantir.github.io/tslint/rules/strict-type-predicates/)
+            * [\`no-floating-promises\`](https://palantir.github.io/tslint/rules/no-floating-promises/)
+        `,
+        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: "object",
+            properties: {
+                [OPTION_PROMISE_CLASSES]: {
+                    type: "array",
+                    items: { type: "string" },
+                },
+            },
+        },
+        optionExamples: [true, [true, { OPTION_PROMISE_CLASSES: ["Thenable"] }]],
+        rationale: Lint.Utils.dedent`
+            There are no situations where one would like to check whether a variable's value is truthy if its type
+            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 from forgetting to 'await' a Promise.
+        `,
+        type: "functionality",
+        typescriptOnly: true,
+        requiresTypeInfo: true,
+        codeExamples,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
+        // tslint:disable-next-line: no-object-literal-type-assertion
+        const rawOptions = { ...this.ruleArguments[0] } as { [OPTION_PROMISE_CLASSES]?: string[] };
+        const promiseClasses =
+            rawOptions[OPTION_PROMISE_CLASSES] !== undefined
+                ? rawOptions[OPTION_PROMISE_CLASSES]!
+                : [];
+
+        return this.applyWithFunction(
+            sourceFile,
+            walk,
+            { promiseClasses: ["Promise", ...promiseClasses] },
+            program.getTypeChecker(),
+        );
+    }
+}
+
+const RULE_MESSAGE = "Promises are not allowed as boolean.";
+
+function walk(context: Lint.WalkContext<Options>, 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 mainType = checker.getTypeAtLocation(expression);
+        if (
+            isPromiseType(mainType) ||
+            (isUnionType(mainType) && mainType.types.every(isPromiseType))
+        ) {
+            context.addFailureAtNode(expression, RULE_MESSAGE);
+        }
+    }
+
+    function isPromiseType(type: ts.Type) {
+        const promiseClasses = context.options.promiseClasses;
+        return type.symbol !== undefined && promiseClasses.indexOf(type.symbol.name) !== -1;
+    }
+}
+
+/** 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)
+    );
+}

test/rules/no-promise-as-boolean/custom-promise/test.ts.lint

@@ -0,0 +1,37 @@
+[typescript]: >= 2.4.0
+
+async function promiseFunc() {
+    return false;
+}
+
+class CustomPromise<T = any> {}
+const customPromise = new CustomPromise<any>();
+
+// Custom promise
+if (customPromise) {}
+    ~~~~~~~~~~~~~     [0]
+
+normalExpression = customPromise;
+const stringLiteral = "text" + customPromise;
+
+const globalUnaryExpression = !!customPromise;
+                                ~~~~~~~~~~~~~  [0]
+
+const globalBinaryExpression = "text" && customPromise;
+                                         ~~~~~~~~~~~~~  [0]
+
+customPromise && console.log("topLevelBinaryExpression");
+~~~~~~~~~~~~~                        [0]
+
+function union(promiseOrUndefined: CustomPromise | undefined, promiseOrFalse: CustomPromise | false, promiseOrNull: CustomPromise | null) {
+    if (promiseOrUndefined || promiseOrFalse || promiseOrNull) {
+        return;
+    }
+}
+
+function normalUrCustomPromise(onlyPromise: CustomPromise | Promise<any>, promiseOrNull: CustomPromise | Promise<any> | null) {
+    if (onlyPromise && promiseOrNull) { }
+        ~~~~~~~~~~~                       [0]
+}
+
+[0]: Promises are not allowed as boolean.

test/rules/no-promise-as-boolean/custom-promise/tsconfig.json

@@ -0,0 +1,6 @@
+{
+    "compilerOptions": {
+        "strict": true,
+        "target": "es6"
+    }
+}

test/rules/no-promise-as-boolean/custom-promise/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-promise-as-boolean": [true, { "promise-classes": ["CustomPromise"] }]
+  }
+}

test/rules/no-promise-as-boolean/es6-promise/test.ts.lint

@@ -0,0 +1,66 @@
+[typescript]: >= 2.4.0
+
+async function promiseFunc() {
+    return false;
+}
+
+normalExpression = promiseFunc();
+const stringLiteral = "text" + promiseFunc();
+
+const globalUnaryExpression = !!promiseFunc();
+                                ~~~~~~~~~~~~~  [0]
+
+const globalBinaryExpression = "text" && promiseFunc();
+                                         ~~~~~~~~~~~~~  [0]
+
+promiseFunc() && console.log("topLevelBinaryExpression");
+~~~~~~~~~~~~~                        [0]
+
+function union(promiseOrUndefined: Promise<any> | undefined, promiseOrFalse: Promise<any> | false, promiseOrNull: Promise<any> | null) {
+    if (promiseOrUndefined || promiseOrFalse || promiseOrNull) {
+        return;
+    }
+};
+
+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]
+            }
+        }
+    }
+}
+
+async function waiter(custumerDecisionPromise: Promise<any>) {
+    if (await custumerDecisionPromise) {
+        console.log("Customer ready to take an order.")
+    }
+}
+
+[0]: Promises are not allowed as boolean.

test/rules/no-promise-as-boolean/es6-promise/tsconfig.json

@@ -0,0 +1,6 @@
+{
+    "compilerOptions": {
+        "strict": true,
+        "target": "es6"
+    }
+}

test/rules/no-promise-as-boolean/es6-promise/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-promise-as-boolean": true
+  }
+}