Skip to content

fix(eslint-plugin): [promise-function-async] Should not report for returned union types containing non-promise #1023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/eslint-plugin/docs/rules/promise-function-async.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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');
}
```

Expand Down
6 changes: 5 additions & 1 deletion packages/eslint-plugin/src/rules/promise-function-async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ export default util.createRule<Options, MessageIds>({
const returnType = checker.getReturnTypeOfSignature(signatures[0]);

if (
!util.containsTypeByName(returnType, allowAny!, allAllowedPromiseNames)
!util.containsAllTypesByName(
returnType,
allowAny!,
allAllowedPromiseNames,
)
) {
return;
}
Expand Down
11 changes: 7 additions & 4 deletions packages/eslint-plugin/src/util/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>,
Expand All @@ -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))
);
}

Expand Down
39 changes: 39 additions & 0 deletions packages/eslint-plugin/tests/rules/promise-function-async.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,26 @@ function returnsUnknown(): unknown {
},
],
},
{
code: `
interface ReadableStream {}
interface Options {
stream: ReadableStream;
}

type Return = ReadableStream | Promise<void>;
const foo = (options: Options): Return => {
return options.stream ? asStream(options) : asPromise(options);
}
`,
},
{
code: `
function foo(): Promise<string> | boolean {
return Math.random() > 0.5 ? Promise.resolve('value') : false;
}
`,
},
],
invalid: [
{
Expand Down Expand Up @@ -379,5 +399,24 @@ const returnAllowedType = () => new PromiseType();
},
],
},
{
code: `
interface SPromise<T> extends Promise<T> {}
function foo(): Promise<string> | SPromise<boolean> {
return Math.random() > 0.5 ? Promise.resolve('value') : Promise.resolve(false);
}
`,
options: [
{
allowedPromiseNames: ['SPromise'],
},
],
errors: [
{
line: 3,
messageId,
},
],
},
],
});