Skip to content

Commit

Permalink
fix(eslint-plugin): [no-misused-promises] factor thenable returning f…
Browse files Browse the repository at this point in the history
…unction overload signatures (#4620)

* fix(eslint-plugin): [no-misused-promises] factor thenable returning function overload signatures

* chore: mention TypeScript issue

* chore: dedup and clean and fix comments

* test: add a few more test cases
  • Loading branch information
JoshuaKGoldberg committed Mar 2, 2022
1 parent 699ef48 commit 56a09e9
Show file tree
Hide file tree
Showing 2 changed files with 176 additions and 18 deletions.
84 changes: 66 additions & 18 deletions packages/eslint-plugin/src/rules/no-misused-promises.ts
Expand Up @@ -394,9 +394,13 @@ function voidFunctionParams(
checker: ts.TypeChecker,
node: ts.CallExpression | ts.NewExpression,
): Set<number> {
const thenableReturnIndices = new Set<number>();
const voidReturnIndices = new Set<number>();
const type = checker.getTypeAtLocation(node.expression);

// We can't use checker.getResolvedSignature because it prefers an early '() => void' over a later '() => Promise<void>'
// See https://github.com/microsoft/TypeScript/issues/48077

for (const subType of tsutils.unionTypeParts(type)) {
// Standard function calls and `new` have two different types of signatures
const signatures = ts.isCallExpression(node)
Expand All @@ -408,50 +412,94 @@ function voidFunctionParams(
parameter,
node.expression,
);
if (isVoidReturningFunctionType(checker, node.expression, type)) {
if (isThenableReturningFunctionType(checker, node.expression, type)) {
thenableReturnIndices.add(index);
} else if (
!thenableReturnIndices.has(index) &&
isVoidReturningFunctionType(checker, node.expression, type)
) {
voidReturnIndices.add(index);
}
}
}
}

for (const index of thenableReturnIndices) {
voidReturnIndices.delete(index);
}

return voidReturnIndices;
}

// Returns true if given type is a void-returning function.
/**
* @returns Whether any call signature of the type has a thenable return type.
*/
function anySignatureIsThenableType(
checker: ts.TypeChecker,
node: ts.Node,
type: ts.Type,
): boolean {
for (const signature of type.getCallSignatures()) {
const returnType = signature.getReturnType();
if (tsutils.isThenableType(checker, node, returnType)) {
return true;
}
}

return false;
}

/**
* @returns Whether type is a thenable-returning function.
*/
function isThenableReturningFunctionType(
checker: ts.TypeChecker,
node: ts.Node,
type: ts.Type,
): boolean {
for (const subType of tsutils.unionTypeParts(type)) {
if (anySignatureIsThenableType(checker, node, subType)) {
return true;
}
}

return false;
}

/**
* @returns Whether type is a void-returning function.
*/
function isVoidReturningFunctionType(
checker: ts.TypeChecker,
node: ts.Node,
type: ts.Type,
): boolean {
let hasVoidReturningFunction = false;
let hasThenableReturningFunction = false;
for (const subType of tsutils.unionTypeParts(type)) {
for (const signature of subType.getCallSignatures()) {
const returnType = signature.getReturnType();

// If a certain positional argument accepts both thenable and void returns,
// a promise-returning function is valid
if (tsutils.isThenableType(checker, node, returnType)) {
return false;
}

if (tsutils.isTypeFlagSet(returnType, ts.TypeFlags.Void)) {
hasVoidReturningFunction = true;
} else if (tsutils.isThenableType(checker, node, returnType)) {
hasThenableReturningFunction = true;
return true;
}
}
}
// If a certain positional argument accepts both thenable and void returns,
// a promise-returning function is valid
return hasVoidReturningFunction && !hasThenableReturningFunction;
return false;
}

// Returns true if the expression is a function that returns a thenable
/**
* @returns Whether expression is a function that returns a thenable.
*/
function returnsThenable(checker: ts.TypeChecker, node: ts.Node): boolean {
const type = checker.getApparentType(checker.getTypeAtLocation(node));

for (const subType of tsutils.unionTypeParts(type)) {
for (const signature of subType.getCallSignatures()) {
const returnType = signature.getReturnType();
if (tsutils.isThenableType(checker, node, returnType)) {
return true;
}
}
if (anySignatureIsThenableType(checker, node, type)) {
return true;
}

return false;
Expand Down
110 changes: 110 additions & 0 deletions packages/eslint-plugin/tests/rules/no-misused-promises.test.ts
Expand Up @@ -252,6 +252,58 @@ const Component: any = () => null;
`,
filename: 'react.tsx',
},
{
code: `
interface ItLike {
(name: string, callback: () => Promise<void>): void;
(name: string, callback: () => void): void;
}
declare const it: ItLike;
it('', async () => {});
`,
},
{
code: `
interface ItLike {
(name: string, callback: () => void): void;
(name: string, callback: () => Promise<void>): void;
}
declare const it: ItLike;
it('', async () => {});
`,
},
{
code: `
interface ItLike {
(name: string, callback: () => void): void;
}
interface ItLike {
(name: string, callback: () => Promise<void>): void;
}
declare const it: ItLike;
it('', async () => {});
`,
},
{
code: `
interface ItLike {
(name: string, callback: () => Promise<void>): void;
}
interface ItLike {
(name: string, callback: () => void): void;
}
declare const it: ItLike;
it('', async () => {});
`,
},
],

invalid: [
Expand Down Expand Up @@ -687,5 +739,63 @@ const Component = (obj: O) => null;
},
],
},
{
code: `
interface ItLike {
(name: string, callback: () => number): void;
(name: string, callback: () => void): void;
}
declare const it: ItLike;
it('', async () => {});
`,
errors: [
{
line: 9,
messageId: 'voidReturnArgument',
},
],
},
{
code: `
interface ItLike {
(name: string, callback: () => number): void;
}
interface ItLike {
(name: string, callback: () => void): void;
}
declare const it: ItLike;
it('', async () => {});
`,
errors: [
{
line: 11,
messageId: 'voidReturnArgument',
},
],
},
{
code: `
interface ItLike {
(name: string, callback: () => void): void;
}
interface ItLike {
(name: string, callback: () => number): void;
}
declare const it: ItLike;
it('', async () => {});
`,
errors: [
{
line: 11,
messageId: 'voidReturnArgument',
},
],
},
],
});

0 comments on commit 56a09e9

Please sign in to comment.