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.

Sign up for GitHub

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

Jump to bottom

feat(eslint-plugin): [no-floating-promises] flag result of .map(async) #7897

Merged
merged 17 commits into from Dec 29, 2023
Merged

feat(eslint-plugin): [no-floating-promises] flag result of .map(async) #7897

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
c37f343
flag arrays of promises
kirkwaiblinger Nov 11, 2023
0244ccd
remove comment
kirkwaiblinger Nov 11, 2023
a69fe00
trying to trigger CI
kirkwaiblinger Nov 12, 2023
8fd0773
Substantial simplification
kirkwaiblinger Nov 13, 2023
ea23d54
add some grody test cases for generics
kirkwaiblinger Nov 13, 2023
a486da2
flip if/else
kirkwaiblinger Nov 13, 2023
8ee0d64
add detail to test case
kirkwaiblinger Nov 13, 2023
2afe794
switch to isArrayLikeType
kirkwaiblinger Nov 13, 2023
6335820
Revert "switch to isArrayLikeType"
kirkwaiblinger Nov 13, 2023
852990e
add checks for tuples
kirkwaiblinger Nov 13, 2023
8c9eadc
One more test case!
kirkwaiblinger Nov 13, 2023
6498556
else if
kirkwaiblinger Nov 19, 2023
8c1f526
Remove unnecessary whitespace
kirkwaiblinger Dec 6, 2023
7817e8a
Remove invalid test case
kirkwaiblinger Dec 6, 2023
81ced8a
Add to docs
kirkwaiblinger Dec 6, 2023
d14de70
Add test cases around array that's fine but tuple that isn't
kirkwaiblinger Dec 6, 2023
428b976
tweaks
kirkwaiblinger Dec 7, 2023
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
182 changes: 116 additions & 66 deletions packages/eslint-plugin/src/rules/no-floating-promises.ts
View file
Open in desktop
kirkwaiblinger marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -23,7 +23,9 @@ type MessageId =
| 'floatingUselessRejectionHandler'
| 'floatingUselessRejectionHandlerVoid'
| 'floatingFixAwait'
| 'floatingFixVoid';
| 'floatingFixVoid'
| 'floatingPromiseArray'
| 'floatingPromiseArrayVoid';

const messageBase =
'Promises must be awaited, end with a call to .catch, or end with a call to .then with a rejection handler.';
Expand All @@ -35,6 +37,13 @@ const messageBaseVoid =
const messageRejectionHandler =
'A rejection handler that is not a function will be ignored.';

const messagePromiseArray =
"An array of Promises may be unintentional. Consider handling the promises' fulfillment or rejection with Promise.all or similar.";
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

const messagePromiseArrayVoid =
"An array of Promises may be unintentional. Consider handling the promises' fulfillment or rejection with Promise.all or similar," +
' or explicitly marking the expression as ignored with the `void` operator.';

