Skip to content

Commit

Permalink
fix(eslint-plugin): [promise-function-async] Should not report… (#1023)
Browse files Browse the repository at this point in the history
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
  • Loading branch information
a-tarasyuk and bradzacher committed Oct 10, 2019
1 parent a3f84e1 commit 514bed9
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 11 deletions.
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,
},
],
},
],
});

0 comments on commit 514bed9

Please sign in to comment.