Skip to content
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

feat(eslint-plugin): Check 'rest' parameters in no-misused-promises #5731

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
92 changes: 79 additions & 13 deletions packages/eslint-plugin/src/rules/no-misused-promises.ts
Expand Up @@ -213,13 +213,13 @@ export default util.createRule<Options, MessageId>({
node: TSESTree.CallExpression | TSESTree.NewExpression,
): void {
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
const voidParams = voidFunctionParams(checker, tsNode);
if (voidParams.size === 0) {
const voidArgs = voidFunctionArguments(checker, tsNode);
if (voidArgs.size === 0) {
return;
}

for (const [index, argument] of node.arguments.entries()) {
if (!voidParams.has(index)) {
if (!voidArgs.has(index)) {
continue;
}

Expand Down Expand Up @@ -486,13 +486,40 @@ function isFunctionParam(
return false;
}

// Get the positions of parameters which are void functions (and not also
function checkThenableOrVoidArgument(
checker: ts.TypeChecker,
node: ts.CallExpression | ts.NewExpression,
type: ts.Type,
index: number,
thenableReturnIndices: Set<number>,
voidReturnIndices: Set<number>,
): void {
if (isThenableReturningFunctionType(checker, node.expression, type)) {
thenableReturnIndices.add(index);
} else if (isVoidReturningFunctionType(checker, node.expression, type)) {
// If a certain argument accepts both thenable and void returns,
// a promise-returning function is valid
if (!thenableReturnIndices.has(index)) {
voidReturnIndices.add(index);
}
}
}

// Get the positions of arguments which are void functions (and not also
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
// thenable functions). These are the candidates for the void-return check at
// the current call site.
function voidFunctionParams(
// If the function parameters end with a 'rest' parameter, then we consider
// the array type parameter (e.g. '...args:Array<SomeType>') when determining
// if trailing arguments are candidates.
function voidFunctionArguments(
checker: ts.TypeChecker,
node: ts.CallExpression | ts.NewExpression,
): Set<number> {
// 'new' can be used without any arguments, as in 'let b = new Object;'
// In this case, there are no argument positions to check, so return early.
if (!node.arguments) {
return new Set<number>();
}
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
const thenableReturnIndices = new Set<number>();
const voidReturnIndices = new Set<number>();
const type = checker.getTypeAtLocation(node.expression);
Expand All @@ -507,17 +534,56 @@ function voidFunctionParams(
: subType.getConstructSignatures();
for (const signature of signatures) {
for (const [index, parameter] of signature.parameters.entries()) {
const type = checker.getTypeOfSymbolAtLocation(
const decl = parameter.valueDeclaration;
let type = checker.getTypeOfSymbolAtLocation(
parameter,
node.expression,
);
if (isThenableReturningFunctionType(checker, node.expression, type)) {
thenableReturnIndices.add(index);
} else if (
!thenableReturnIndices.has(index) &&
isVoidReturningFunctionType(checker, node.expression, type)
) {
voidReturnIndices.add(index);

// If this is a array 'rest' parameter, check all of the argument indices
// from the current argument to the end.
// Note - we currently do not support 'spread' arguments - adding support for them
// is tracked in https://github.com/typescript-eslint/typescript-eslint/issues/5744
if (decl && ts.isParameter(decl) && decl.dotDotDotToken) {
if (checker.isArrayType(type)) {
// Unwrap 'Array<MaybeVoidFunction>' to 'MaybeVoidFunction',
// so that we'll handle it in the same way as a non-rest
// 'param: MaybeVoidFunction'
type = checker.getTypeArguments(type)[0];
for (let i = index; i < node.arguments.length; i++) {
checkThenableOrVoidArgument(
checker,
node,
type,
i,
thenableReturnIndices,
voidReturnIndices,
);
}
} else if (checker.isTupleType(type)) {
// Check each type in the tuple - for example, [boolean, () => void] would
// add the index of the second tuple parameter to 'voidReturnIndices'
const typeArgs = checker.getTypeArguments(type);
for (let i = index; i < node.arguments.length; i++) {
checkThenableOrVoidArgument(
checker,
node,
typeArgs[i - index],
i,
thenableReturnIndices,
voidReturnIndices,
);
}
}
} else {
checkThenableOrVoidArgument(
checker,
node,
type,
index,
thenableReturnIndices,
voidReturnIndices,
);
}
}
}
Expand Down
129 changes: 129 additions & 0 deletions packages/eslint-plugin/tests/rules/no-misused-promises.test.ts
Expand Up @@ -373,6 +373,52 @@ console.log([...Promise.resolve(42)]);
`,
options: [{ checksSpreads: false }],
},
`
function spreadAny(..._args: any): void {}

spreadAny(
true,
() => Promise.resolve(1),
() => Promise.resolve(false),
);
`,
`
function spreadArrayAny(..._args: Array<any>): void {}

spreadArrayAny(
true,
() => Promise.resolve(1),
() => Promise.resolve(false),
);
`,
`
function spreadArrayUnknown(..._args: Array<unknown>): void {}

spreadArrayUnknown(() => Promise.resolve(true), 1, 2);

function spreadArrayFuncPromise(
..._args: Array<() => Promise<undefined>>
): void {}

spreadArrayFuncPromise(
() => Promise.resolve(undefined),
() => Promise.resolve(undefined),
);
`,
// Prettier adds a () but this tests arguments being undefined, not []
// eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting
`
class TakeCallbacks {
constructor(...callbacks: Array<() => void>) {}
}

new TakeCallbacks;
new TakeCallbacks();
new TakeCallbacks(
() => 1,
() => true,
);
`,
],

invalid: [
Expand Down Expand Up @@ -970,5 +1016,88 @@ console.log({ ...(condition ? Promise.resolve({ key: 42 }) : {}) });
{ line: 7, messageId: 'spread' },
],
},
{
code: `
function restPromises(first: Boolean, ...callbacks: Array<() => void>): void {}

restPromises(
true,
() => Promise.resolve(true),
() => Promise.resolve(null),
() => true,
() => Promise.resolve('Hello'),
);
`,
errors: [
{ line: 6, messageId: 'voidReturnArgument' },
{ line: 7, messageId: 'voidReturnArgument' },
{ line: 9, messageId: 'voidReturnArgument' },
],
},
{
code: `
type MyUnion = (() => void) | boolean;

function restUnion(first: string, ...callbacks: Array<MyUnion>): void {}
restUnion('Testing', false, () => Promise.resolve(true));
`,
errors: [{ line: 5, messageId: 'voidReturnArgument' }],
},
{
code: `
function restTupleOne(first: string, ...callbacks: [() => void]): void {}
restTupleOne('My string', () => Promise.resolve(1));
`,
errors: [{ line: 3, messageId: 'voidReturnArgument' }],
},
{
code: `
function restTupleTwo(
first: boolean,
...callbacks: [undefined, () => void, undefined]
): void {}

restTupleTwo(true, undefined, () => Promise.resolve(true), undefined);
`,
errors: [{ line: 7, messageId: 'voidReturnArgument' }],
},
{
code: `
function restTupleFour(
first: number,
...callbacks: [() => void, boolean, () => void, () => void]
): void;

restTupleFour(
1,
() => Promise.resolve(true),
false,
() => {},
() => Promise.resolve(1),
);
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
`,
errors: [
{ line: 9, messageId: 'voidReturnArgument' },
{ line: 12, messageId: 'voidReturnArgument' },
],
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
},
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
{
// Prettier adds a () but this tests arguments being undefined, not []
// eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting
code: `
class TakesVoidCb {
constructor(first: string, ...args: Array<() => void>);
}

new TakesVoidCb;
new TakesVoidCb();
new TakesVoidCb(
'Testing',
() => {},
() => Promise.resolve(true),
);
`,
errors: [{ line: 11, messageId: 'voidReturnArgument' }],
},
],
});