diff --git a/packages/eslint-plugin/docs/rules/promise-function-async.md b/packages/eslint-plugin/docs/rules/promise-function-async.md index e697217dc5d..f3095ed1808 100644 --- a/packages/eslint-plugin/docs/rules/promise-function-async.md +++ b/packages/eslint-plugin/docs/rules/promise-function-async.md @@ -11,10 +11,12 @@ Ensures that each function is only capable of: - returning a rejected promise, or - throwing an Error object. -In contrast, non-`async` `Promise` - returning functions are technically capable of either. +In contrast, non-`async`, `Promise`-returning functions are technically capable of either. Code that handles the results of those functions will often need to handle both cases, which can get complex. This rule's practice removes a requirement for creating code to handle both cases. +> When functions return unions of `Promise` and non-`Promise` types implicitly, it is usually a mistake—this rule flags those cases. If it is intentional, make the return type explicitly to allow the rule to pass. + ## Examples Examples of code for this rule @@ -29,6 +31,10 @@ const arrowFunctionReturnsPromise = () => Promise.resolve('value'); function functionReturnsPromise() { return Promise.resolve('value'); } + +function functionReturnsUnionWithPromiseImplicitly(p: boolean) { + return p ? 'value' : Promise.resolve('value'); +} ``` ### ✅ Correct @@ -39,4 +45,15 @@ const arrowFunctionReturnsPromise = async () => Promise.resolve('value'); async function functionReturnsPromise() { return Promise.resolve('value'); } + +// An explicit return type that is not Promise means this function cannot be made async, so it is ignored by the rule +function functionReturnsUnionWithPromiseExplicitly( + p: boolean, +): string | Promise { + return p ? 'value' : Promise.resolve('value'); +} + +async function functionReturnsUnionWithPromiseImplicitly(p: boolean) { + return p ? 'value' : Promise.resolve('value'); +} ``` diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index 4387bc52c9b..34fbc7ee672 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -115,6 +115,8 @@ export default util.createRule({ returnType, allowAny!, allAllowedPromiseNames, + // If no return type is explicitly set, we check if any parts of the return type match a Promise (instead of requiring all to match). + node.returnType == null, ) ) { // Return type is not a promise diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index ccf99b6afcb..089e6ba617b 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -165,6 +165,23 @@ abstract class Test { } `, }, + ` +function promiseInUnionWithExplicitReturnType( + p: boolean, +): Promise | number { + return p ? Promise.resolve(5) : 5; +} + `, + ` +function explicitReturnWithPromiseInUnion(): Promise | number { + return 5; +} + `, + ` +async function asyncFunctionReturningUnion(p: boolean) { + return p ? Promise.resolve(5) : 5; +} + `, ], invalid: [ { @@ -752,5 +769,22 @@ const foo = { }, ], }, + { + code: ` +function promiseInUnionWithoutExplicitReturnType(p: boolean) { + return p ? Promise.resolve(5) : 5; +} + `, + errors: [ + { + messageId, + }, + ], + output: ` +async function promiseInUnionWithoutExplicitReturnType(p: boolean) { + return p ? Promise.resolve(5) : 5; +} + `, + }, ], }); diff --git a/packages/type-utils/src/containsAllTypesByName.ts b/packages/type-utils/src/containsAllTypesByName.ts index 07aa20dac04..9c3fb522236 100644 --- a/packages/type-utils/src/containsAllTypesByName.ts +++ b/packages/type-utils/src/containsAllTypesByName.ts @@ -5,13 +5,16 @@ import { isTypeFlagSet } from './typeFlagUtils'; /** * @param type Type being checked by name. + * @param allowAny Whether to consider `any` and `unknown` to match. * @param allowedNames Symbol names checking on the type. - * @returns Whether the type is, extends, or contains all of the allowed names. + * @param matchAnyInstead Whether to instead just check if any parts match, rather than all parts. + * @returns Whether the type is, extends, or contains the allowed names (or all matches the allowed names, if mustMatchAll is true). */ export function containsAllTypesByName( type: ts.Type, allowAny: boolean, allowedNames: Set, + matchAnyInstead = false, ): boolean { if (isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) { return !allowAny; @@ -26,16 +29,21 @@ export function containsAllTypesByName( return true; } + const predicate = (t: ts.Type): boolean => + containsAllTypesByName(t, allowAny, allowedNames, matchAnyInstead); + if (isUnionOrIntersectionType(type)) { - return type.types.every(t => - containsAllTypesByName(t, allowAny, allowedNames), - ); + return matchAnyInstead + ? type.types.some(predicate) + : type.types.every(predicate); } const bases = type.getBaseTypes(); + return ( typeof bases !== 'undefined' && - bases.length > 0 && - bases.every(t => containsAllTypesByName(t, allowAny, allowedNames)) + (matchAnyInstead + ? bases.some(predicate) + : bases.length > 0 && bases.every(predicate)) ); }