From c37f34365e1f4017257cd9c318bdfe1403647cbd Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sat, 11 Nov 2023 01:34:53 -0700 Subject: [PATCH 01/17] flag arrays of promises --- .../src/rules/no-floating-promises.ts | 225 +++++++++++++----- .../tests/rules/no-floating-promises.test.ts | 77 +++++- 2 files changed, 233 insertions(+), 69 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 066469c3507d..23650946ea15 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -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.'; @@ -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."; + +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({ name: 'no-floating-promises', meta: { @@ -54,6 +63,9 @@ export default createRule({ messageBase + ' ' + messageRejectionHandler, floatingUselessRejectionHandlerVoid: messageBaseVoid + ' ' + messageRejectionHandler, + + floatingPromiseArray: messagePromiseArray, + floatingPromiseArrayVoid: messagePromiseArrayVoid, }, schema: [ { @@ -97,74 +109,81 @@ export default createRule({ expression = expression.expression; } - const { isUnhandled, nonFunctionHandler } = isUnhandledPromise( - checker, - expression, - ); + const { isUnhandled, nonFunctionHandler, promiseArray } = + isUnhandledPromise(checker, expression); if (isUnhandled) { - if (options.ignoreVoid) { - 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]], - ')', - ), - ]; + if (!promiseArray) { + if (options.ignoreVoid) { + 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]], + ')', + ), + ]; + }, }, - }, - ], - }); + ], + }); + } 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]], + ')', + ), + ]; + }, + }, + ], + }); + } } 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]], - ')', - ), - ]; - }, - }, - ], + messageId: options.ignoreVoid + ? 'floatingPromiseArrayVoid' + : 'floatingPromiseArray', }); } } @@ -206,7 +225,11 @@ export default createRule({ 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 @@ -229,8 +252,14 @@ export default createRule({ return isUnhandledPromise(checker, node.argument); } + if (isPromiseArray(checker, services.esTreeNodeToTSNodeMap.get(node))) { + return { isUnhandled: true, promiseArray: true }; + } + // Check the type. At this point it can't be unhandled if it isn't a promise - if (!isPromiseLike(checker, services.esTreeNodeToTSNodeMap.get(node))) { + if ( + !isPromiseLikeNode(checker, services.esTreeNodeToTSNodeMap.get(node)) + ) { return { isUnhandled: false }; } @@ -296,11 +325,75 @@ export default createRule({ }, }); +function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { + const type = checker.getTypeAtLocation(node); + + // NOTE - I've chosen isArrayType rather than isArrayLikeType because the + // main real use case here is to catch the result of `.map(f)`, + // which should always be a native array type. + // PR QUESTION - should we be using isArrayLikeType instead? + if (checker.isArrayType(type)) { + const arrayType = checker.getTypeArguments(type)[0]; + if (isPromiseLikeType(checker, arrayType)) { + return true; + } + } + + return false; +} + +function isPromiseLikeType(checker: ts.TypeChecker, type: ts.Type): boolean { + // Get the apparent type, which considers type aliases. + const apparentType = checker.getApparentType(type); + + // is any possible union type Promise-like? + return tsutils.unionTypeParts(apparentType).some(typeUnionComponent => { + // Must have 'then' property to be Promise-like. + const thenProperty = typeUnionComponent.getProperty('then'); + if (!thenProperty) { + return false; + } + + // Does the 'then' property have any fitting call signature? + const hasFittingCallSignature = checker + .getTypeOfSymbol(thenProperty) + .getCallSignatures() + .some(signature => { + // Call signature must have onFulfilled and onRejected parameters. + if (!(signature.parameters.length >= 2)) { + return false; + } + + const [onFulFilled, onRejected] = signature.parameters; + + // Can each handler parameter accept a callable? + const bothParametersAreCallable = [onFulFilled, onRejected].every( + handler => { + const handlerType = checker.getTypeOfSymbol(handler); + + const acceptsCallable = tsutils + .unionTypeParts(handlerType) + .some( + handlerTypeUnionComponent => + handlerTypeUnionComponent.getCallSignatures().length !== 0, + ); + + return acceptsCallable; + }, + ); + + return bothParametersAreCallable; + }); + + return hasFittingCallSignature; + }); +} + // 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 { +function isPromiseLikeNode(checker: ts.TypeChecker, node: ts.Node): boolean { const type = checker.getTypeAtLocation(node); for (const ty of tsutils.unionTypeParts(checker.getApparentType(type))) { const then = ty.getProperty('then'); diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index c89e4316dd94..1be360c9cb20 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -222,7 +222,7 @@ async function test() { ` async function test() { class Thenable { - then(callback: () => {}): Thenable { + then(callback: () => void): Thenable { return new Thenable(); } } @@ -264,7 +264,7 @@ async function test() { ` async function test() { class CatchableThenable { - then(callback: () => {}, callback: () => {}): CatchableThenable { + then(callback: () => void, callback: () => void): CatchableThenable { return new CatchableThenable(); } } @@ -475,6 +475,35 @@ Promise.reject() .finally(() => {}); `, }, + { + code: ` +await Promise.all([Promise.resolve(), Promise.resolve()]); + `, + }, + { + code: ` +declare const promiseArray: Array>; +void promiseArray; + `, + }, + { + // This one is a bit of a head-scratcher, knowing that the rule recursively + // checks .finally() expressions. However, ultimately, the whole expression + // is not a promise (it's invalid TS), so it's not a floating promise. + code: ` +[Promise.reject(), Promise.reject()].finally(() => {}); + `, + }, + { + code: ` +[Promise.reject(), Promise.reject()].then(() => {}); + `, + }, + { + code: ` +[1, 2, void Promise.reject(), 3]; + `, + }, ], invalid: [ @@ -997,7 +1026,7 @@ async function test() { code: ` async function test() { class CatchableThenable { - then(callback: () => {}, callback: () => {}): CatchableThenable { + then(callback: () => void, callback: () => void): CatchableThenable { return new CatchableThenable(); } } @@ -1651,5 +1680,47 @@ Promise.reject(new Error('message')).finally(() => {}); `, errors: [{ line: 2, messageId: 'floatingVoid' }], }, + { + code: ` +[1, 2, 3].map(() => Promise.reject()); + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +declare const array: unknown[]; +array.map(() => Promise.reject()); + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +declare const promiseArray: Array>; +void promiseArray; + `, + options: [{ ignoreVoid: false }], + errors: [{ line: 3, messageId: 'floatingPromiseArray' }], + }, + { + code: ` +[1, 2, Promise.reject(), 3]; + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +[1, 2, Promise.reject().catch(() => {}), 3]; + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +const data = ['test']; +data.map(async () => { + await new Promise((_res, rej) => setTimeout(rej, 1000)); +}); + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); From 0244ccd426e9480a3854ea8db0cf05a8123d5e2f Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sat, 11 Nov 2023 14:07:17 -0700 Subject: [PATCH 02/17] remove comment --- packages/eslint-plugin/src/rules/no-floating-promises.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 23650946ea15..87b853a77618 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -327,11 +327,6 @@ export default createRule({ function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { const type = checker.getTypeAtLocation(node); - - // NOTE - I've chosen isArrayType rather than isArrayLikeType because the - // main real use case here is to catch the result of `.map(f)`, - // which should always be a native array type. - // PR QUESTION - should we be using isArrayLikeType instead? if (checker.isArrayType(type)) { const arrayType = checker.getTypeArguments(type)[0]; if (isPromiseLikeType(checker, arrayType)) { From a69fe007f7c9e21718f3ea5171e009bb7d8e4816 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sat, 11 Nov 2023 20:22:59 -0700 Subject: [PATCH 03/17] trying to trigger CI From 8fd0773f757d678b595e330f5ea7cd55f05a29c0 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 12 Nov 2023 21:24:41 -0700 Subject: [PATCH 04/17] Substantial simplification --- .../src/rules/no-floating-promises.ts | 80 +++++-------------- .../tests/rules/no-floating-promises.test.ts | 33 ++++++++ 2 files changed, 54 insertions(+), 59 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 87b853a77618..e2023ac9102e 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -252,14 +252,16 @@ export default createRule({ return isUnhandledPromise(checker, node.argument); } - if (isPromiseArray(checker, services.esTreeNodeToTSNodeMap.get(node))) { + const tsNode = services.esTreeNodeToTSNodeMap.get(node); + + // Check the type. At this point it can't be unhandled if it isn't a promise + // or array thereof. + + if (isPromiseArray(checker, tsNode)) { return { isUnhandled: true, promiseArray: true }; } - // Check the type. At this point it can't be unhandled if it isn't a promise - if ( - !isPromiseLikeNode(checker, services.esTreeNodeToTSNodeMap.get(node)) - ) { + if (!isPromiseLike(checker, tsNode)) { return { isUnhandled: false }; } @@ -327,69 +329,29 @@ export default createRule({ function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { const type = checker.getTypeAtLocation(node); - if (checker.isArrayType(type)) { - const arrayType = checker.getTypeArguments(type)[0]; - if (isPromiseLikeType(checker, arrayType)) { - return true; + for (const ty of tsutils + .unionTypeParts(type) + .map(t => checker.getApparentType(t))) { + if (checker.isArrayType(ty)) { + const arrayType = checker.getTypeArguments(ty)[0]; + if (isPromiseLike(checker, node, arrayType)) { + return true; + } } } - return false; } -function isPromiseLikeType(checker: ts.TypeChecker, type: ts.Type): boolean { - // Get the apparent type, which considers type aliases. - const apparentType = checker.getApparentType(type); - - // is any possible union type Promise-like? - return tsutils.unionTypeParts(apparentType).some(typeUnionComponent => { - // Must have 'then' property to be Promise-like. - const thenProperty = typeUnionComponent.getProperty('then'); - if (!thenProperty) { - return false; - } - - // Does the 'then' property have any fitting call signature? - const hasFittingCallSignature = checker - .getTypeOfSymbol(thenProperty) - .getCallSignatures() - .some(signature => { - // Call signature must have onFulfilled and onRejected parameters. - if (!(signature.parameters.length >= 2)) { - return false; - } - - const [onFulFilled, onRejected] = signature.parameters; - - // Can each handler parameter accept a callable? - const bothParametersAreCallable = [onFulFilled, onRejected].every( - handler => { - const handlerType = checker.getTypeOfSymbol(handler); - - const acceptsCallable = tsutils - .unionTypeParts(handlerType) - .some( - handlerTypeUnionComponent => - handlerTypeUnionComponent.getCallSignatures().length !== 0, - ); - - return acceptsCallable; - }, - ); - - return bothParametersAreCallable; - }); - - return hasFittingCallSignature; - }); -} - // 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 isPromiseLikeNode(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) { diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 1be360c9cb20..7817bd26eb2c 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1682,6 +1682,16 @@ Promise.reject(new Error('message')).finally(() => {}); }, { code: ` +function _>>( + maybePromiseArray: T | undefined, +): void { + maybePromiseArray?.[0]; +} + `, + errors: [{ line: 5, messageId: 'floatingVoid' }], + }, + { + code: ` [1, 2, 3].map(() => Promise.reject()); `, errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], @@ -1722,5 +1732,28 @@ data.map(async () => { `, errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], }, + { + code: ` +function f>>(a: T): void { + a; +} + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +declare const a: Array> | undefined; +a; + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +function f>>(a: T | undefined): void { + a; +} + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); From ea23d54257fd595b5e304d82c1f82be884d498c8 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 12 Nov 2023 21:49:24 -0700 Subject: [PATCH 05/17] add some grody test cases for generics --- .../tests/rules/no-floating-promises.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 7817bd26eb2c..266169f0b66d 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -504,6 +504,11 @@ void promiseArray; [1, 2, void Promise.reject(), 3]; `, }, + { + code: ` +['I', 'am', 'just', 'an', 'array']; + `, + }, ], invalid: [ @@ -1682,8 +1687,8 @@ Promise.reject(new Error('message')).finally(() => {}); }, { code: ` -function _>>( - maybePromiseArray: T | undefined, +function _>>( + maybePromiseArray: S | undefined, ): void { maybePromiseArray?.[0]; } @@ -1734,6 +1739,16 @@ data.map(async () => { }, { code: ` +function _>>>( + maybePromiseArrayArray: S | undefined, +): void { + maybePromiseArrayArray?.[0]; +} + `, + errors: [{ line: 5, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` function f>>(a: T): void { a; } From a486da2247f40b6b5abc8dba4e8324ffce420cce Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 12 Nov 2023 21:51:56 -0700 Subject: [PATCH 06/17] flip if/else --- .../src/rules/no-floating-promises.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index e2023ac9102e..dfe579825dd6 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -113,7 +113,14 @@ export default createRule({ isUnhandledPromise(checker, expression); if (isUnhandled) { - if (!promiseArray) { + if (promiseArray) { + context.report({ + node, + messageId: options.ignoreVoid + ? 'floatingPromiseArrayVoid' + : 'floatingPromiseArray', + }); + } else { if (options.ignoreVoid) { context.report({ node, @@ -178,13 +185,6 @@ export default createRule({ ], }); } - } else { - context.report({ - node, - messageId: options.ignoreVoid - ? 'floatingPromiseArrayVoid' - : 'floatingPromiseArray', - }); } } }, From 8ee0d64ceec78f8ee0400019c3d722f8cc344f30 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 12 Nov 2023 22:30:56 -0700 Subject: [PATCH 07/17] add detail to test case --- .../eslint-plugin/tests/rules/no-floating-promises.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 266169f0b66d..dcf83b15e591 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -500,9 +500,12 @@ void promiseArray; `, }, { + // Expressions aren't checked by this rule, so this just becomes an array + // of number | undefined, which is fine regardless of the ignoreVoid setting. code: ` [1, 2, void Promise.reject(), 3]; `, + options: [{ ignoreVoid: false }], }, { code: ` From 2afe79401108b9b0429c06a787bf9e7af0c0f1b4 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 12 Nov 2023 23:09:30 -0700 Subject: [PATCH 08/17] switch to isArrayLikeType --- packages/eslint-plugin/src/rules/no-floating-promises.ts | 2 +- .../eslint-plugin/tests/rules/no-floating-promises.test.ts | 6 ++++++ packages/eslint-plugin/typings/typescript.d.ts | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index dfe579825dd6..8e089344716e 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -332,7 +332,7 @@ function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { for (const ty of tsutils .unionTypeParts(type) .map(t => checker.getApparentType(t))) { - if (checker.isArrayType(ty)) { + if (checker.isArrayLikeType(ty)) { const arrayType = checker.getTypeArguments(ty)[0]; if (isPromiseLike(checker, node, arrayType)) { return true; diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index dcf83b15e591..6a942132a550 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1773,5 +1773,11 @@ function f>>(a: T | undefined): void { `, errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], }, + { + code: ` +[Promise.reject()] as const; + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index e08a8fb42267..e61b1fe85b68 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -14,6 +14,10 @@ declare module 'typescript' { * - `readonly foo[]` */ isArrayType(type: Type): type is TypeReference; + /** + * True if this type is assignable to `ReadonlyArray`. + */ + isArrayLikeType(type: Type): type is TypeReference; /** * @returns `true` if the given type is a tuple type: * - `[foo]` From 63358203c1f322094c881790c586cb8ea3ee42b7 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 13 Nov 2023 00:27:24 -0700 Subject: [PATCH 09/17] Revert "switch to isArrayLikeType" This reverts commit 2afe79401108b9b0429c06a787bf9e7af0c0f1b4. --- packages/eslint-plugin/src/rules/no-floating-promises.ts | 2 +- .../eslint-plugin/tests/rules/no-floating-promises.test.ts | 6 ------ packages/eslint-plugin/typings/typescript.d.ts | 4 ---- 3 files changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 8e089344716e..dfe579825dd6 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -332,7 +332,7 @@ function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { for (const ty of tsutils .unionTypeParts(type) .map(t => checker.getApparentType(t))) { - if (checker.isArrayLikeType(ty)) { + if (checker.isArrayType(ty)) { const arrayType = checker.getTypeArguments(ty)[0]; if (isPromiseLike(checker, node, arrayType)) { return true; diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 6a942132a550..dcf83b15e591 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1773,11 +1773,5 @@ function f>>(a: T | undefined): void { `, errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], }, - { - code: ` -[Promise.reject()] as const; - `, - errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], - }, ], }); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index e61b1fe85b68..e08a8fb42267 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -14,10 +14,6 @@ declare module 'typescript' { * - `readonly foo[]` */ isArrayType(type: Type): type is TypeReference; - /** - * True if this type is assignable to `ReadonlyArray`. - */ - isArrayLikeType(type: Type): type is TypeReference; /** * @returns `true` if the given type is a tuple type: * - `[foo]` From 852990e370c1a3e4ccd7e40aa1684829c395aed8 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 13 Nov 2023 10:09:42 -0700 Subject: [PATCH 10/17] add checks for tuples --- .../eslint-plugin/src/rules/no-floating-promises.ts | 8 ++++++++ .../tests/rules/no-floating-promises.test.ts | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index dfe579825dd6..779e1c59882e 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -338,6 +338,14 @@ function isPromiseArray(checker: ts.TypeChecker, node: ts.Node): boolean { return true; } } + + if (checker.isTupleType(ty)) { + for (const tupleElementType of checker.getTypeArguments(ty)) { + if (isPromiseLike(checker, node, tupleElementType)) { + return true; + } + } + } } return false; } diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index dcf83b15e591..67d60eda64bc 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1773,5 +1773,18 @@ function f>>(a: T | undefined): void { `, errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], }, + { + code: ` +[Promise.reject()] as const; + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` +declare function cursed(): [Promise, Promise]; +cursed(); + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); From 8c9eadce024678c938c82b76af00846c59e9dbb0 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Mon, 13 Nov 2023 12:31:55 -0700 Subject: [PATCH 11/17] One more test case! --- .../tests/rules/no-floating-promises.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index 67d60eda64bc..ee48be2e9b60 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1786,5 +1786,17 @@ cursed(); `, errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], }, + { + code: ` +[ + 'Type Argument number ', + 1, + 'is not', + Promise.resolve(), + 'but it still is flagged', +] as const; + `, + errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); From 64985560ef7cd0396b79d58513ae11ea322fa910 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Sun, 19 Nov 2023 15:18:46 -0700 Subject: [PATCH 12/17] else if --- .../src/rules/no-floating-promises.ts | 122 +++++++++--------- 1 file changed, 60 insertions(+), 62 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 779e1c59882e..29118805a3a2 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -120,71 +120,69 @@ export default createRule({ ? 'floatingPromiseArrayVoid' : 'floatingPromiseArray', }); - } else { - if (options.ignoreVoid) { - 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]], - ')', - ), - ]; - }, + } else if (options.ignoreVoid) { + 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]], + ')', + ), + ]; }, - ], - }); - } 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, + }, + ], + }); + } 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 (isHigherPrecedenceThanUnary(tsNode)) { - return fixer.insertTextBefore(node, 'await '); - } - return [ - fixer.insertTextBefore(node, 'await ('), - fixer.insertTextAfterRange( - [expression.range[1], expression.range[1]], - ')', - ), - ]; - }, + } + 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]], + ')', + ), + ]; }, - ], - }); - } + }, + ], + }); } } }, From 8c1f52613854fa04a2c2f983f6a9a1a3ee3cc878 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 5 Dec 2023 19:15:04 -0700 Subject: [PATCH 13/17] Remove unnecessary whitespace --- packages/eslint-plugin/src/rules/no-floating-promises.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts index 29118805a3a2..6f3a0fe6e97b 100644 --- a/packages/eslint-plugin/src/rules/no-floating-promises.ts +++ b/packages/eslint-plugin/src/rules/no-floating-promises.ts @@ -63,7 +63,6 @@ export default createRule({ messageBase + ' ' + messageRejectionHandler, floatingUselessRejectionHandlerVoid: messageBaseVoid + ' ' + messageRejectionHandler, - floatingPromiseArray: messagePromiseArray, floatingPromiseArrayVoid: messagePromiseArrayVoid, }, From 7817e8ad81bb192626e107d2a8eea4b44bae9f24 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 5 Dec 2023 19:15:47 -0700 Subject: [PATCH 14/17] Remove invalid test case --- .../tests/rules/no-floating-promises.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index ee48be2e9b60..ffef681f355e 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -486,14 +486,6 @@ declare const promiseArray: Array>; void promiseArray; `, }, - { - // This one is a bit of a head-scratcher, knowing that the rule recursively - // checks .finally() expressions. However, ultimately, the whole expression - // is not a promise (it's invalid TS), so it's not a floating promise. - code: ` -[Promise.reject(), Promise.reject()].finally(() => {}); - `, - }, { code: ` [Promise.reject(), Promise.reject()].then(() => {}); From 81ced8a251618c33239b810966b92550ad08841b Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Tue, 5 Dec 2023 19:39:30 -0700 Subject: [PATCH 15/17] Add to docs --- .../eslint-plugin/docs/rules/no-floating-promises.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.md b/packages/eslint-plugin/docs/rules/no-floating-promises.md index 69eca84b179b..3803cf03502b 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.md +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.md @@ -17,6 +17,13 @@ Valid ways of handling a Promise-valued statement include: - Calling its `.then()` with two arguments - Calling its `.catch()` with one argument +This rule also reports when an array of Promises is created and not properly handled. The main way to resolve this is by using one of the Promise concurrency methods to create a single promise, then handling that according to the procedure above. These methods include: + +- `Promise.all()`, +- `Promise.allSettled()`, +- `Promise.any()` +- `Promise.race()` + :::tip `no-floating-promises` only detects unhandled Promise _statements_. See [`no-misused-promises`](./no-misused-promises.md) for detecting code that provides Promises to _logical_ locations such as if statements. @@ -40,6 +47,8 @@ returnsPromise().then(() => {}); Promise.reject('value').catch(); Promise.reject('value').finally(); + +[1, 2, 3].map(async x => x + 1); ``` ### ✅ Correct @@ -59,6 +68,8 @@ returnsPromise().then( Promise.reject('value').catch(() => {}); await Promise.reject('value').finally(() => {}); + +await Promise.all([1, 2, 3].map(async x => x + 1)); ``` ## Options From d14de703a53fd5db07f72afb9cb2ba55c25fe7bb Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Wed, 6 Dec 2023 14:57:41 -0700 Subject: [PATCH 16/17] Add test cases around array that's fine but tuple that isn't --- .../tests/rules/no-floating-promises.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts index ffef681f355e..434c9735a46d 100644 --- a/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts +++ b/packages/eslint-plugin/tests/rules/no-floating-promises.test.ts @@ -1790,5 +1790,21 @@ cursed(); `, errors: [{ line: 2, messageId: 'floatingPromiseArrayVoid' }], }, + { + code: ` + declare const arrayOrPromiseTuple: + | Array + | [number, number, Promise, string]; + arrayOrPromiseTuple; + `, + errors: [{ line: 5, messageId: 'floatingPromiseArrayVoid' }], + }, + { + code: ` + declare const okArrayOrPromiseArray: Array | Array>; + okArrayOrPromiseArray; + `, + errors: [{ line: 3, messageId: 'floatingPromiseArrayVoid' }], + }, ], }); From 428b9762dc4b9c520a71cbbf5eb3060e961d4373 Mon Sep 17 00:00:00 2001 From: Kirk Waiblinger Date: Thu, 7 Dec 2023 10:11:25 -0700 Subject: [PATCH 17/17] tweaks --- packages/eslint-plugin/docs/rules/no-floating-promises.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/docs/rules/no-floating-promises.md b/packages/eslint-plugin/docs/rules/no-floating-promises.md index 3803cf03502b..449b553c48d8 100644 --- a/packages/eslint-plugin/docs/rules/no-floating-promises.md +++ b/packages/eslint-plugin/docs/rules/no-floating-promises.md @@ -17,7 +17,7 @@ Valid ways of handling a Promise-valued statement include: - Calling its `.then()` with two arguments - Calling its `.catch()` with one argument -This rule also reports when an array of Promises is created and not properly handled. The main way to resolve this is by using one of the Promise concurrency methods to create a single promise, then handling that according to the procedure above. These methods include: +This rule also reports when an Array containing Promises is created and not properly handled. The main way to resolve this is by using one of the Promise concurrency methods to create a single Promise, then handling that according to the procedure above. These methods include: - `Promise.all()`, - `Promise.allSettled()`, @@ -115,3 +115,7 @@ If you do not use Promise-like values in your codebase, or want to allow them to ## Related To - [`no-misused-promises`](./no-misused-promises.md) + +## Further Reading + +- ["Using Promises" MDN documentation](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises). Note especially the sections on [Promise rejection events](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises#promise_rejection_events) and [Composition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Using_promises#composition).