-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(eslint-plugin): [await-thenable] report invalid (non-promise) values passed to promise aggregator methods #11267
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
Changes from all commits
4bd647c
20ff75d
921942e
e5bffbf
62a6eed
d46056a
ad08eb1
bcf18dc
a409343
d7fb0b1
99343da
0e1ac05
94cbf8c
67095b7
39551b1
81aead1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,13 @@ | ||
| import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; | ||
| import type { TSESLint } from '@typescript-eslint/utils'; | ||
| import type * as ts from 'typescript'; | ||
|
|
||
| import { TSESTree } from '@typescript-eslint/utils'; | ||
| import * as tsutils from 'ts-api-utils'; | ||
|
|
||
| import { | ||
| Awaitable, | ||
| createRule, | ||
| getConstrainedTypeAtLocation, | ||
| getFixOrSuggest, | ||
| getParserServices, | ||
| isAwaitKeyword, | ||
|
|
@@ -14,12 +17,14 @@ import { | |
| NullThrowsReasons, | ||
| } from '../util'; | ||
| import { getForStatementHeadLoc } from '../util/getForStatementHeadLoc'; | ||
| import { isPromiseAggregatorMethod } from '../util/isPromiseAggregatorMethod'; | ||
|
|
||
| export type MessageId = | ||
| | 'await' | ||
| | 'awaitUsingOfNonAsyncDisposable' | ||
| | 'convertToOrdinaryFor' | ||
| | 'forAwaitOfNonAsyncIterable' | ||
| | 'invalidPromiseAggregatorInput' | ||
| | 'removeAwait'; | ||
|
|
||
| export default createRule<[], MessageId>({ | ||
|
|
@@ -39,6 +44,8 @@ export default createRule<[], MessageId>({ | |
| convertToOrdinaryFor: 'Convert to an ordinary `for...of` loop.', | ||
| forAwaitOfNonAsyncIterable: | ||
| 'Unexpected `for await...of` of a value that is not async iterable.', | ||
| invalidPromiseAggregatorInput: | ||
| 'Unexpected iterable of non-Promise (non-"Thenable") values passed to promise aggregator.', | ||
| removeAwait: 'Remove unnecessary `await`.', | ||
| }, | ||
| schema: [], | ||
|
|
@@ -84,6 +91,53 @@ export default createRule<[], MessageId>({ | |
| } | ||
| }, | ||
|
|
||
| CallExpression(node: TSESTree.CallExpression): void { | ||
| if (!isPromiseAggregatorMethod(context, services, node)) { | ||
| return; | ||
| } | ||
|
|
||
| const argument = node.arguments.at(0); | ||
|
|
||
| if (argument == null) { | ||
| return; | ||
| } | ||
|
|
||
| if (argument.type === TSESTree.AST_NODE_TYPES.ArrayExpression) { | ||
| for (const element of argument.elements) { | ||
| if (element == null) { | ||
| continue; | ||
kirkwaiblinger marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| const type = getConstrainedTypeAtLocation(services, element); | ||
| const tsNode = services.esTreeNodeToTSNodeMap.get(element); | ||
|
|
||
| if (containsNonAwaitableType(type, tsNode, checker)) { | ||
| context.report({ | ||
| node: element, | ||
| messageId: 'invalidPromiseAggregatorInput', | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const type = getConstrainedTypeAtLocation(services, argument); | ||
|
|
||
| if ( | ||
| isInvalidPromiseAggregatorInput( | ||
| checker, | ||
| services.esTreeNodeToTSNodeMap.get(argument), | ||
| type, | ||
| ) | ||
| ) { | ||
| context.report({ | ||
| node: argument, | ||
| messageId: 'invalidPromiseAggregatorInput', | ||
| }); | ||
| } | ||
| }, | ||
|
|
||
| 'ForOfStatement[await=true]'(node: TSESTree.ForOfStatement): void { | ||
| const type = services.getTypeAtLocation(node.right); | ||
| if (isTypeAnyType(type)) { | ||
|
|
@@ -176,3 +230,75 @@ export default createRule<[], MessageId>({ | |
| }; | ||
| }, | ||
| }); | ||
|
|
||
| function isInvalidPromiseAggregatorInput( | ||
| checker: ts.TypeChecker, | ||
| node: ts.Node, | ||
| type: ts.Type, | ||
| ): boolean { | ||
| // non array/tuple/iterable types already show up as a type error | ||
| if (!isIterable(type, checker)) { | ||
| return false; | ||
| } | ||
|
|
||
| for (const part of tsutils.unionConstituents(type)) { | ||
| const valueTypes = getValueTypesOfArrayLike(part, checker); | ||
|
|
||
| if (valueTypes != null) { | ||
| for (const typeArgument of valueTypes) { | ||
| if (containsNonAwaitableType(typeArgument, node, checker)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| function getValueTypesOfArrayLike( | ||
| type: ts.Type, | ||
| checker: ts.TypeChecker, | ||
| ): readonly ts.Type[] | null { | ||
| if (checker.isTupleType(type)) { | ||
| return checker.getTypeArguments(type); | ||
| } | ||
|
|
||
| if (checker.isArrayLikeType(type)) { | ||
| return [ | ||
| nullThrows( | ||
| type.getNumberIndexType(), | ||
| 'number index type should exist on an array-like', | ||
| ), | ||
| ]; | ||
| } | ||
|
|
||
| // `Iterable<...>` | ||
| if (tsutils.isTypeReference(type)) { | ||
| return checker.getTypeArguments(type).slice(0, 1); | ||
| } | ||
|
|
||
| return null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (cov nit) this might also be unreachable? 🤷 But if so
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I completely missed codcov, thanks! I've updated the PR with your suggestions. I'm not sure about this one though, it's hard to tell if it's truly unreachable or if there's a case I've missed. I think this is OK either way (though I think this would require throwing like is done here instead of |
||
| } | ||
|
|
||
| function containsNonAwaitableType( | ||
| type: ts.Type, | ||
| node: ts.Node, | ||
| checker: ts.TypeChecker, | ||
| ): boolean { | ||
| return tsutils | ||
| .unionConstituents(type) | ||
| .some( | ||
| typeArgumentPart => | ||
| needsToBeAwaited(checker, node, typeArgumentPart) === Awaitable.Never, | ||
| ); | ||
| } | ||
|
|
||
| function isIterable(type: ts.Type, checker: ts.TypeChecker): boolean { | ||
| return tsutils | ||
| .unionConstituents(type) | ||
| .every( | ||
| part => | ||
| !!tsutils.getWellKnownSymbolPropertyOfType(part, 'iterator', checker), | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import type { | ||
| ParserServicesWithTypeInformation, | ||
| TSESTree, | ||
| } from '@typescript-eslint/utils'; | ||
| import type { RuleContext } from '@typescript-eslint/utils/ts-eslint'; | ||
|
|
||
| import { | ||
| getConstrainedTypeAtLocation, | ||
| isPromiseConstructorLike, | ||
| } from '@typescript-eslint/type-utils'; | ||
| import { AST_NODE_TYPES } from '@typescript-eslint/utils'; | ||
|
|
||
| import { getStaticMemberAccessValue } from './misc'; | ||
|
|
||
| const PROMISE_CONSTRUCTOR_ARRAY_METHODS = new Set<unknown>([ | ||
| 'all', | ||
| 'allSettled', | ||
| 'race', | ||
| 'any', | ||
| ]); | ||
|
|
||
| export function isPromiseAggregatorMethod( | ||
| context: RuleContext<string, unknown[]>, | ||
| services: ParserServicesWithTypeInformation, | ||
| node: TSESTree.CallExpression, | ||
| ): boolean { | ||
| if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { | ||
| return false; | ||
| } | ||
|
|
||
| const staticAccessValue = getStaticMemberAccessValue(node.callee, context); | ||
|
|
||
| if (!PROMISE_CONSTRUCTOR_ARRAY_METHODS.has(staticAccessValue)) { | ||
| return false; | ||
| } | ||
|
|
||
| return isPromiseConstructorLike( | ||
| services.program, | ||
| getConstrainedTypeAtLocation(services, node.callee.object), | ||
| ); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.