Skip to content

Commit 514bed9

Browse files
a-tarasyukbradzacher
andcommittedOct 10, 2019
fix(eslint-plugin): [promise-function-async] Should not report… (#1023)
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
1 parent a3f84e1 commit 514bed9

File tree

4 files changed

+57
-11
lines changed

4 files changed

+57
-11
lines changed
 

‎packages/eslint-plugin/docs/rules/promise-function-async.md

+6-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Ensures that each function is only capable of:
66
- returning a rejected promise, or
77
- throwing an Error object.
88

9-
In contrast, non-`async` `Promise`-returning functions are technically capable of either.
9+
In contrast, non-`async` `Promise` - returning functions are technically capable of either.
1010
Code that handles the results of those functions will often need to handle both cases, which can get complex.
1111
This rule's practice removes a requirement for creating code to handle both cases.
1212

@@ -17,18 +17,18 @@ Examples of **incorrect** code for this rule
1717
```ts
1818
const arrowFunctionReturnsPromise = () => Promise.resolve('value');
1919

20-
function functionDeturnsPromise() {
21-
return Math.random() > 0.5 ? Promise.resolve('value') : false;
20+
function functionReturnsPromise() {
21+
return Promise.resolve('value');
2222
}
2323
```
2424

2525
Examples of **correct** code for this rule
2626

2727
```ts
28-
const arrowFunctionReturnsPromise = async () => 'value';
28+
const arrowFunctionReturnsPromise = async () => Promise.resolve('value');
2929

30-
async function functionDeturnsPromise() {
31-
return Math.random() > 0.5 ? 'value' : false;
30+
async function functionReturnsPromise() {
31+
return Promise.resolve('value');
3232
}
3333
```
3434

‎packages/eslint-plugin/src/rules/promise-function-async.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,11 @@ export default util.createRule<Options, MessageIds>({
9898
const returnType = checker.getReturnTypeOfSignature(signatures[0]);
9999

100100
if (
101-
!util.containsTypeByName(returnType, allowAny!, allAllowedPromiseNames)
101+
!util.containsAllTypesByName(
102+
returnType,
103+
allowAny!,
104+
allAllowedPromiseNames,
105+
)
102106
) {
103107
return;
104108
}

‎packages/eslint-plugin/src/util/types.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ import ts from 'typescript';
88
/**
99
* @param type Type being checked by name.
1010
* @param allowedNames Symbol names checking on the type.
11-
* @returns Whether the type is, extends, or contains any of the allowed names.
11+
* @returns Whether the type is, extends, or contains all of the allowed names.
1212
*/
13-
export function containsTypeByName(
13+
export function containsAllTypesByName(
1414
type: ts.Type,
1515
allowAny: boolean,
1616
allowedNames: Set<string>,
@@ -31,13 +31,16 @@ export function containsTypeByName(
3131
}
3232

3333
if (isUnionOrIntersectionType(type)) {
34-
return type.types.some(t => containsTypeByName(t, allowAny, allowedNames));
34+
return type.types.every(t =>
35+
containsAllTypesByName(t, allowAny, allowedNames),
36+
);
3537
}
3638

3739
const bases = type.getBaseTypes();
3840
return (
3941
typeof bases !== 'undefined' &&
40-
bases.some(t => containsTypeByName(t, allowAny, allowedNames))
42+
bases.length > 0 &&
43+
bases.every(t => containsAllTypesByName(t, allowAny, allowedNames))
4144
);
4245
}
4346

‎packages/eslint-plugin/tests/rules/promise-function-async.test.ts

+39
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,26 @@ function returnsUnknown(): unknown {
106106
},
107107
],
108108
},
109+
{
110+
code: `
111+
interface ReadableStream {}
112+
interface Options {
113+
stream: ReadableStream;
114+
}
115+
116+
type Return = ReadableStream | Promise<void>;
117+
const foo = (options: Options): Return => {
118+
return options.stream ? asStream(options) : asPromise(options);
119+
}
120+
`,
121+
},
122+
{
123+
code: `
124+
function foo(): Promise<string> | boolean {
125+
return Math.random() > 0.5 ? Promise.resolve('value') : false;
126+
}
127+
`,
128+
},
109129
],
110130
invalid: [
111131
{
@@ -379,5 +399,24 @@ const returnAllowedType = () => new PromiseType();
379399
},
380400
],
381401
},
402+
{
403+
code: `
404+
interface SPromise<T> extends Promise<T> {}
405+
function foo(): Promise<string> | SPromise<boolean> {
406+
return Math.random() > 0.5 ? Promise.resolve('value') : Promise.resolve(false);
407+
}
408+
`,
409+
options: [
410+
{
411+
allowedPromiseNames: ['SPromise'],
412+
},
413+
],
414+
errors: [
415+
{
416+
line: 3,
417+
messageId,
418+
},
419+
],
420+
},
382421
],
383422
});

0 commit comments

Comments
 (0)
Please sign in to comment.