From 65a9c882d9a1892a2e32d42f9373a27b93ebb857 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 10 Jul 2019 15:08:55 +0200 Subject: [PATCH 01/13] Add rule 'no-promise-as-boolean' This commit fixes #3983 --- src/configs/all.ts | 1 + src/rules/noPromiseAsBooleanRule.ts | 128 ++++++++++++++++++ test/rules/no-promise-as-boolean/test.ts.lint | 54 ++++++++ .../rules/no-promise-as-boolean/tsconfig.json | 8 ++ test/rules/no-promise-as-boolean/tslint.json | 5 + 5 files changed, 196 insertions(+) create mode 100644 src/rules/noPromiseAsBooleanRule.ts create mode 100644 test/rules/no-promise-as-boolean/test.ts.lint create mode 100644 test/rules/no-promise-as-boolean/tsconfig.json create mode 100644 test/rules/no-promise-as-boolean/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/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts new file mode 100644 index 00000000000..d754baccc53 --- /dev/null +++ b/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"]], + rationale: Lint.Utils.dedent` + There are no situations where one would like to check if a variable's value is truthy if it's 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, where the user most of the times wants to 'await' a Promise first. + `, + 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, 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) + ); +} diff --git a/test/rules/no-promise-as-boolean/test.ts.lint b/test/rules/no-promise-as-boolean/test.ts.lint new file mode 100644 index 00000000000..e8d922d1e19 --- /dev/null +++ b/test/rules/no-promise-as-boolean/test.ts.lint @@ -0,0 +1,54 @@ +[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 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] + } + } + } +} + +[0]: Promises are not allowed as boolean. diff --git a/test/rules/no-promise-as-boolean/tsconfig.json b/test/rules/no-promise-as-boolean/tsconfig.json new file mode 100644 index 00000000000..6013b5fadca --- /dev/null +++ b/test/rules/no-promise-as-boolean/tsconfig.json @@ -0,0 +1,8 @@ +{ + "compilerOptions": { + "module": "commonjs", + "sourceMap": true, + "strict": true, + "target": "es6" + } + } diff --git a/test/rules/no-promise-as-boolean/tslint.json b/test/rules/no-promise-as-boolean/tslint.json new file mode 100644 index 00000000000..88591c2c285 --- /dev/null +++ b/test/rules/no-promise-as-boolean/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-promise-as-boolean": true + } +} From 92eb5b7ce847321122b8e6d05187af20aa42d0f1 Mon Sep 17 00:00:00 2001 From: Guido <7917821+guidsdo@users.noreply.github.com> Date: Wed, 17 Jul 2019 11:25:35 +0200 Subject: [PATCH 02/13] Review: fix grammar in rule rationale Co-Authored-By: Josh Goldberg --- src/rules/noPromiseAsBooleanRule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index d754baccc53..04bbc572ad6 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -49,12 +49,12 @@ export class Rule extends Lint.Rules.TypedRule { }, optionExamples: [true, [true, "Thenable"]], rationale: Lint.Utils.dedent` - There are no situations where one would like to check if a variable's value is truthy if it's type + 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, where the user most of the times wants to 'await' a Promise first. + This rule prevents common bugs from forgetting to 'await' a Promise. `, type: "functionality", typescriptOnly: true, From a156004bbee580ddccd82dac81e24eeedfa568a7 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 11:34:21 +0200 Subject: [PATCH 03/13] Review: add extra test cases for union types --- test/rules/no-promise-as-boolean/test.ts.lint | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/rules/no-promise-as-boolean/test.ts.lint b/test/rules/no-promise-as-boolean/test.ts.lint index e8d922d1e19..3e84aa07fff 100644 --- a/test/rules/no-promise-as-boolean/test.ts.lint +++ b/test/rules/no-promise-as-boolean/test.ts.lint @@ -16,6 +16,12 @@ const globalBinaryExpression = "text" && promiseFunc(); 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] From 67ed2fb58b74f99c8aa7d49c97b426922bf52180 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 12:14:46 +0200 Subject: [PATCH 04/13] Review: use options object for no-promise-ad-boolean rule --- src/rules/noPromiseAsBooleanRule.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index 04bbc572ad6..55a99cc8353 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -28,6 +28,12 @@ import * as ts from "typescript"; import * as Lint from "../index"; +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 = { @@ -41,13 +47,15 @@ export class Rule extends Lint.Rules.TypedRule { type reference names. `, options: { - type: "list", - listType: { - type: "array", - items: { type: "string" }, + type: "object", + properties: { + [OPTION_PROMISE_CLASSES]: { + type: "array", + items: { type: "string" }, + }, }, }, - optionExamples: [true, [true, "Thenable"]], + 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. @@ -63,10 +71,11 @@ export class Rule extends Lint.Rules.TypedRule { /* tslint:enable:object-literal-sort-keys */ public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] { + const rawOptions = { ...this.ruleArguments[0] } as { [OPTION_PROMISE_CLASSES]?: string[] }; return this.applyWithFunction( sourceFile, walk, - ["Promise", ...(this.ruleArguments as string[])], + { promiseClasses: ["Promise", ...(rawOptions[OPTION_PROMISE_CLASSES] || [])] }, program.getTypeChecker(), ); } @@ -74,7 +83,7 @@ export class Rule extends Lint.Rules.TypedRule { const RULE_MESSAGE = "Promises are not allowed as boolean."; -function walk(context: Lint.WalkContext, checker: ts.TypeChecker): void { +function walk(context: Lint.WalkContext, checker: ts.TypeChecker): void { const { sourceFile } = context; return ts.forEachChild(sourceFile, cb); @@ -112,7 +121,7 @@ function walk(context: Lint.WalkContext, checker: ts.TypeChecker): voi function checkExpression(expression: ts.Expression): void { const { symbol } = checker.getTypeAtLocation(expression); - if (symbol !== undefined && context.options.indexOf(symbol.name) !== -1) { + if (symbol !== undefined && context.options.promiseClasses.indexOf(symbol.name) !== -1) { context.addFailureAtNode(expression, RULE_MESSAGE); } } From a75b9f420704395dcba416c0c0f3fdd20af70d06 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 13:08:21 +0200 Subject: [PATCH 05/13] Move es6 promise tests to separate folder --- .../no-promise-as-boolean/{ => es6-promise}/test.ts.lint | 0 .../rules/no-promise-as-boolean/es6-promise/tsconfig.json | 6 ++++++ .../no-promise-as-boolean/{ => es6-promise}/tslint.json | 0 test/rules/no-promise-as-boolean/tsconfig.json | 8 -------- 4 files changed, 6 insertions(+), 8 deletions(-) rename test/rules/no-promise-as-boolean/{ => es6-promise}/test.ts.lint (100%) create mode 100644 test/rules/no-promise-as-boolean/es6-promise/tsconfig.json rename test/rules/no-promise-as-boolean/{ => es6-promise}/tslint.json (100%) delete mode 100644 test/rules/no-promise-as-boolean/tsconfig.json diff --git a/test/rules/no-promise-as-boolean/test.ts.lint b/test/rules/no-promise-as-boolean/es6-promise/test.ts.lint similarity index 100% rename from test/rules/no-promise-as-boolean/test.ts.lint rename to test/rules/no-promise-as-boolean/es6-promise/test.ts.lint 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/tslint.json b/test/rules/no-promise-as-boolean/es6-promise/tslint.json similarity index 100% rename from test/rules/no-promise-as-boolean/tslint.json rename to test/rules/no-promise-as-boolean/es6-promise/tslint.json diff --git a/test/rules/no-promise-as-boolean/tsconfig.json b/test/rules/no-promise-as-boolean/tsconfig.json deleted file mode 100644 index 6013b5fadca..00000000000 --- a/test/rules/no-promise-as-boolean/tsconfig.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "compilerOptions": { - "module": "commonjs", - "sourceMap": true, - "strict": true, - "target": "es6" - } - } From 66d0a20577d8c1fee18f25e0f58afa5465cf5101 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 13:19:26 +0200 Subject: [PATCH 06/13] Review: add tests for third party promises --- .../custom-promise/test.ts.lint | 32 +++++++++++++++++++ .../custom-promise/tsconfig.json | 6 ++++ .../custom-promise/tslint.json | 5 +++ 3 files changed, 43 insertions(+) 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 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..6657df0ed7b --- /dev/null +++ b/test/rules/no-promise-as-boolean/custom-promise/test.ts.lint @@ -0,0 +1,32 @@ +[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; + } +} + +[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"] }] + } +} From 631d1750ef43e8e1a4ead676a75e998acb621702 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 13:20:17 +0200 Subject: [PATCH 07/13] Add support for union typed promises in rule --- src/rules/noPromiseAsBooleanRule.ts | 13 +++++++++---- .../custom-promise/test.ts.lint | 5 +++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index 55a99cc8353..14abc606217 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -120,10 +120,15 @@ function walk(context: Lint.WalkContext, checker: ts.TypeChecker): void } function checkExpression(expression: ts.Expression): void { - const { symbol } = checker.getTypeAtLocation(expression); - if (symbol !== undefined && context.options.promiseClasses.indexOf(symbol.name) !== -1) { - context.addFailureAtNode(expression, RULE_MESSAGE); - } + const mainType = checker.getTypeAtLocation(expression); + if (isPromiseType(mainType) || (mainType.isUnion() && 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; } } 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 index 6657df0ed7b..57a4b051488 100644 --- a/test/rules/no-promise-as-boolean/custom-promise/test.ts.lint +++ b/test/rules/no-promise-as-boolean/custom-promise/test.ts.lint @@ -29,4 +29,9 @@ function union(promiseOrUndefined: CustomPromise | undefined, promiseOrFalse: Cu } } +function normalUrCustomPromise(onlyPromise: CustomPromise | Promise, promiseOrNull: CustomPromise | Promise | null) { + if (onlyPromise && promiseOrNull) { } + ~~~~~~~~~~~ [0] +} + [0]: Promises are not allowed as boolean. From e3ed70ad5e0bfcd1d30a43196ccfa30e2543ed7e Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 13:47:52 +0200 Subject: [PATCH 08/13] Add extra test that awaits the promise --- test/rules/no-promise-as-boolean/es6-promise/test.ts.lint | 6 ++++++ 1 file changed, 6 insertions(+) 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 index 3e84aa07fff..ba86acaed03 100644 --- a/test/rules/no-promise-as-boolean/es6-promise/test.ts.lint +++ b/test/rules/no-promise-as-boolean/es6-promise/test.ts.lint @@ -57,4 +57,10 @@ function funky(maybePromise?: Promise) { } } +async function waiter(custumerDecisionPromise: Promise) { + if (await custumerDecisionPromise) { + console.log("Customer ready to take an order.") + } +} + [0]: Promises are not allowed as boolean. From 463788957c744d84316de6a517e466837e9a3bac Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 13:49:38 +0200 Subject: [PATCH 09/13] Review: add example for new noPromiseAsBoolean rule --- .../noPromiseAsBoolean.examples.ts | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 src/rules/code-examples/noPromiseAsBoolean.examples.ts 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."); + } + `, + }, +]; From ea275ea34c7d20721dc8c679a3181540543778d1 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 15:59:20 +0200 Subject: [PATCH 10/13] Review: recommend other related rule --- src/rules/noPromiseAsBooleanRule.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index 14abc606217..33c632974dd 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -28,6 +28,8 @@ 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 { @@ -39,6 +41,14 @@ export class Rule extends Lint.Rules.TypedRule { 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 to following tslint 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, @@ -67,6 +77,8 @@ export class Rule extends Lint.Rules.TypedRule { type: "functionality", typescriptOnly: true, requiresTypeInfo: true, + codeExamples, + }; /* tslint:enable:object-literal-sort-keys */ From ec2107ffaf015c6b2a30820ce925d6acb21408c1 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 16:00:43 +0200 Subject: [PATCH 11/13] Fix build by using tsutils --- src/rules/noPromiseAsBooleanRule.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index 33c632974dd..1940f538360 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -22,6 +22,7 @@ import { isForStatement, isIfStatement, isPrefixUnaryExpression, + isUnionType, isWhileStatement, } from "tsutils"; import * as ts from "typescript"; @@ -133,7 +134,7 @@ function walk(context: Lint.WalkContext, checker: ts.TypeChecker): void function checkExpression(expression: ts.Expression): void { const mainType = checker.getTypeAtLocation(expression); - if (isPromiseType(mainType) || (mainType.isUnion() && mainType.types.every(isPromiseType))) { + if (isPromiseType(mainType) || (isUnionType(mainType) && mainType.types.every(isPromiseType))) { context.addFailureAtNode(expression, RULE_MESSAGE); } } From 354aa5c6ab4e5c00730b22f5e1c20cd6f479d1f2 Mon Sep 17 00:00:00 2001 From: guidojo Date: Wed, 17 Jul 2019 16:03:28 +0200 Subject: [PATCH 12/13] Fix tslint issues --- src/rules/noPromiseAsBooleanRule.ts | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index 1940f538360..f62838be479 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -79,16 +79,21 @@ export class Rule extends Lint.Rules.TypedRule { 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", ...(rawOptions[OPTION_PROMISE_CLASSES] || [])] }, + { promiseClasses: ["Promise", ...promiseClasses] }, program.getTypeChecker(), ); } @@ -134,9 +139,12 @@ function walk(context: Lint.WalkContext, checker: ts.TypeChecker): void 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); - } + if ( + isPromiseType(mainType) || + (isUnionType(mainType) && mainType.types.every(isPromiseType)) + ) { + context.addFailureAtNode(expression, RULE_MESSAGE); + } } function isPromiseType(type: ts.Type) { From 26983a36d2b1e31e7fe2a9bcdfee4828bb9a7cf1 Mon Sep 17 00:00:00 2001 From: Guido <7917821+guidsdo@users.noreply.github.com> Date: Sat, 27 Jul 2019 17:45:28 +0200 Subject: [PATCH 13/13] Review: fix typo Co-Authored-By: Josh Goldberg --- src/rules/noPromiseAsBooleanRule.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/noPromiseAsBooleanRule.ts b/src/rules/noPromiseAsBooleanRule.ts index f62838be479..e6413b52d82 100644 --- a/src/rules/noPromiseAsBooleanRule.ts +++ b/src/rules/noPromiseAsBooleanRule.ts @@ -45,7 +45,7 @@ export class Rule extends Lint.Rules.TypedRule { descriptionDetails: Lint.Utils.dedent` For the most accurate findings, set \`"strict": true\` in your \`tsconfig.json\`. - It's recommended to enable to following tslint rules as well: + 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/)