From 514bed95fd68d75eecdf3719e52bfac0f3cd8fc2 Mon Sep 17 00:00:00 2001 From: Alexander T Date: Thu, 10 Oct 2019 20:17:41 +0300 Subject: [PATCH] =?UTF-8?q?fix(eslint-plugin):=20[promise-function-async]?= =?UTF-8?q?=20Should=20not=20report=E2=80=A6=20(#1023)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Brad Zacher --- .../docs/rules/promise-function-async.md | 12 +++--- .../src/rules/promise-function-async.ts | 6 ++- packages/eslint-plugin/src/util/types.ts | 11 ++++-- .../rules/promise-function-async.test.ts | 39 +++++++++++++++++++ 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/promise-function-async.md b/packages/eslint-plugin/docs/rules/promise-function-async.md index 5ccddf45ed4..79000974cea 100644 --- a/packages/eslint-plugin/docs/rules/promise-function-async.md +++ b/packages/eslint-plugin/docs/rules/promise-function-async.md @@ -6,7 +6,7 @@ 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. @@ -17,18 +17,18 @@ Examples of **incorrect** code for this rule ```ts const arrowFunctionReturnsPromise = () => Promise.resolve('value'); -function functionDeturnsPromise() { - return Math.random() > 0.5 ? Promise.resolve('value') : false; +function functionReturnsPromise() { + return Promise.resolve('value'); } ``` Examples of **correct** code for this rule ```ts -const arrowFunctionReturnsPromise = async () => 'value'; +const arrowFunctionReturnsPromise = async () => Promise.resolve('value'); -async function functionDeturnsPromise() { - return Math.random() > 0.5 ? 'value' : false; +async function functionReturnsPromise() { + return 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 1575cf02dfc..d907f3d45a5 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -98,7 +98,11 @@ export default util.createRule({ const returnType = checker.getReturnTypeOfSignature(signatures[0]); if ( - !util.containsTypeByName(returnType, allowAny!, allAllowedPromiseNames) + !util.containsAllTypesByName( + returnType, + allowAny!, + allAllowedPromiseNames, + ) ) { return; } diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 6f769d7f950..72213da6ebb 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -8,9 +8,9 @@ import ts from 'typescript'; /** * @param type Type being checked by name. * @param allowedNames Symbol names checking on the type. - * @returns Whether the type is, extends, or contains any of the allowed names. + * @returns Whether the type is, extends, or contains all of the allowed names. */ -export function containsTypeByName( +export function containsAllTypesByName( type: ts.Type, allowAny: boolean, allowedNames: Set, @@ -31,13 +31,16 @@ export function containsTypeByName( } if (isUnionOrIntersectionType(type)) { - return type.types.some(t => containsTypeByName(t, allowAny, allowedNames)); + return type.types.every(t => + containsAllTypesByName(t, allowAny, allowedNames), + ); } const bases = type.getBaseTypes(); return ( typeof bases !== 'undefined' && - bases.some(t => containsTypeByName(t, allowAny, allowedNames)) + bases.length > 0 && + bases.every(t => containsAllTypesByName(t, allowAny, allowedNames)) ); } 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 46100db7982..66a65a21ffd 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -106,6 +106,26 @@ function returnsUnknown(): unknown { }, ], }, + { + code: ` +interface ReadableStream {} +interface Options { + stream: ReadableStream; +} + +type Return = ReadableStream | Promise; +const foo = (options: Options): Return => { + return options.stream ? asStream(options) : asPromise(options); +} + `, + }, + { + code: ` +function foo(): Promise | boolean { + return Math.random() > 0.5 ? Promise.resolve('value') : false; +} + `, + }, ], invalid: [ { @@ -379,5 +399,24 @@ const returnAllowedType = () => new PromiseType(); }, ], }, + { + code: ` +interface SPromise extends Promise {} +function foo(): Promise | SPromise { + return Math.random() > 0.5 ? Promise.resolve('value') : Promise.resolve(false); +} + `, + options: [ + { + allowedPromiseNames: ['SPromise'], + }, + ], + errors: [ + { + line: 3, + messageId, + }, + ], + }, ], });