export default createRule<Options, MessageId>({
name: 'no-floating-promises',
meta: {
Expand All @@ -54,6 +63,9 @@ export default createRule<Options, MessageId>({
messageBase + ' ' + messageRejectionHandler,
floatingUselessRejectionHandlerVoid:
messageBaseVoid + ' ' + messageRejectionHandler,

kirkwaiblinger marked this conversation as resolved.
Show resolved Hide resolved
floatingPromiseArray: messagePromiseArray,
floatingPromiseArrayVoid: messagePromiseArrayVoid,
},
schema: [
{
Expand Down Expand Up @@ -97,75 +109,82 @@ export default createRule<Options, MessageId>({
expression = expression.expression;
}

const { isUnhandled, nonFunctionHandler } = isUnhandledPromise(
checker,
expression,
);
const { isUnhandled, nonFunctionHandler, promiseArray } =
isUnhandledPromise(checker, expression);

if (isUnhandled) {
if (options.ignoreVoid) {
if (promiseArray) {
context.report({
node,
messageId: nonFunctionHandler
? 'floatingUselessRejectionHandlerVoid'
: 'floatingVoid',
suggest: [
{
messageId: 'floatingFixVoid',
fix(fixer): TSESLint.RuleFix | TSESLint.RuleFix[] {
const tsNode = services.esTreeNodeToTSNodeMap.get(
node.expression,
);
if (isHigherPrecedenceThanUnary(tsNode)) {
return fixer.insertTextBefore(node, 'void ');
}
return [
fixer.insertTextBefore(node, 'void ('),
fixer.insertTextAfterRange(
[expression.range[1], expression.range[1]],
')',
),
];
},
},
],
messageId: options.ignoreVoid
? 'floatingPromiseArrayVoid'
: 'floatingPromiseArray',
});
} else {
context.report({
node,
messageId: nonFunctionHandler
? 'floatingUselessRejectionHandler'
: 'floating',
suggest: [
{
messageId: 'floatingFixAwait',
fix(fixer): TSESLint.RuleFix | TSESLint.RuleFix[] {
if (
expression.type === AST_NODE_TYPES.UnaryExpression &&
expression.operator === 'void'
) {
return fixer.replaceTextRange(
[expression.range[0], expression.range[0] + 4],
'await',
if (options.ignoreVoid) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
context.report({
node,
messageId: nonFunctionHandler
? 'floatingUselessRejectionHandlerVoid'
: 'floatingVoid',
suggest: [
{
messageId: 'floatingFixVoid',
fix(fixer): TSESLint.RuleFix | TSESLint.RuleFix[] {
const tsNode = services.esTreeNodeToTSNodeMap.get(
node.expression,
);
}
const tsNode = services.esTreeNodeToTSNodeMap.get(
node.expression,
);
if (isHigherPrecedenceThanUnary(tsNode)) {
return fixer.insertTextBefore(node, 'await ');
}
return [
fixer.insertTextBefore(node, 'await ('),
fixer.insertTextAfterRange(
[expression.range[1], expression.range[1]],
')',
),
];
if (isHigherPrecedenceThanUnary(tsNode)) {
return fixer.insertTextBefore(node, 'void ');
}
return [
fixer.insertTextBefore(node, 'void ('),
fixer.insertTextAfterRange(
[expression.range[1], expression.range[1]],
')',
),
];
},
},
},
],
});
],
});
} else {
context.report({
node,
messageId: nonFunctionHandler
? 'floatingUselessRejectionHandler'
: 'floating',
suggest: [
{
messageId: 'floatingFixAwait',
fix(fixer): TSESLint.RuleFix | TSESLint.RuleFix[] {
if (
expression.type === AST_NODE_TYPES.UnaryExpression &&
expression.operator === 'void'
) {
return fixer.replaceTextRange(
[expression.range[0], expression.range[0] + 4],
'await',
);
}
const tsNode = services.esTreeNodeToTSNodeMap.get(
node.expression,
);
if (isHigherPrecedenceThanUnary(tsNode)) {
return fixer.insertTextBefore(node, 'await ');
}
return [
fixer.insertTextBefore(node, 'await ('),
fixer.insertTextAfterRange(
[expression.range[1], expression.range[1]],
')',
),
];
},
},
],
});
}
}
}
},
Expand Down Expand Up @@ -206,7 +225,11 @@ export default createRule<Options, MessageId>({
function isUnhandledPromise(
checker: ts.TypeChecker,
node: TSESTree.Node,
): { isUnhandled: boolean; nonFunctionHandler?: boolean } {
): {
isUnhandled: boolean;
nonFunctionHandler?: boolean;
promiseArray?: boolean;
} {
// First, check expressions whose resulting types may not be promise-like
if (node.type === AST_NODE_TYPES.SequenceExpression) {
// Any child in a comma expression could return a potentially unhandled
Expand All @@ -229,8 +252,16 @@ export default createRule<Options, MessageId>({
return isUnhandledPromise(checker, node.argument);
}

const tsNode = services.esTreeNodeToTSNodeMap.get(node);

// Check the type. At this point it can't be unhandled if it isn't a promise
if (!isPromiseLike(checker, services.esTreeNodeToTSNodeMap.get(node))) {
// or array thereof.

if (isPromiseArray(checker, tsNode)) {
return { isUnhandled: true, promiseArray: true };
}

if (!isPromiseLike(checker, tsNode)) {
return { isUnhandled: false };
}

Expand Down Expand Up @@ -296,12 +327,31 @@ export default createRule<Options, MessageId>({
},
});

function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean {
const type = checker.getTypeAtLocation(node);
for (const ty of tsutils
.unionTypeParts(type)
.map(t => checker.getApparentType(t))) {
if (checker.isArrayType(ty)) {
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
const arrayType = checker.getTypeArguments(ty)[0];
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
if (isPromiseLike(checker, node, arrayType)) {
return true;
}
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
}
}
return false;
}

// Modified from tsutils.isThenable() to only consider thenables which can be
// rejected/caught via a second parameter. Original source (MIT licensed):
//
// https://github.com/ajafff/tsutils/blob/49d0d31050b44b81e918eae4fbaf1dfe7b7286af/util/type.ts#L95-L125
function isPromiseLike(checker: ts.TypeChecker, node: ts.Node): boolean {
const type = checker.getTypeAtLocation(node);
function isPromiseLike(
checker: ts.TypeChecker,
node: ts.Node,
type?: ts.Type,
): boolean {
type ??= checker.getTypeAtLocation(node);
for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) {
const then = ty.getProperty('then');
if (then === undefined) {
Expand Down