From a3a0b328ab02d1880d6115a4dc4990553fab7982 Mon Sep 17 00:00:00 2001 From: Guido <7917821+guidsdo@users.noreply.github.com> Date: Mon, 29 Jul 2019 15:11:51 +0200 Subject: [PATCH] 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 * 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 --- src/configs/all.ts | 1 + .../noPromiseAsBoolean.examples.ts | 72 ++++++++ src/rules/noPromiseAsBooleanRule.ts | 163 ++++++++++++++++++ .../custom-promise/test.ts.lint | 37 ++++ .../custom-promise/tsconfig.json | 6 + .../custom-promise/tslint.json | 5 + .../es6-promise/test.ts.lint | 66 +++++++ .../es6-promise/tsconfig.json | 6 + .../es6-promise/tslint.json | 5 + 9 files changed, 361 insertions(+) create mode 100644 src/rules/code-examples/noPromiseAsBoolean.examples.ts create mode 100644 src/rules/noPromiseAsBooleanRule.ts create mode 100644 test/rules/no-promise-as-boolean/custom-promise/test.ts.lint create mode 100644 test/rules/no-promise-as-boolean/custom-promise/tsconfig.json create mode 100644 test/rules/no-promise-as-boolean/custom-promise/tslint.json create mode 100644 test/rules/no-promise-as-boolean/es6-promise/test.ts.lint create mode 100644 test/rules/no-promise-as-boolean/es6-promise/tsconfig.json create mode 100644 test/rules/no-promise-as-boolean/es6-promise/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index cc6303fc945..4bf818be218 100644 --- a/src/configs/all.ts +++ b/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, diff --git a/src/rules/code-examples/noPromiseAsBoolean.examples.ts b/src/rules/code-examples/noPromiseAsBoolean.examples.ts new file mode 100644 index 00000000000..ed3c8260653 --- /dev/null +++ b/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) { + if (await custumerDecisionPromise) { + console.log("Customer ready to take an order.") + } + } + `, + fail: Lint.Utils.dedent` + async function waiter(custumerDecisionPromise: Promise) { + 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) { + 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) { + console.log(\`\${orderId} contains the following items:\`); + + for (let index = 0; orderedFoodPromise; index++) { + console.log("orderedFoodPromise[index]"); + } + + console.log("Done."); + } + `, + }, +]; diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts new file mode 100644 index 00000000000..e6413b52d82 --- /dev/null +++ b/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, 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) + ); +} diff --git a/test/rules/no-promise-as-boolean/custom-promise/test.ts.lint b/test/rules/no-promise-as-boolean/custom-promise/test.ts.lint new file mode 100644 index 00000000000..57a4b051488 --- /dev/null +++ b/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 {} +const customPromise = new CustomPromise(); + +// 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, promiseOrNull: CustomPromise | Promise | null) { + if (onlyPromise && promiseOrNull) { } + ~~~~~~~~~~~ [0] +} + +[0]: Promises are not allowed as boolean. diff --git a/test/rules/no-promise-as-boolean/custom-promise/tsconfig.json b/test/rules/no-promise-as-boolean/custom-promise/tsconfig.json new file mode 100644 index 00000000000..f8f3eb52d44 --- /dev/null +++ b/test/rules/no-promise-as-boolean/custom-promise/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": true, + "target": "es6" + } +} diff --git a/test/rules/no-promise-as-boolean/custom-promise/tslint.json b/test/rules/no-promise-as-boolean/custom-promise/tslint.json new file mode 100644 index 00000000000..654b35ecb70 --- /dev/null +++ b/test/rules/no-promise-as-boolean/custom-promise/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-promise-as-boolean": [true, { "promise-classes": ["CustomPromise"] }] + } +} diff --git a/test/rules/no-promise-as-boolean/es6-promise/test.ts.lint b/test/rules/no-promise-as-boolean/es6-promise/test.ts.lint new file mode 100644 index 00000000000..ba86acaed03 --- /dev/null +++ b/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 | undefined, promiseOrFalse: Promise | false, promiseOrNull: Promise | null) { + if (promiseOrUndefined || promiseOrFalse || promiseOrNull) { + return; + } +}; + +function funky(maybePromise?: Promise) { + 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) { + if (await custumerDecisionPromise) { + console.log("Customer ready to take an order.") + } +} + +[0]: Promises are not allowed as boolean. diff --git a/test/rules/no-promise-as-boolean/es6-promise/tsconfig.json b/test/rules/no-promise-as-boolean/es6-promise/tsconfig.json new file mode 100644 index 00000000000..f8f3eb52d44 --- /dev/null +++ b/test/rules/no-promise-as-boolean/es6-promise/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "strict": true, + "target": "es6" + } +} diff --git a/test/rules/no-promise-as-boolean/es6-promise/tslint.json b/test/rules/no-promise-as-boolean/es6-promise/tslint.json new file mode 100644 index 00000000000..88591c2c285 --- /dev/null +++ b/test/rules/no-promise-as-boolean/es6-promise/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-promise-as-boolean": true + } +}