From ade7f0555636b19657385c28a4ddecef68386598 Mon Sep 17 00:00:00 2001 From: Jeff Principe Date: Wed, 12 Jun 2019 21:26:11 -0700 Subject: [PATCH 1/3] feat(eslint-plugin): add new rule no-misused-promises --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-misused-promises.md | 120 ++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + .../src/rules/no-misused-promises.ts | 254 ++++++++++++++++ .../tests/rules/no-misused-promises.test.ts | 282 ++++++++++++++++++ .../experimental-utils/src/ts-eslint/Rule.ts | 1 + 6 files changed, 660 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-misused-promises.md create mode 100644 packages/eslint-plugin/src/rules/no-misused-promises.ts create mode 100644 packages/eslint-plugin/tests/rules/no-misused-promises.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index ff6a11b3726..94c010fbc27 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -151,6 +151,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | | [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md) | Disallows magic numbers | | | | | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | | +| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | | | :thought_balloon: | | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :heavy_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | | [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression | :heavy_check_mark: | | | diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md new file mode 100644 index 00000000000..18eedca2899 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -0,0 +1,120 @@ +# Avoid using promises in places not designed to handle them (no-misused-promises) + +This rule forbids using promises in places where the Typescript compiler +allows them but they are not handled properly. These situations can often arise +due to a missing `await` keyword or just a misunderstanding of the way async +functions are handled/awaited. + +## Rule Details + +Examples of **incorrect** code for this rule with the `"conditional"` check: + +```ts +const promise = Promise.resolve('value'); + +if (promise) { + // Do something +} + +const val = promise ? 123 : 456; + +while (promise) { + // Do something +} +``` + +Examples of **incorrect** code for this rule with the `"void-return"` check: + +```ts +[1, 2, 3].forEach(async value => { + await doSomething(value); +}); + +new Promise(async (resolve, reject) => { + await doSomething(); + resolve(); +}); + +const eventEmitter = new EventEmitter(); +eventEmitter.on('some-event', async () => { + await doSomething(); +}); +``` + +Examples of **correct** code for this rule: + +```ts +const promise = Promise.resolve('value'); + +if (await promise) { + // Do something +} + +const val = (await promise) ? 123 : 456; + +while (await promise) { + // Do something +} + +for (const value of [1, 2, 3]) { + await doSomething(value); +} + +new Promise((resolve, reject) => { + // Do something + resolve(); +}); + +const eventEmitter = new EventEmitter(); +eventEmitter.on('some-event', () => { + doSomething(); +}); +``` + +## Options + +This rule accepts a single option which is an object with a single `checks` +property indicating which types of misuse to flag. As of right now, there are +two checks available ("conditional", "void-return"), which are both enabled by +default. + +If you only want conditionals to be checked, your configuration will look like +this: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checks": ["conditional"] + } + ] +} +``` + +Likewise, if you only want to check for functions that return promises where a +void return is expected, you can configure the rule like this: + +```json +{ + "@typescript-eslint/no-misused-promises": [ + "error", + { + "checks": ["void-return"] + } + ] +} +``` + +## When Not To Use It + +If you do not use Promises in your codebase or are not concerned with possible +misuses of them outside of what the Typescript compiler will check. + +## Related to + +- [`no-floating-promises`]('./no-floating-promises.md') + +## Further Reading + +- [Typescript void function assignability](https://github.com/Microsoft/TypeScript/wiki/FAQ#why-are-functions-returning-non-void-assignable-to-function-returning-void) diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index c92f6a36d8b..b8621086d97 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -25,6 +25,7 @@ import noForInArray from './no-for-in-array'; import noInferrableTypes from './no-inferrable-types'; import noMagicNumbers from './no-magic-numbers'; import noMisusedNew from './no-misused-new'; +import noMisusedPromises from './no-misused-promises'; import noNamespace from './no-namespace'; import noNonNullAssertion from './no-non-null-assertion'; import noObjectLiteralTypeAssertion from './no-object-literal-type-assertion'; @@ -82,6 +83,7 @@ export default { 'no-inferrable-types': noInferrableTypes, 'no-magic-numbers': noMagicNumbers, 'no-misused-new': noMisusedNew, + 'no-misused-promises': noMisusedPromises, 'no-namespace': noNamespace, 'no-non-null-assertion': noNonNullAssertion, 'no-object-literal-type-assertion': noObjectLiteralTypeAssertion, diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts new file mode 100644 index 00000000000..85583386be1 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -0,0 +1,254 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import ts from 'typescript'; + +import * as util from '../util'; + +type Options = [ + { + checks: ('conditional' | 'void-return')[]; + } +]; + +export default util.createRule({ + name: 'no-misused-promises', + meta: { + docs: { + description: 'Avoid using promises in places not designed to handle them', + category: 'Best Practices', + recommended: false, + }, + messages: { + voidReturn: + 'Promise returned in function argument where a void return was expected.', + conditional: 'Expected non-Promise value in a boolean conditional.', + }, + schema: [ + { + type: 'object', + properties: { + checks: { + type: 'array', + items: { + type: 'string', + enum: ['conditional', 'void-return'], + }, + }, + }, + }, + ], + type: 'problem', + }, + defaultOptions: [ + { + checks: ['conditional', 'void-return'], + }, + ], + + create(context, [{ checks }]) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + const conditionalChecks: TSESLint.RuleListener = { + ConditionalExpression(node) { + checkConditional(node.test); + }, + DoWhileStatement(node) { + checkConditional(node.test); + }, + ForStatement(node) { + if (node.test) { + checkConditional(node.test); + } + }, + IfStatement(node) { + checkConditional(node.test); + }, + LogicalExpression(node) { + // We only check the lhs of a logical expression because the rhs might + // be the return value of a short circuit expression. + checkConditional(node.left); + }, + UnaryExpression(node) { + if (node.operator === '!') { + checkConditional(node.argument); + } + }, + WhileStatement(node) { + checkConditional(node.test); + }, + }; + + const voidReturnChecks: TSESLint.RuleListener = { + CallExpression(node) { + checkArguments(node); + }, + NewExpression(node) { + checkArguments(node); + }, + }; + + function checkConditional(node: TSESTree.Expression) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); + if (isAlwaysThenable(checker, tsNode)) { + context.report({ + messageId: 'conditional', + node, + }); + } + } + + function checkArguments( + node: TSESTree.CallExpression | TSESTree.NewExpression, + ) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get< + ts.CallExpression | ts.NewExpression + >(node); + const voidParams = voidFunctionParams(checker, tsNode); + if (voidParams.size === 0) { + return; + } + + for (const [index, argument] of node.arguments.entries()) { + if (!voidParams.has(index)) { + continue; + } + + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(argument); + if (returnsThenable(checker, tsNode as ts.Expression)) { + context.report({ + messageId: 'voidReturn', + node: argument, + }); + } + } + } + + return { + ...(checks.includes('conditional') ? conditionalChecks : {}), + ...(checks.includes('void-return') ? voidReturnChecks : {}), + }; + }, +}); + +// Variation on the thenable check which requires all forms of the type (read: +// alternates in a union) to be thenable. Otherwise, you might be trying to +// check if something is defined or undefined and get caught because one of the +// branches is thenable. +function isAlwaysThenable(checker: ts.TypeChecker, node: ts.Node) { + const type = checker.getTypeAtLocation(node); + + outer: for (const subType of tsutils.unionTypeParts( + checker.getApparentType(type), + )) { + const thenProp = subType.getProperty('then'); + + // If one of the alternates has no then property, it is not thenable in all + // cases. + if (thenProp === undefined) { + return false; + } + + // We walk through each variation of the then property. Since we know it + // exists at this point, we just need at least one of the alternates to + // be of the right form to consider it thenable. + const thenType = checker.getTypeOfSymbolAtLocation(thenProp, node); + for (const subType of tsutils.unionTypeParts(thenType)) { + for (const signature of subType.getCallSignatures()) { + if ( + signature.parameters.length !== 0 && + isFunctionParam(checker, signature.parameters[0], node) + ) { + continue outer; + } + } + } + + // If no flavors of the then property are thenable, we don't consider the + // overall type to be thenable + return false; + } + + // If all variants are considered thenable (i.e. haven't returned false), we + // consider the overall type thenable + return true; +} + +function isFunctionParam( + checker: ts.TypeChecker, + param: ts.Symbol, + node: ts.Node, +): boolean { + const type: ts.Type | undefined = checker.getApparentType( + checker.getTypeOfSymbolAtLocation(param, node), + ); + for (const subType of tsutils.unionTypeParts(type)) { + if (subType.getCallSignatures().length !== 0) { + return true; + } + } + return false; +} + +// Get the positions of parameters which are void functions (and not also +// thenable functions). These are the candidates for the void-return check at +// the current call site. +function voidFunctionParams( + checker: ts.TypeChecker, + node: ts.CallExpression | ts.NewExpression, +) { + const voidReturnIndices = new Set(); + const thenableReturnIndices = new Set(); + const type = checker.getTypeAtLocation(node.expression); + + for (const subType of tsutils.unionTypeParts(type)) { + // Standard function calls and `new` have two different types of signatures + const signatures = ts.isCallExpression(node) + ? subType.getCallSignatures() + : subType.getConstructSignatures(); + for (const signature of signatures) { + for (const [index, parameter] of signature.parameters.entries()) { + const type = checker.getTypeOfSymbolAtLocation( + parameter, + node.expression, + ); + for (const subType of tsutils.unionTypeParts(type)) { + for (const signature of subType.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) { + voidReturnIndices.add(index); + } else if ( + tsutils.isThenableType(checker, node.expression, returnType) + ) { + thenableReturnIndices.add(index); + } + } + } + } + } + } + + // If a certain positional argument accepts both thenable and void returns, + // a promise-returning function is valid + for (const thenable of thenableReturnIndices) { + voidReturnIndices.delete(thenable); + } + + return voidReturnIndices; +} + +// Returns true if the expression is a function that returns a thenable +function returnsThenable(checker: ts.TypeChecker, node: ts.Expression) { + const type = checker.getApparentType(checker.getTypeAtLocation(node)); + + for (const subType of tsutils.unionTypeParts(type)) { + for (const signature of subType.getCallSignatures()) { + const returnType = signature.getReturnType(); + if (tsutils.isThenableType(checker, node, returnType)) { + return true; + } + } + } + + return false; +} diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts new file mode 100644 index 00000000000..5c373375cf7 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -0,0 +1,282 @@ +import rule from '../../src/rules/no-misused-promises'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-misused-promises', rule, { + valid: [ + `if (true) {}`, + { + code: `if (Promise.resolve()) {}`, + options: [{ checks: ['void-return'] }], + }, + ` +if (true) {} +else if (false) {} +else {} +`, + { + code: ` +if (Promise.resolve()) {} +else if (Promise.resolve()) {} +else {} +`, + options: [{ checks: ['void-return'] }], + }, + `for (;;) {}`, + `for (let i; i < 10; i++) {}`, + { + code: `for (let i; Promise.resolve(); i++) {}`, + options: [{ checks: ['void-return'] }], + }, + `do {} while (true);`, + { + code: `do {} while (Promise.resolve())`, + options: [{ checks: ['void-return'] }], + }, + `while (true) {}`, + { + code: `while (Promise.resolve()) {}`, + options: [{ checks: ['void-return'] }], + }, + `true ? 123 : 456`, + { + code: `Promise.resolve() ? 123 : 456`, + options: [{ checks: ['void-return'] }], + }, + `if (!true) {}`, + { + code: `if (!Promise.resolve()) {}`, + options: [{ checks: ['void-return'] }], + }, + `(await Promise.resolve()) || false`, + { + code: `Promise.resolve() || false`, + options: [{ checks: ['void-return'] }], + }, + `(true && await Promise.resolve()) || false`, + { + code: `(true && Promise.resolve()) || false`, + options: [{ checks: ['void-return'] }], + }, + `false || (true && Promise.resolve())`, + ` +async function test() { + if (await Promise.resolve()) {} +} +`, + ` +async function test() { + const mixed: Promise | undefined = Promise.resolve(); + if (mixed) { + await mixed; + } +} +`, + `if (~Promise.resolve()) {}`, + ` +interface NotQuiteThenable { + then(param: string): void; + then(): void; +} +const value: NotQuiteThenable = { then() {} }; +if (value) {} +`, + `[1, 2, 3].forEach(val => {});`, + { + code: `[1, 2, 3].forEach(async val => {});`, + options: [{ checks: ['conditional'] }], + }, + `new Promise((resolve, reject) => resolve());`, + { + code: `new Promise(async (resolve, reject) => resolve());`, + options: [{ checks: ['conditional'] }], + }, + `Promise.all(['abc', 'def'].map(async val => { await val; }))`, + ` +const fn: (arg: () => Promise | void) => void = () => {}; +fn(() => Promise.resolve()); +`, + ], + + invalid: [ + { + code: `if (Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: ` +if (Promise.resolve()) {} +else if (Promise.resolve()) {} +else {} +`, + errors: [ + { + line: 2, + messageId: 'conditional', + }, + { + line: 3, + messageId: 'conditional', + }, + ], + }, + { + code: `for (let i; Promise.resolve(); i++) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `do {} while (Promise.resolve())`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `while (Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `Promise.resolve() ? 123 : 456`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `if (!Promise.resolve()) {}`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `Promise.resolve() || false`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: `(true && Promise.resolve()) || false`, + errors: [ + { + line: 1, + messageId: 'conditional', + }, + ], + }, + { + code: ` +[Promise.resolve(), Promise.reject()].forEach( + async val => { await val; } +); +`, + errors: [ + { + line: 3, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +new Promise(async (resolve, reject) => { + await Promise.resolve(); + resolve(); +}); +`, + errors: [ + { + line: 2, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', async (err, res) => { + await res; +}); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', (err, res) => Promise.resolve(res)); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + { + code: ` +const fnWithCallback = (arg: string, cb: (err: any, res: string) => void) => { + cb(null, arg); +}; + +fnWithCallback('val', (err, res) => { + if (err) { + return 'abc'; + } else { + return Promise.resolve(res); + } +}); +`, + errors: [ + { + line: 6, + messageId: 'voidReturn', + }, + ], + }, + ], +}); diff --git a/packages/experimental-utils/src/ts-eslint/Rule.ts b/packages/experimental-utils/src/ts-eslint/Rule.ts index 388f64e99fc..ad7cbce0651 100644 --- a/packages/experimental-utils/src/ts-eslint/Rule.ts +++ b/packages/experimental-utils/src/ts-eslint/Rule.ts @@ -270,6 +270,7 @@ interface RuleListener { JSXSpreadChild?: RuleFunction; JSXText?: RuleFunction; LabeledStatement?: RuleFunction; + LogicalExpression?: RuleFunction; MemberExpression?: RuleFunction; MetaProperty?: RuleFunction; MethodDefinition?: RuleFunction; From 0eaf244e7c8a9268f5b6aa3f60326e2754ca17d1 Mon Sep 17 00:00:00 2001 From: Jeff Principe Date: Fri, 28 Jun 2019 22:34:13 -0700 Subject: [PATCH 2/3] chore: review feedback --- .../docs/rules/no-misused-promises.md | 23 +++--- .../src/rules/no-misused-promises.ts | 77 +++++++++---------- .../tests/rules/no-misused-promises.test.ts | 22 +++--- 3 files changed, 60 insertions(+), 62 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-misused-promises.md b/packages/eslint-plugin/docs/rules/no-misused-promises.md index 18eedca2899..65e3084ae26 100644 --- a/packages/eslint-plugin/docs/rules/no-misused-promises.md +++ b/packages/eslint-plugin/docs/rules/no-misused-promises.md @@ -7,7 +7,7 @@ functions are handled/awaited. ## Rule Details -Examples of **incorrect** code for this rule with the `"conditional"` check: +Examples of **incorrect** code for this rule with `checksConditionals: true`: ```ts const promise = Promise.resolve('value'); @@ -23,7 +23,7 @@ while (promise) { } ``` -Examples of **incorrect** code for this rule with the `"void-return"` check: +Examples of **incorrect** code for this rule with `checksVoidReturn: true`: ```ts [1, 2, 3].forEach(async value => { @@ -73,34 +73,33 @@ eventEmitter.on('some-event', () => { ## Options -This rule accepts a single option which is an object with a single `checks` -property indicating which types of misuse to flag. As of right now, there are -two checks available ("conditional", "void-return"), which are both enabled by -default. +This rule accepts a single option which is an object with `checksConditionals` +and `checksVoidReturn` properties indicating which types of misuse to flag. +Both are enabled by default -If you only want conditionals to be checked, your configuration will look like -this: +If you don't want functions that return promises where a void return is +expected to be checked, your configuration will look like this: ```json { "@typescript-eslint/no-misused-promises": [ "error", { - "checks": ["conditional"] + "checksVoidReturn": false } ] } ``` -Likewise, if you only want to check for functions that return promises where a -void return is expected, you can configure the rule like this: +Likewise, if you don't want to check conditionals, you can configure the rule +like this: ```json { "@typescript-eslint/no-misused-promises": [ "error", { - "checks": ["void-return"] + "checksConditionals": false } ] } diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts index 85583386be1..7b1181ff179 100644 --- a/packages/eslint-plugin/src/rules/no-misused-promises.ts +++ b/packages/eslint-plugin/src/rules/no-misused-promises.ts @@ -6,7 +6,8 @@ import * as util from '../util'; type Options = [ { - checks: ('conditional' | 'void-return')[]; + checksConditionals?: boolean; + checksVoidReturn?: boolean; } ]; @@ -27,12 +28,11 @@ export default util.createRule({ { type: 'object', properties: { - checks: { - type: 'array', - items: { - type: 'string', - enum: ['conditional', 'void-return'], - }, + checksConditionals: { + type: 'boolean', + }, + checksVoidReturn: { + type: 'boolean', }, }, }, @@ -41,29 +41,20 @@ export default util.createRule({ }, defaultOptions: [ { - checks: ['conditional', 'void-return'], + checksConditionals: true, + checksVoidReturn: true, }, ], - create(context, [{ checks }]) { + create(context, [{ checksConditionals, checksVoidReturn }]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); const conditionalChecks: TSESLint.RuleListener = { - ConditionalExpression(node) { - checkConditional(node.test); - }, - DoWhileStatement(node) { - checkConditional(node.test); - }, - ForStatement(node) { - if (node.test) { - checkConditional(node.test); - } - }, - IfStatement(node) { - checkConditional(node.test); - }, + ConditionalExpression: checkTestConditional, + DoWhileStatement: checkTestConditional, + ForStatement: checkTestConditional, + IfStatement: checkTestConditional, LogicalExpression(node) { // We only check the lhs of a logical expression because the rhs might // be the return value of a short circuit expression. @@ -74,20 +65,20 @@ export default util.createRule({ checkConditional(node.argument); } }, - WhileStatement(node) { - checkConditional(node.test); - }, + WhileStatement: checkTestConditional, }; const voidReturnChecks: TSESLint.RuleListener = { - CallExpression(node) { - checkArguments(node); - }, - NewExpression(node) { - checkArguments(node); - }, + CallExpression: checkArguments, + NewExpression: checkArguments, }; + function checkTestConditional(node: { test: TSESTree.Expression | null }) { + if (node.test) { + checkConditional(node.test); + } + } + function checkConditional(node: TSESTree.Expression) { const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node); if (isAlwaysThenable(checker, tsNode)) { @@ -125,8 +116,8 @@ export default util.createRule({ } return { - ...(checks.includes('conditional') ? conditionalChecks : {}), - ...(checks.includes('void-return') ? voidReturnChecks : {}), + ...(checksConditionals ? conditionalChecks : {}), + ...(checksVoidReturn ? voidReturnChecks : {}), }; }, }); @@ -138,9 +129,7 @@ export default util.createRule({ function isAlwaysThenable(checker: ts.TypeChecker, node: ts.Node) { const type = checker.getTypeAtLocation(node); - outer: for (const subType of tsutils.unionTypeParts( - checker.getApparentType(type), - )) { + for (const subType of tsutils.unionTypeParts(checker.getApparentType(type))) { const thenProp = subType.getProperty('then'); // If one of the alternates has no then property, it is not thenable in all @@ -153,20 +142,30 @@ function isAlwaysThenable(checker: ts.TypeChecker, node: ts.Node) { // exists at this point, we just need at least one of the alternates to // be of the right form to consider it thenable. const thenType = checker.getTypeOfSymbolAtLocation(thenProp, node); + let hasThenableSignature = false; for (const subType of tsutils.unionTypeParts(thenType)) { for (const signature of subType.getCallSignatures()) { if ( signature.parameters.length !== 0 && isFunctionParam(checker, signature.parameters[0], node) ) { - continue outer; + hasThenableSignature = true; + break; } } + + // We only need to find one variant of the then property that has a + // function signature for it to be thenable. + if (hasThenableSignature) { + break; + } } // If no flavors of the then property are thenable, we don't consider the // overall type to be thenable - return false; + if (!hasThenableSignature) { + return false; + } } // If all variants are considered thenable (i.e. haven't returned false), we diff --git a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts index 5c373375cf7..5d4c7fcf4b8 100644 --- a/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-misused-promises.test.ts @@ -17,7 +17,7 @@ ruleTester.run('no-misused-promises', rule, { `if (true) {}`, { code: `if (Promise.resolve()) {}`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, ` if (true) {} @@ -30,43 +30,43 @@ if (Promise.resolve()) {} else if (Promise.resolve()) {} else {} `, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `for (;;) {}`, `for (let i; i < 10; i++) {}`, { code: `for (let i; Promise.resolve(); i++) {}`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `do {} while (true);`, { code: `do {} while (Promise.resolve())`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `while (true) {}`, { code: `while (Promise.resolve()) {}`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `true ? 123 : 456`, { code: `Promise.resolve() ? 123 : 456`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `if (!true) {}`, { code: `if (!Promise.resolve()) {}`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `(await Promise.resolve()) || false`, { code: `Promise.resolve() || false`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `(true && await Promise.resolve()) || false`, { code: `(true && Promise.resolve()) || false`, - options: [{ checks: ['void-return'] }], + options: [{ checksConditionals: false }], }, `false || (true && Promise.resolve())`, ` @@ -94,12 +94,12 @@ if (value) {} `[1, 2, 3].forEach(val => {});`, { code: `[1, 2, 3].forEach(async val => {});`, - options: [{ checks: ['conditional'] }], + options: [{ checksVoidReturn: false }], }, `new Promise((resolve, reject) => resolve());`, { code: `new Promise(async (resolve, reject) => resolve());`, - options: [{ checks: ['conditional'] }], + options: [{ checksVoidReturn: false }], }, `Promise.all(['abc', 'def'].map(async val => { await val; }))`, ` From f321cedcd8be17505932aa894cb5295695e0b28b Mon Sep 17 00:00:00 2001 From: Jeff Principe Date: Mon, 15 Jul 2019 23:09:16 -0700 Subject: [PATCH 3/3] chore: regenerate plugin configs --- packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/configs/base.json | 4 +++- packages/eslint-plugin/src/configs/recommended.json | 2 -- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 5fd043c4c11..10831d4b246 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -36,6 +36,7 @@ "no-magic-numbers": "off", "@typescript-eslint/no-magic-numbers": "error", "@typescript-eslint/no-misused-new": "error", + "@typescript-eslint/no-misused-promises": "error", "@typescript-eslint/no-namespace": "error", "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-object-literal-type-assertion": "error", diff --git a/packages/eslint-plugin/src/configs/base.json b/packages/eslint-plugin/src/configs/base.json index 9b6931ad616..6f56100a6ae 100644 --- a/packages/eslint-plugin/src/configs/base.json +++ b/packages/eslint-plugin/src/configs/base.json @@ -3,5 +3,7 @@ "parserOptions": { "sourceType": "module" }, - "plugins": ["@typescript-eslint"] + "plugins": [ + "@typescript-eslint" + ] } diff --git a/packages/eslint-plugin/src/configs/recommended.json b/packages/eslint-plugin/src/configs/recommended.json index d26fd25d01c..0e6d6d9f665 100644 --- a/packages/eslint-plugin/src/configs/recommended.json +++ b/packages/eslint-plugin/src/configs/recommended.json @@ -24,13 +24,11 @@ "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-object-literal-type-assertion": "error", "@typescript-eslint/no-parameter-properties": "error", - "@typescript-eslint/no-triple-slash-reference": "error", "no-unused-vars": "off", "@typescript-eslint/no-unused-vars": "warn", "no-use-before-define": "off", "@typescript-eslint/no-use-before-define": "error", "@typescript-eslint/no-var-requires": "error", - "@typescript-eslint/prefer-interface": "error", "@typescript-eslint/prefer-namespace-keyword": "error", "@typescript-eslint/type-annotation-spacing": "error" }