From 2fa678f902fc97e062c6d2d097036cc0eabac4b6 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:00:19 +0200 Subject: [PATCH 01/10] initial implementation --- .../eslint-plugin/src/rules/await-thenable.ts | 4 +- packages/eslint-plugin/src/util/index.ts | 1 + .../src/util/needsToBeAwaited.ts | 44 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/src/util/needsToBeAwaited.ts diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index c1b50766e3ae..c51719cec133 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -3,12 +3,14 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; import { + Awaitable, createRule, getFixOrSuggest, getParserServices, isAwaitKeyword, isTypeAnyType, isTypeUnknownType, + needsToBeAwaited, nullThrows, NullThrowsReasons, } from '../util'; @@ -57,7 +59,7 @@ export default createRule<[], MessageId>({ const originalNode = services.esTreeNodeToTSNodeMap.get(node); - if (!tsutils.isThenableType(checker, originalNode.expression, type)) { + if (needsToBeAwaited(checker, originalNode, type) === Awaitable.Never) { context.report({ node, messageId: 'await', diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 3f1e4b5cb2dc..3aa935a51a95 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -20,6 +20,7 @@ export * from './isUndefinedIdentifier'; export * from './misc'; export * from './needsPrecedingSemiColon'; export * from './objectIterators'; +export * from './needsToBeAwaited'; export * from './scopeUtils'; export * from './types'; diff --git a/packages/eslint-plugin/src/util/needsToBeAwaited.ts b/packages/eslint-plugin/src/util/needsToBeAwaited.ts new file mode 100644 index 000000000000..50ca06e3a576 --- /dev/null +++ b/packages/eslint-plugin/src/util/needsToBeAwaited.ts @@ -0,0 +1,44 @@ +import type * as ts from 'typescript'; + +import { + isTypeAnyType, + isTypeUnknownType, +} from '@typescript-eslint/type-utils'; +import * as tsutils from 'ts-api-utils'; + +export enum Awaitable { + Always, + Never, + May, +} + +export function needsToBeAwaited( + checker: ts.TypeChecker, + node: ts.Node, + type: ts.Type, +): Awaitable { + // `any` and `unknown` types may need to be awaited + if (isTypeAnyType(type) || isTypeUnknownType(type)) { + return Awaitable.May; + } + + // 'thenable' values should always be be awaited + if (tsutils.isThenableType(checker, node, type)) { + return Awaitable.Always; + } + + // recurse into a type parameter constraint + if (tsutils.isTypeParameter(type)) { + const constraint = type.getConstraint(); + + // if there's no constraint, this may need to be awaited + if (!constraint) { + return Awaitable.May; + } + + return needsToBeAwaited(checker, node, constraint); + } + + // anything else should not be awaited + return Awaitable.Never; +} From 8dbfec10511fbdf2d4e665bd358afc203e4cc1e0 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:05:04 +0200 Subject: [PATCH 02/10] share code with return-await --- packages/eslint-plugin/src/rules/return-await.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 7795894a9584..a6a0b2241967 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -1,17 +1,16 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import * as tsutils from 'ts-api-utils'; import * as ts from 'typescript'; import { + Awaitable, createRule, getFixOrSuggest, getParserServices, isAwaitExpression, isAwaitKeyword, - isTypeAnyType, - isTypeUnknownType, + needsToBeAwaited, nullThrows, } from '../util'; import { getOperatorPrecedence } from '../util/getOperatorPrecedence'; @@ -304,16 +303,16 @@ export default createRule({ } const type = checker.getTypeAtLocation(child); - const isThenable = tsutils.isThenableType(checker, expression, type); + const awaited = needsToBeAwaited(checker, expression, type); // handle awaited _non_thenables - if (!isThenable) { + if (awaited !== Awaitable.Always) { if (isAwait) { - // any/unknown could be thenable; do not enforce whether they are `await`ed. - if (isTypeAnyType(type) || isTypeUnknownType(type)) { + if (awaited === Awaitable.May) { return; } + context.report({ node, messageId: 'nonPromiseAwait', From 169f100c565e2d1d39a80b9787ee68cc7a449322 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:10:33 +0200 Subject: [PATCH 03/10] tests --- .../tests/rules/await-thenable.test.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 0739334cf130..54641cdc7c82 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -275,6 +275,41 @@ async function foo() { async function iterateUsing(arr: Array) { for (await using foo of arr) { } +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper>(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper>(value: T) { + return await value; } `, }, @@ -639,5 +674,31 @@ async function foo() { }, ], }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + errors: [ + { + column: 10, + endColumn: 21, + endLine: 3, + line: 3, + messageId: 'await', + suggestions: [ + { + messageId: 'removeAwait', + output: ` +async function wrapper(value: T) { + return value; +} + `, + }, + ], + }, + ], + }, ], }); From 92fb850ce2d8f059a215e506820e25de92bae4d0 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:14:51 +0200 Subject: [PATCH 04/10] small changes --- packages/eslint-plugin/src/rules/await-thenable.ts | 3 ++- packages/eslint-plugin/src/rules/return-await.ts | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index c51719cec133..dd68b20e32f0 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -58,8 +58,9 @@ export default createRule<[], MessageId>({ } const originalNode = services.esTreeNodeToTSNodeMap.get(node); + const certainty = needsToBeAwaited(checker, originalNode, type); - if (needsToBeAwaited(checker, originalNode, type) === Awaitable.Never) { + if (certainty === Awaitable.Never) { context.report({ node, messageId: 'await', diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index a6a0b2241967..00d240abca5c 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -303,13 +303,13 @@ export default createRule({ } const type = checker.getTypeAtLocation(child); - const awaited = needsToBeAwaited(checker, expression, type); + const certainty = needsToBeAwaited(checker, expression, type); // handle awaited _non_thenables - if (awaited !== Awaitable.Always) { + if (certainty !== Awaitable.Always) { if (isAwait) { - if (awaited === Awaitable.May) { + if (certainty === Awaitable.May) { return; } From 18f3e7b8e291e59d4b876f6bbb9e05059dfa42f6 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:15:44 +0200 Subject: [PATCH 05/10] remove unnecessary spaces --- packages/eslint-plugin/src/rules/return-await.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/return-await.ts b/packages/eslint-plugin/src/rules/return-await.ts index 00d240abca5c..18cc6e9e420a 100644 --- a/packages/eslint-plugin/src/rules/return-await.ts +++ b/packages/eslint-plugin/src/rules/return-await.ts @@ -312,7 +312,6 @@ export default createRule({ if (certainty === Awaitable.May) { return; } - context.report({ node, messageId: 'nonPromiseAwait', From fa7e350d78aaa6014745697a00978ca7643ea9d3 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:18:17 +0200 Subject: [PATCH 06/10] adjust tests --- .../tests/rules/await-thenable.test.ts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 54641cdc7c82..8e70aeec3e7d 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -310,6 +310,22 @@ async function wrapper>(value: T) { code: ` async function wrapper>(value: T) { return await value; +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } } `, }, @@ -693,6 +709,36 @@ async function wrapper(value: T) { output: ` async function wrapper(value: T) { return value; +} + `, + }, + ], + }, + ], + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + errors: [ + { + column: 12, + endColumn: 23, + endLine: 4, + line: 4, + messageId: 'await', + suggestions: [ + { + messageId: 'removeAwait', + output: ` +class C { + async wrapper(value: T) { + return value; + } } `, }, From 6481ef02998462be6bac38a0f7640b121daf0ce1 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 9 Nov 2024 17:39:19 +0200 Subject: [PATCH 07/10] adjust tests --- .../tests/rules/await-thenable.test.ts | 7 ----- .../tests/rules/return-await.test.ts | 30 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 8e70aeec3e7d..943ee53b1015 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -315,13 +315,6 @@ async function wrapper>(value: T) { }, { code: ` -async function wrapper(value: T) { - return await value; -} - `, - }, - { - code: ` class C { async wrapper(value: T) { return await value; diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index e7610eaba6ee..939c4608fde8 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -445,6 +445,36 @@ return Promise.resolve(42); { using foo = 1 as any; return Promise.resolve(42); +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } } `, }, From 3b7cf260c0950cfc92892af97358e79293362d22 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 11 Nov 2024 20:42:43 +0200 Subject: [PATCH 08/10] Update packages/eslint-plugin/src/util/needsToBeAwaited.ts Co-authored-by: Kirk Waiblinger --- .../src/util/needsToBeAwaited.ts | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/packages/eslint-plugin/src/util/needsToBeAwaited.ts b/packages/eslint-plugin/src/util/needsToBeAwaited.ts index 50ca06e3a576..e6d675cc321e 100644 --- a/packages/eslint-plugin/src/util/needsToBeAwaited.ts +++ b/packages/eslint-plugin/src/util/needsToBeAwaited.ts @@ -17,28 +17,27 @@ export function needsToBeAwaited( node: ts.Node, type: ts.Type, ): Awaitable { + // can't use `getConstrainedTypeAtLocation` directly since it's bugged for + // unconstrained generics. + const constrainedType = !tsutils.isTypeParameter(type) + ? type + : checker.getBaseConstraintOfType(type); + + // unconstrained generic types should be treated as unknown + if (constrainedType == null) { + return Awaitable.May; + } + // `any` and `unknown` types may need to be awaited - if (isTypeAnyType(type) || isTypeUnknownType(type)) { + if (isTypeAnyType(constrainedType) || isTypeUnknownType(constrainedType)) { return Awaitable.May; } // 'thenable' values should always be be awaited - if (tsutils.isThenableType(checker, node, type)) { + if (tsutils.isThenableType(checker, node, constrainedType)) { return Awaitable.Always; } - // recurse into a type parameter constraint - if (tsutils.isTypeParameter(type)) { - const constraint = type.getConstraint(); - - // if there's no constraint, this may need to be awaited - if (!constraint) { - return Awaitable.May; - } - - return needsToBeAwaited(checker, node, constraint); - } - // anything else should not be awaited return Awaitable.Never; } From 7b3b64f891251e0b2fbb472ad0bfaaea7536e0ae Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 11 Nov 2024 20:43:54 +0200 Subject: [PATCH 09/10] remove unnecessary code --- packages/eslint-plugin/src/rules/await-thenable.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts index dd68b20e32f0..6fb49227470a 100644 --- a/packages/eslint-plugin/src/rules/await-thenable.ts +++ b/packages/eslint-plugin/src/rules/await-thenable.ts @@ -9,7 +9,6 @@ import { getParserServices, isAwaitKeyword, isTypeAnyType, - isTypeUnknownType, needsToBeAwaited, nullThrows, NullThrowsReasons, @@ -53,9 +52,6 @@ export default createRule<[], MessageId>({ return { AwaitExpression(node): void { const type = services.getTypeAtLocation(node.argument); - if (isTypeAnyType(type) || isTypeUnknownType(type)) { - return; - } const originalNode = services.esTreeNodeToTSNodeMap.get(node); const certainty = needsToBeAwaited(checker, originalNode, type); From aa274e9f94eed2a360884f327d08db9cb2b5cf5b Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 11 Nov 2024 20:45:33 +0200 Subject: [PATCH 10/10] test a case with a generic whose constraint is another generic --- .../tests/rules/await-thenable.test.ts | 48 +++++++++++ .../tests/rules/return-await.test.ts | 80 +++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/await-thenable.test.ts b/packages/eslint-plugin/tests/rules/await-thenable.test.ts index 943ee53b1015..1860db956c3e 100644 --- a/packages/eslint-plugin/tests/rules/await-thenable.test.ts +++ b/packages/eslint-plugin/tests/rules/await-thenable.test.ts @@ -319,6 +319,24 @@ class C { async wrapper(value: T) { return await value; } +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } } `, }, @@ -732,6 +750,36 @@ class C { async wrapper(value: T) { return value; } +} + `, + }, + ], + }, + ], + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + errors: [ + { + column: 12, + endColumn: 23, + endLine: 4, + line: 4, + messageId: 'await', + suggestions: [ + { + messageId: 'removeAwait', + output: ` +class C { + async wrapper(value: T) { + return value; + } } `, }, diff --git a/packages/eslint-plugin/tests/rules/return-await.test.ts b/packages/eslint-plugin/tests/rules/return-await.test.ts index 939c4608fde8..9a44a5954d66 100644 --- a/packages/eslint-plugin/tests/rules/return-await.test.ts +++ b/packages/eslint-plugin/tests/rules/return-await.test.ts @@ -475,6 +475,24 @@ class C { async wrapper(value: T) { return await value; } +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } } `, }, @@ -1597,6 +1615,68 @@ async function outerFunction() { }; const innerFunction = async () => asyncFn(); +} + `, + }, + { + code: ` +async function wrapper(value: T) { + return await value; +} + `, + errors: [ + { + line: 3, + messageId: 'nonPromiseAwait', + }, + ], + output: ` +async function wrapper(value: T) { + return value; +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + errors: [ + { + line: 4, + messageId: 'nonPromiseAwait', + }, + ], + output: ` +class C { + async wrapper(value: T) { + return value; + } +} + `, + }, + { + code: ` +class C { + async wrapper(value: T) { + return await value; + } +} + `, + errors: [ + { + line: 4, + messageId: 'nonPromiseAwait', + }, + ], + output: ` +class C { + async wrapper(value: T) { + return value; + } } `, },