From 5302e728c76562ba4035af27447f0225d4ff0768 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Mon, 18 Dec 2023 12:36:55 -0800 Subject: [PATCH 01/26] feat(eslint-plugin): [thenable-in-promise-aggregators] resolve #1804 --- packages/eslint-plugin/src/rules/index.ts | 2 + .../rules/thenable-in-promise-aggregators.ts | 124 ++++++ .../thenable-in-promise-aggregators.test.ts | 358 ++++++++++++++++++ 3 files changed, 484 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts create mode 100644 packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 94cce184242d..06ff2a764d92 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -132,6 +132,7 @@ import spaceBeforeFunctionParen from './space-before-function-paren'; import spaceInfixOps from './space-infix-ops'; import strictBooleanExpressions from './strict-boolean-expressions'; import switchExhaustivenessCheck from './switch-exhaustiveness-check'; +import thenableInPromiseAggregators from './thenable-in-promise-aggregators'; import tripleSlashReference from './triple-slash-reference'; import typeAnnotationSpacing from './type-annotation-spacing'; import typedef from './typedef'; @@ -273,6 +274,7 @@ export default { 'space-infix-ops': spaceInfixOps, 'strict-boolean-expressions': strictBooleanExpressions, 'switch-exhaustiveness-check': switchExhaustivenessCheck, + 'thenable-in-promise-aggregators': thenableInPromiseAggregators, 'triple-slash-reference': tripleSlashReference, 'type-annotation-spacing': typeAnnotationSpacing, typedef: typedef, diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts new file mode 100644 index 000000000000..e5a50117abe6 --- /dev/null +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -0,0 +1,124 @@ +import * as tsutils from 'ts-api-utils'; +import { + isTypeAnyType, + isTypeUnknownType, +} from '@typescript-eslint/type-utils'; +import { createRule, getParserServices } from '../util'; + +export default createRule({ + name: 'thenable-in-promise-aggregators', + meta: { + docs: { + description: + 'Disallow passing non-Thenable values to promise aggregators', + recommended: 'recommended', + requiresTypeChecking: true, + }, + messages: { + inArray: + 'Unexpected non-Thenable value in array passed to promise aggregator.', + arrayArg: + 'Unexpected array of non-Thenable values passed to promise aggregator.', + nonArrayArg: 'Unexpected non-array passed to promise aggregator.', + }, + schema: [], + type: 'problem', + }, + defaultOptions: [], + + create(context) { + const services = getParserServices(context); + const checker = services.program.getTypeChecker(); + + return { + CallExpression(node): void { + if (node.callee.type !== 'MemberExpression') { + return; + } + if (node.callee.object.type !== 'Identifier') { + return; + } + if (node.callee.object.name !== 'Promise') { + return; + } + if (node.callee.property.type !== 'Identifier') { + return; + } + + const { name } = node.callee.property; + if (!['race', 'all', 'allSettled'].includes(name)) { + return; + } + + const { arguments: args } = node; + if (args.length !== 1) { + return; + } + + const arg = args[0]; + if (arg.type === 'ArrayExpression') { + const { elements } = arg; + if (elements.length === 0) { + return; + } + + for (const element of elements) { + if (element === null) { + continue; + } + const elementType = services.getTypeAtLocation(element); + if (isTypeAnyType(elementType) || isTypeUnknownType(elementType)) { + continue; + } + + const originalNode = services.esTreeNodeToTSNodeMap.get(element); + if (tsutils.isThenableType(checker, originalNode, elementType)) { + continue; + } + + context.report({ + messageId: 'inArray', + node: element, + }); + } + } else { + const argType = services.getTypeAtLocation(arg); + if (isTypeAnyType(argType) || isTypeUnknownType(argType)) { + return; + } + + if (!checker.isArrayType(argType)) { + context.report({ + messageId: 'nonArrayArg', + node: arg, + }); + return; + } + + if (argType.typeArguments === undefined) { + return; + } + + if (argType.typeArguments.length < 1) { + return; + } + + const typeArg = argType.typeArguments[0]; + if (isTypeAnyType(typeArg) || isTypeUnknownType(typeArg)) { + return; + } + + const originalNode = services.esTreeNodeToTSNodeMap.get(arg); + if (tsutils.isThenableType(checker, originalNode, typeArg)) { + return; + } + + context.report({ + messageId: 'arrayArg', + node: arg, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts new file mode 100644 index 000000000000..ab0e6087df3d --- /dev/null +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -0,0 +1,358 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import rule from '../../src/rules/await-thenable'; +import { getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const messageIdInArray = 'inArray'; +const messageIdArrayArg = 'arrayArg'; +const messageIdNonArrayArg = 'nonArrayArg'; + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('thenable-in-promise-aggregators', rule, { + valid: [ + ` +async function test() { + await Promise.race([Promise.resolve(3)]); +} + `, + ` +async function test() { + await Promise.all([Promise.resolve(3)]); +} + `, + ` +async function test() { + await Promise.allSettled([Promise.resolve(3)]); +} + `, + ` +async function test() { + await Promise.race([(async () => true)()]); +} + `, + ` +async function test() { + function returnsPromise() { + return Promise.resolve('value'); + } + await Promise.race([returnsPromise()]); +} + `, + ` +async function test() { + async function returnsPromiseAsync() {} + await Promise.race([returnsPromiseAsync()]); +} + `, + ` +async function test() { + let anyValue: any; + await Promise.race([anyValue]); +} + `, + ` +async function test() { + let unknownValue: unknown; + await Promise.race([unknownValue]); +} + `, + ` +async function test() { + const numberPromise: Promise; + await Promise.race([numberPromise]); +} + `, + ` +async function test() { + class Foo extends Promise {} + const foo: Foo = Foo.resolve(2); + await Promise.race([foo]); + + class Bar extends Foo {} + const bar: Bar = Bar.resolve(2); + await Promise.race([bar]); +} + `, + ` +async function test() { + await Promise.race([(Math.random() > 0.5 ? numberPromise : 0)]); + await Promise.race([(Math.random() > 0.5 ? foo : 0)]); + await Promise.race([(Math.random() > 0.5 ? bar : 0)]); + + const intersectionPromise: Promise & number; + await Promise.race([intersectionPromise]); +} + `, + ` +async function test() { + class Thenable { + then(callback: () => {}) {} + } + const thenable = new Thenable(); + + await Promise.race([thenable]); +} + `, + ` +const doSomething = async ( + obj1: { a?: { b?: { c?: () => Promise } } }, + obj2: { a?: { b?: { c: () => Promise } } }, + obj3: { a?: { b: { c?: () => Promise } } }, + obj4: { a: { b: { c?: () => Promise } } }, + obj5: { a?: () => { b?: { c?: () => Promise } } }, + obj6?: { a: { b: { c?: () => Promise } } }, + callback?: () => Promise, +): Promise => { + await Promise.all([ + obj1.a?.b?.c?.(), + obj2.a?.b?.c(), + obj3.a?.b.c?.(), + obj4.a.b.c?.(), + obj5.a?.().b?.c?.(), + obj6?.a.b.c?.() + ]); + + await Promise.allSettled([callback?.()]); +}; + `, + ` +async function test() { + const promiseArr: Promise[]; + await Promise.all(promiseArr); +} + `, + ` +async function test() { + const intersectionArr: (Promise & number)[]; + await Promise.all(intersectionArr); +} + `, + ` +async function test() { + const values = [1, 2, 3]; + await Promise.all(values.map(value => Promise.resolve(value))); +} + `, + ` +async function test() { + const values = [1, 2, 3]; + await Promise.all(values.map(async (value) => {})); +} + `, + ], + + invalid: [ + { + code: 'await Promise.race([0]);', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: 'await Promise.all([0]);', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: 'await Promise.allSettled([0]);', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: 'await Promise.race([Promise.resolve(3), 0]);', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: 'async () => await Promise.race([await Promise.resolve(3)]);', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: `async () => await Promise.race([Math.random() > 0.5 ? '' : 0]);`, + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: ` +class NonPromise extends Array {} +await Promise.race([new NonPromise()]); + `, + errors: [ + { + line: 3, + messageIdArrayArg, + suggestions: [], + }, + ], + }, + { + code: ` +async function test() { + class IncorrectThenable { + then() {} + } + const thenable = new IncorrectThenable(); + + await Promise.race([thenable]); +} + `, + errors: [ + { + line: 8, + messageIdArrayArg, + }, + ], + }, + { + code: ` +declare const callback: (() => void) | undefined; +await Promise.race([callback?.()]); + `, + errors: [ + { + line: 3, + messageIdArrayArg, + }, + ], + }, + { + code: ` +declare const obj: { a?: { b?: () => void } }; +await Promise.race([obj.a?.b?.()]); + `, + errors: [ + { + line: 3, + messageIdArrayArg, + }, + ], + }, + { + code: ` +declare const obj: { a: { b: { c?: () => void } } } | undefined; +await Promise.race([obj?.a.b.c?.()]); + `, + errors: [ + { + line: 3, + messageIdArrayArg, + }, + ], + }, + { + code: ` +declare const wrappedPromise: { promise: Promise }; +declare const stdPromise: Promise; +await Promise.all([wrappedPromise, stdPromise]); + `, + errors: [ + { + line: 3, + messageIdInArray, + }, + ], + }, + { + code: 'await Promise.race(3);', + errors: [ + { + line: 1, + messageIdNonArrayArg, + }, + ], + }, + { + code: 'await Promise.all(3);', + errors: [ + { + line: 1, + messageIdNonArrayArg, + }, + ], + }, + { + code: 'await Promise.allSettled({ foo: 3 });', + errors: [ + { + line: 1, + messageIdNonArrayArg, + }, + ], + }, + { + code: 'await Promise.race(undefined);', + errors: [ + { + line: 1, + messageIdNonArrayArg, + }, + ], + }, + { + code: ` +declare const promiseArr: Promise; +await Promise.all(promiseArr); + `, + errors: [ + { + line: 3, + messageIdNonArrayArg, + }, + ], + }, + { + code: 'await Promise.all([0, 1].map((v) => v)', + errors: [ + { + line: 1, + messageIdArrayArg, + }, + ], + }, + { + code: ` +declare const promiseArr: Promise[]; +await Promise.all(promiseArr.map((v) => await v)); + `, + errors: [ + { + line: 3, + messageIdArrayArg, + }, + ], + }, + ], +}); From 3c79e83014dbe1cecff70fa9747c7d54904caf66 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Mon, 18 Dec 2023 13:27:02 -0800 Subject: [PATCH 02/26] Working tests/spelling/lint --- .../rules/thenable-in-promise-aggregators.md | 49 +++++++++++++ packages/eslint-plugin/src/configs/all.ts | 1 + .../src/configs/disable-type-checked.ts | 1 + .../src/configs/recommended-type-checked.ts | 1 + .../src/configs/strict-type-checked.ts | 1 + .../rules/thenable-in-promise-aggregators.ts | 17 +++-- .../thenable-in-promise-aggregators.test.ts | 72 +++++++++---------- .../thenable-in-promise-aggregators.shot | 14 ++++ 8 files changed, 113 insertions(+), 43 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md create mode 100644 packages/eslint-plugin/tests/schema-snapshots/thenable-in-promise-aggregators.shot diff --git a/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md new file mode 100644 index 000000000000..cd4200554ea1 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md @@ -0,0 +1,49 @@ +--- +description: 'Disallow passing non-Thenable values to promise aggregators.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/thenable-in-promise-aggregators** for documentation. + +A "Thenable" value is an object which has a `then` method, such as a Promise. +The `await` keyword is generally used to retrieve the result of calling a Thenable's `then` method. + +When multiple Thenable's are running at the same time, it is sometimes desirable to wait until any one of them resolves (`Promise.race`), all of them resolve or any of them reject (`Promise.all`), or all of them resolve or reject (`Promise.allSettled`). + +Each of these functions accept an iterable of promises as input and return a single +Promise. +If a non-Thenable is passed, it is ignored. +While doing so is valid JavaScript, it is often a programmer error, such as forgetting to unwrap a wrapped promise, or using the `await` keyword on the individual promises, which defeats the purpose of using one of these Promise aggregators. + +## Examples + + + +### ❌ Incorrect + +```ts +await Promise.race(['value1', 'value2']); + +await Promise.race([ + await new Promise(resolve => setTimeout(resolve, 3000)), + await new Promise(resolve => setTimeout(resolve, 6000)), +]); +``` + +### ✅ Correct + +```ts +await Promise.race([Promise.resolve('value1'), Promise.resolve('value2')]); + +await Promise.race([ + new Promise(resolve => setTimeout(resolve, 3000)), + new Promise(resolve => setTimeout(resolve, 6000)), +]); +``` + +## When Not To Use It + +If you want to allow code to use `Promise.race`, `Promise.all`, or `Promise.allSettled` on arrays of non-promise values. +This is generally not preferred but can sometimes be useful for visual consistency. +You might consider using [ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) for those specific situations instead of completely disabling this rule. diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 3a5e6343b197..0fbbc691e96c 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -146,6 +146,7 @@ export = { '@typescript-eslint/sort-type-constituents': 'error', '@typescript-eslint/strict-boolean-expressions': 'error', '@typescript-eslint/switch-exhaustiveness-check': 'error', + '@typescript-eslint/thenable-in-promise-aggregators': 'error', '@typescript-eslint/triple-slash-reference': 'error', '@typescript-eslint/typedef': 'error', '@typescript-eslint/unbound-method': 'error', diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index 4cd82bf2414e..ebf6d21e9ee9 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -57,6 +57,7 @@ export = { '@typescript-eslint/return-await': 'off', '@typescript-eslint/strict-boolean-expressions': 'off', '@typescript-eslint/switch-exhaustiveness-check': 'off', + '@typescript-eslint/thenable-in-promise-aggregators': 'off', '@typescript-eslint/unbound-method': 'off', }, }; diff --git a/packages/eslint-plugin/src/configs/recommended-type-checked.ts b/packages/eslint-plugin/src/configs/recommended-type-checked.ts index ab0f50394612..a49fb32a1c87 100644 --- a/packages/eslint-plugin/src/configs/recommended-type-checked.ts +++ b/packages/eslint-plugin/src/configs/recommended-type-checked.ts @@ -47,6 +47,7 @@ export = { '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', + '@typescript-eslint/thenable-in-promise-aggregators': 'error', '@typescript-eslint/triple-slash-reference': 'error', '@typescript-eslint/unbound-method': 'error', }, diff --git a/packages/eslint-plugin/src/configs/strict-type-checked.ts b/packages/eslint-plugin/src/configs/strict-type-checked.ts index a0f82563b1f3..7a90194e3cfe 100644 --- a/packages/eslint-plugin/src/configs/strict-type-checked.ts +++ b/packages/eslint-plugin/src/configs/strict-type-checked.ts @@ -71,6 +71,7 @@ export = { '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', + '@typescript-eslint/thenable-in-promise-aggregators': 'error', '@typescript-eslint/triple-slash-reference': 'error', '@typescript-eslint/unbound-method': 'error', '@typescript-eslint/unified-signatures': 'error', diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index e5a50117abe6..326aac77fd23 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -1,8 +1,11 @@ -import * as tsutils from 'ts-api-utils'; import { isTypeAnyType, isTypeUnknownType, } from '@typescript-eslint/type-utils'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as tsutils from 'ts-api-utils'; + import { createRule, getParserServices } from '../util'; export default createRule({ @@ -31,17 +34,17 @@ export default createRule({ const checker = services.program.getTypeChecker(); return { - CallExpression(node): void { - if (node.callee.type !== 'MemberExpression') { + CallExpression(node: TSESTree.CallExpression): void { + if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { return; } - if (node.callee.object.type !== 'Identifier') { + if (node.callee.object.type !== AST_NODE_TYPES.Identifier) { return; } if (node.callee.object.name !== 'Promise') { return; } - if (node.callee.property.type !== 'Identifier') { + if (node.callee.property.type !== AST_NODE_TYPES.Identifier) { return; } @@ -56,14 +59,14 @@ export default createRule({ } const arg = args[0]; - if (arg.type === 'ArrayExpression') { + if (arg.type === AST_NODE_TYPES.ArrayExpression) { const { elements } = arg; if (elements.length === 0) { return; } for (const element of elements) { - if (element === null) { + if (element == null) { continue; } const elementType = services.getTypeAtLocation(element); diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index ab0e6087df3d..416c877ca4ee 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -1,6 +1,6 @@ import { RuleTester } from '@typescript-eslint/rule-tester'; -import rule from '../../src/rules/await-thenable'; +import rule from '../../src/rules/thenable-in-promise-aggregators'; import { getFixturesRootDir } from '../RuleTester'; const rootDir = getFixturesRootDir(); @@ -84,9 +84,9 @@ async function test() { `, ` async function test() { - await Promise.race([(Math.random() > 0.5 ? numberPromise : 0)]); - await Promise.race([(Math.random() > 0.5 ? foo : 0)]); - await Promise.race([(Math.random() > 0.5 ? bar : 0)]); + await Promise.race([Math.random() > 0.5 ? numberPromise : 0]); + await Promise.race([Math.random() > 0.5 ? foo : 0]); + await Promise.race([Math.random() > 0.5 ? bar : 0]); const intersectionPromise: Promise & number; await Promise.race([intersectionPromise]); @@ -118,7 +118,7 @@ const doSomething = async ( obj3.a?.b.c?.(), obj4.a.b.c?.(), obj5.a?.().b?.c?.(), - obj6?.a.b.c?.() + obj6?.a.b.c?.(), ]); await Promise.allSettled([callback?.()]); @@ -126,26 +126,26 @@ const doSomething = async ( `, ` async function test() { - const promiseArr: Promise[]; - await Promise.all(promiseArr); + const promiseArr: Promise[]; + await Promise.all(promiseArr); } `, ` async function test() { - const intersectionArr: (Promise & number)[]; - await Promise.all(intersectionArr); + const intersectionArr: (Promise & number)[]; + await Promise.all(intersectionArr); } `, ` async function test() { - const values = [1, 2, 3]; - await Promise.all(values.map(value => Promise.resolve(value))); + const values = [1, 2, 3]; + await Promise.all(values.map(value => Promise.resolve(value))); } `, ` async function test() { - const values = [1, 2, 3]; - await Promise.all(values.map(async (value) => {})); + const values = [1, 2, 3]; + await Promise.all(values.map(async value => {})); } `, ], @@ -156,7 +156,7 @@ async function test() { errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -165,7 +165,7 @@ async function test() { errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -174,7 +174,7 @@ async function test() { errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -183,7 +183,7 @@ async function test() { errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -192,16 +192,16 @@ async function test() { errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, { - code: `async () => await Promise.race([Math.random() > 0.5 ? '' : 0]);`, + code: "async () => await Promise.race([Math.random() > 0.5 ? '' : 0]);", errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -213,7 +213,7 @@ await Promise.race([new NonPromise()]); errors: [ { line: 3, - messageIdArrayArg, + messageId: messageIdInArray, suggestions: [], }, ], @@ -232,7 +232,7 @@ async function test() { errors: [ { line: 8, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -244,7 +244,7 @@ await Promise.race([callback?.()]); errors: [ { line: 3, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -256,7 +256,7 @@ await Promise.race([obj.a?.b?.()]); errors: [ { line: 3, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -268,7 +268,7 @@ await Promise.race([obj?.a.b.c?.()]); errors: [ { line: 3, - messageIdArrayArg, + messageId: messageIdInArray, }, ], }, @@ -280,8 +280,8 @@ await Promise.all([wrappedPromise, stdPromise]); `, errors: [ { - line: 3, - messageIdInArray, + line: 4, + messageId: messageIdInArray, }, ], }, @@ -290,7 +290,7 @@ await Promise.all([wrappedPromise, stdPromise]); errors: [ { line: 1, - messageIdNonArrayArg, + messageId: messageIdNonArrayArg, }, ], }, @@ -299,7 +299,7 @@ await Promise.all([wrappedPromise, stdPromise]); errors: [ { line: 1, - messageIdNonArrayArg, + messageId: messageIdNonArrayArg, }, ], }, @@ -308,7 +308,7 @@ await Promise.all([wrappedPromise, stdPromise]); errors: [ { line: 1, - messageIdNonArrayArg, + messageId: messageIdNonArrayArg, }, ], }, @@ -317,7 +317,7 @@ await Promise.all([wrappedPromise, stdPromise]); errors: [ { line: 1, - messageIdNonArrayArg, + messageId: messageIdNonArrayArg, }, ], }, @@ -329,28 +329,28 @@ await Promise.all(promiseArr); errors: [ { line: 3, - messageIdNonArrayArg, + messageId: messageIdNonArrayArg, }, ], }, { - code: 'await Promise.all([0, 1].map((v) => v)', + code: 'await Promise.all([0, 1].map(v => v));', errors: [ { line: 1, - messageIdArrayArg, + messageId: messageIdArrayArg, }, ], }, { code: ` declare const promiseArr: Promise[]; -await Promise.all(promiseArr.map((v) => await v)); +await Promise.all(promiseArr.map(v => await v)); `, errors: [ { line: 3, - messageIdArrayArg, + messageId: messageIdArrayArg, }, ], }, diff --git a/packages/eslint-plugin/tests/schema-snapshots/thenable-in-promise-aggregators.shot b/packages/eslint-plugin/tests/schema-snapshots/thenable-in-promise-aggregators.shot new file mode 100644 index 000000000000..9f966c14d5e2 --- /dev/null +++ b/packages/eslint-plugin/tests/schema-snapshots/thenable-in-promise-aggregators.shot @@ -0,0 +1,14 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Rule schemas should be convertible to TS types for documentation purposes thenable-in-promise-aggregators 1`] = ` +" +# SCHEMA: + +[] + + +# TYPES: + +/** No options declared */ +type Options = [];" +`; From ac00bbfdf0b1f7c0e93f9594b4dcd0dffe334d06 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 05:41:36 -0800 Subject: [PATCH 03/26] Remove accidental linebreak mid-sentence --- .../docs/rules/thenable-in-promise-aggregators.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md index cd4200554ea1..0d18f0dfebe7 100644 --- a/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md +++ b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md @@ -11,8 +11,7 @@ The `await` keyword is generally used to retrieve the result of calling a Thenab When multiple Thenable's are running at the same time, it is sometimes desirable to wait until any one of them resolves (`Promise.race`), all of them resolve or any of them reject (`Promise.all`), or all of them resolve or reject (`Promise.allSettled`). -Each of these functions accept an iterable of promises as input and return a single -Promise. +Each of these functions accept an iterable of promises as input and return a single Promise. If a non-Thenable is passed, it is ignored. While doing so is valid JavaScript, it is often a programmer error, such as forgetting to unwrap a wrapped promise, or using the `await` keyword on the individual promises, which defeats the purpose of using one of these Promise aggregators. From 453b5c3470e63f271c32f59800705bc9912ff52e Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 05:43:15 -0800 Subject: [PATCH 04/26] Remove from recommended --- packages/eslint-plugin/src/configs/recommended-type-checked.ts | 1 - .../eslint-plugin/src/rules/thenable-in-promise-aggregators.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/src/configs/recommended-type-checked.ts b/packages/eslint-plugin/src/configs/recommended-type-checked.ts index a49fb32a1c87..ab0f50394612 100644 --- a/packages/eslint-plugin/src/configs/recommended-type-checked.ts +++ b/packages/eslint-plugin/src/configs/recommended-type-checked.ts @@ -47,7 +47,6 @@ export = { '@typescript-eslint/require-await': 'error', '@typescript-eslint/restrict-plus-operands': 'error', '@typescript-eslint/restrict-template-expressions': 'error', - '@typescript-eslint/thenable-in-promise-aggregators': 'error', '@typescript-eslint/triple-slash-reference': 'error', '@typescript-eslint/unbound-method': 'error', }, diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 326aac77fd23..9de0dba19159 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -14,7 +14,7 @@ export default createRule({ docs: { description: 'Disallow passing non-Thenable values to promise aggregators', - recommended: 'recommended', + recommended: 'strict', requiresTypeChecking: true, }, messages: { From c2a10e250174922db3c09efc4d1a3e7569cf8dca Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 06:58:31 -0800 Subject: [PATCH 05/26] Support Promise aliases --- .../rules/thenable-in-promise-aggregators.ts | 127 ++++++++++++++-- .../thenable-in-promise-aggregators.test.ts | 138 ++++++++++++++++++ 2 files changed, 252 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 9de0dba19159..b1e2d8ac1688 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -5,7 +5,7 @@ import { import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; - +import ts from 'typescript'; import { createRule, getParserServices } from '../util'; export default createRule({ @@ -33,28 +33,129 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); - return { - CallExpression(node: TSESTree.CallExpression): void { - if (node.callee.type !== AST_NODE_TYPES.MemberExpression) { - return; + const aggregateFunctionNames = ['all', 'race', 'allSettled']; + + function skipChainExpression( + node: T, + ): T | TSESTree.ChainElement { + return node.type === AST_NODE_TYPES.ChainExpression + ? node.expression + : node; + } + + function isSymbolFromDefaultLibrary( + program: ts.Program, + symbol: ts.Symbol | undefined, + ): boolean { + if (!symbol) { + return false; + } + + const declarations = symbol.getDeclarations() ?? []; + for (const declaration of declarations) { + const sourceFile = declaration.getSourceFile(); + if (program.isSourceFileDefaultLibrary(sourceFile)) { + return true; } - if (node.callee.object.type !== AST_NODE_TYPES.Identifier) { - return; + } + + return false; + } + + // Is like Promise, or a class/interface extending from Promise + function isPromiseClassLike(program: ts.Program, type: ts.Type): boolean { + if (type.isIntersection()) { + return type.types.some(t => isPromiseLike(program, t)); + } + if (type.isUnion()) { + return type.types.every(t => isPromiseLike(program, t)); + } + + const symbol = type.getSymbol(); + if (!symbol) { + return false; + } + + if ( + symbol.getName() === 'Promise' && + isSymbolFromDefaultLibrary(program, symbol) + ) { + return true; + } + + if (symbol.flags & (ts.SymbolFlags.Class | ts.SymbolFlags.Interface)) { + const checker = program.getTypeChecker(); + + for (const baseType of checker.getBaseTypes(type as ts.InterfaceType)) { + if (isPromiseLike(program, baseType)) { + return true; + } + } + } + + return false; + } + + // Is like PromiseConstructor, or a class/interface extending from Promise + function isPromiseLike(program: ts.Program, type: ts.Type): boolean { + if (type.isIntersection()) { + return type.types.some(t => isPromiseLike(program, t)); + } + if (type.isUnion()) { + return type.types.every(t => isPromiseLike(program, t)); + } + + const symbol = type.getSymbol(); + if (!symbol) { + return false; + } + + if ( + symbol.getName() === 'PromiseConstructor' && + isSymbolFromDefaultLibrary(program, symbol) + ) { + return true; + } + + if (symbol.flags & (ts.SymbolFlags.Class | ts.SymbolFlags.Interface)) { + const checker = program.getTypeChecker(); + + for (const baseType of checker.getBaseTypes(type as ts.InterfaceType)) { + if (isPromiseClassLike(program, baseType)) { + return true; + } } - if (node.callee.object.name !== 'Promise') { + } + + return false; + } + + return { + CallExpression(node: TSESTree.CallExpression): void { + const callee = skipChainExpression(node.callee); + if (callee.type !== AST_NODE_TYPES.MemberExpression) { return; } - if (node.callee.property.type !== AST_NODE_TYPES.Identifier) { + + if (callee.computed) { + if ( + callee.property.type !== AST_NODE_TYPES.Literal || + typeof callee.property.value !== 'string' || + !aggregateFunctionNames.includes(callee.property.value) + ) { + return; + } + } else if (!aggregateFunctionNames.includes(callee.property.name)) { return; } - const { name } = node.callee.property; - if (!['race', 'all', 'allSettled'].includes(name)) { + const callerType = services.getTypeAtLocation(callee.object); + if (!isPromiseLike(services.program, callerType)) { return; } - const { arguments: args } = node; - if (args.length !== 1) { + const args = node.arguments; + if (args.length < 1) { return; } diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 416c877ca4ee..4476de6c4013 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -146,6 +146,60 @@ async function test() { async function test() { const values = [1, 2, 3]; await Promise.all(values.map(async value => {})); +} + `, + ` +async function test() { + const foo = Promise; + await foo.all([Promise.resolve(3)]); +} + `, + ` +async function test() { + const foo = Promise; + await Promise.all([foo.resolve(3)]); +} + `, + ` +async function test() { + class Foo extends Promise {} + await Foo.all([Foo.resolve(3)]); +} + `, + ` +async function test() { + const foo = Promise; + await Promise.all([ + new foo(resolve => { + resolve(); + }), + ]); +} + `, + ` +async function test() { + class Foo extends Promise {} + const myfn = () => + new Foo(resolve => { + resolve(3); + }); + await Promise.all([myfn()]); +} + `, + ` +async function test() { + await Promise.resolve?.([Promise.resolve(3)]); +} + `, + ` +async function test() { + await Promise?.resolve?.([Promise.resolve(3)]); +} + `, + ` +async function test() { + const foo = Promise; + await foo.resolve?.([foo.resolve(3)]); } `, ], @@ -285,6 +339,69 @@ await Promise.all([wrappedPromise, stdPromise]); }, ], }, + { + code: ` +const foo = Promise; +await foo.race([0]); + `, + errors: [ + { + line: 3, + messageId: messageIdInArray, + }, + ], + }, + { + code: ` +class Foo extends Promise {} +await Foo.all([0]); + `, + errors: [ + { + line: 3, + messageId: messageIdInArray, + }, + ], + }, + { + code: ` +const foo = (() => Promise)(); +await foo.all([0]); + `, + errors: [ + { + line: 3, + messageId: messageIdInArray, + }, + ], + }, + { + code: 'await Promise.race?.([0]);', + errors: [ + { + line: 1, + messageId: messageIdInArray, + }, + ], + }, + { + code: 'await Promise?.race?.([0]);', + errors: [ + { + line: 1, + messageId: messageIdInArray, + }, + ], + }, + { + code: 'await Promise?.race?.([0]);', + errors: [ + { + line: 1, + messageId: messageIdInArray, + }, + ], + }, { code: 'await Promise.race(3);', errors: [ @@ -321,6 +438,15 @@ await Promise.all([wrappedPromise, stdPromise]); }, ], }, + { + code: 'await Promise.race?.(undefined);', + errors: [ + { + line: 1, + messageId: messageIdNonArrayArg, + }, + ], + }, { code: ` declare const promiseArr: Promise; @@ -354,5 +480,17 @@ await Promise.all(promiseArr.map(v => await v)); }, ], }, + { + code: ` +declare const arr: number[]; +await Promise.all?.(arr); + `, + errors: [ + { + line: 3, + messageId: messageIdArrayArg, + }, + ], + }, ], }); From e3e2a4adb4507cbf1c7caa657106caaa561aaf49 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:08:35 -0800 Subject: [PATCH 06/26] Support tuple args --- .../rules/thenable-in-promise-aggregators.ts | 50 ++++++++++++++----- .../thenable-in-promise-aggregators.test.ts | 32 ++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index b1e2d8ac1688..225b1f7a63da 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -185,20 +185,15 @@ export default createRule({ node: element, }); } - } else { - const argType = services.getTypeAtLocation(arg); - if (isTypeAnyType(argType) || isTypeUnknownType(argType)) { - return; - } + return; + } - if (!checker.isArrayType(argType)) { - context.report({ - messageId: 'nonArrayArg', - node: arg, - }); - return; - } + const argType = services.getTypeAtLocation(arg); + if (isTypeAnyType(argType) || isTypeUnknownType(argType)) { + return; + } + if (checker.isArrayType(argType)) { if (argType.typeArguments === undefined) { return; } @@ -221,7 +216,38 @@ export default createRule({ messageId: 'arrayArg', node: arg, }); + return; + } + + if (checker.isTupleType(argType)) { + if (argType.typeArguments === undefined) { + return; + } + + const originalNode = services.esTreeNodeToTSNodeMap.get(arg); + for (const typeArg of argType.typeArguments) { + if (isTypeAnyType(typeArg) || isTypeUnknownType(typeArg)) { + continue; + } + + if (tsutils.isThenableType(checker, originalNode, typeArg)) { + continue; + } + + context.report({ + messageId: 'arrayArg', + node: arg, + }); + return; + } + return; } + + context.report({ + messageId: 'nonArrayArg', + node: arg, + }); + return; }, }; }, diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 4476de6c4013..85240f37a1e2 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -200,6 +200,17 @@ async function test() { async function test() { const foo = Promise; await foo.resolve?.([foo.resolve(3)]); +} + `, + ` +async function test() { + const promisesTuple: [Promise] = [Promise.resolve(3)]; + await Promise.all(promisesTuple); +} + `, + ` +async function test() { + await Promise.all([Promise.resolve(6)] as const); } `, ], @@ -492,5 +503,26 @@ await Promise.all?.(arr); }, ], }, + { + code: ` +declare const foo: [number]; +await Promise.race(foo); + `, + errors: [ + { + line: 3, + messageId: messageIdArrayArg, + }, + ], + }, + { + code: 'await Promise.race([0] as const);', + errors: [ + { + line: 1, + messageId: messageIdArrayArg, + }, + ], + }, ], }); From 056dfe160ca989dc7e1b5593580faa2b266cde32 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:23:25 -0800 Subject: [PATCH 07/26] Handle empty arrays / omitted elements in array --- .../rules/thenable-in-promise-aggregators.ts | 6 +++++- .../thenable-in-promise-aggregators.test.ts | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 225b1f7a63da..65de05e0fb56 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -168,7 +168,11 @@ export default createRule({ for (const element of elements) { if (element == null) { - continue; + context.report({ + messageId: 'inArray', + node: arg, + }); + return; } const elementType = services.getTypeAtLocation(element); if (isTypeAnyType(elementType) || isTypeUnknownType(elementType)) { diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 85240f37a1e2..268ccdd78ff6 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -35,6 +35,11 @@ async function test() { } `, ` +async function test() { + await Promise.all([]); +} + `, + ` async function test() { await Promise.race([(async () => true)()]); } @@ -211,6 +216,12 @@ async function test() { ` async function test() { await Promise.all([Promise.resolve(6)] as const); +} + `, + ` +async function test() { + const foo = Array(); + await Promise.all(foo); } `, ], @@ -413,6 +424,15 @@ await foo.all([0]); }, ], }, + { + code: 'await Promise.all([,]);', + errors: [ + { + line: 1, + messageId: messageIdInArray, + }, + ], + }, { code: 'await Promise.race(3);', errors: [ From 03c4fc9243b6d12960f9a61f17284d3f4e24a686 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:23:52 -0800 Subject: [PATCH 08/26] Assert impossible missing type args --- .../src/rules/thenable-in-promise-aggregators.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 65de05e0fb56..9b3453363127 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -198,12 +198,11 @@ export default createRule({ } if (checker.isArrayType(argType)) { - if (argType.typeArguments === undefined) { - return; - } - - if (argType.typeArguments.length < 1) { - return; + if ( + argType.typeArguments == null || + argType.typeArguments.length < 1 + ) { + throw new Error('Expected to find type arguments for an array.'); } const typeArg = argType.typeArguments[0]; From 00909d3d1dbb22b73c91bee8b443d179e67e6085 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:26:35 -0800 Subject: [PATCH 09/26] Test array of any/unknown in valid --- .../rules/thenable-in-promise-aggregators.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 268ccdd78ff6..a7baac0a80f7 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -222,6 +222,18 @@ async function test() { async function test() { const foo = Array(); await Promise.all(foo); +} + `, + ` +async function test() { + const arrOfAny: any[] = []; + await Promise.all(arrOfAny) +} + `, + ` +async function test() { + const arrOfUnknown: unknown[] = []; + await Promise.all(arrOfAny) } `, ], From da403fff52a49d4bb11b56467783926b33c999a8 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:28:50 -0800 Subject: [PATCH 10/26] Remove unnecessary suggestions line --- .../tests/rules/thenable-in-promise-aggregators.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index a7baac0a80f7..c016f5985983 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -302,7 +302,6 @@ await Promise.race([new NonPromise()]); { line: 3, messageId: messageIdInArray, - suggestions: [], }, ], }, From fada2335f4d6e27d22833026e06d9206c96e4eeb Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:33:16 -0800 Subject: [PATCH 11/26] yarn lint --fix --- .../src/rules/thenable-in-promise-aggregators.ts | 3 ++- .../tests/rules/thenable-in-promise-aggregators.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 9b3453363127..2b72c95cfdd0 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -5,7 +5,8 @@ import { import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; -import ts from 'typescript'; +import * as ts from 'typescript'; + import { createRule, getParserServices } from '../util'; export default createRule({ diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index c016f5985983..390de6b687ea 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -227,13 +227,13 @@ async function test() { ` async function test() { const arrOfAny: any[] = []; - await Promise.all(arrOfAny) + await Promise.all(arrOfAny); } `, ` async function test() { const arrOfUnknown: unknown[] = []; - await Promise.all(arrOfAny) + await Promise.all(arrOfAny); } `, ], @@ -418,7 +418,7 @@ await foo.all([0]); ], }, { - code: 'await Promise?.race?.([0]);', + code: 'await Promise?.race([0]);', errors: [ { line: 1, From d918f42e7c2ceb5fadf2c351fab7eafed8722529 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:34:37 -0800 Subject: [PATCH 12/26] Remove unnecessary aliasing on messageId --- .../thenable-in-promise-aggregators.test.ts | 63 +++++++++---------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 390de6b687ea..841a1f28f102 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -4,9 +4,6 @@ import rule from '../../src/rules/thenable-in-promise-aggregators'; import { getFixturesRootDir } from '../RuleTester'; const rootDir = getFixturesRootDir(); -const messageIdInArray = 'inArray'; -const messageIdArrayArg = 'arrayArg'; -const messageIdNonArrayArg = 'nonArrayArg'; const ruleTester = new RuleTester({ parserOptions: { @@ -244,7 +241,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -253,7 +250,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -262,7 +259,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -271,7 +268,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -280,7 +277,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -289,7 +286,7 @@ async function test() { errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -301,7 +298,7 @@ await Promise.race([new NonPromise()]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -319,7 +316,7 @@ async function test() { errors: [ { line: 8, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -331,7 +328,7 @@ await Promise.race([callback?.()]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -343,7 +340,7 @@ await Promise.race([obj.a?.b?.()]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -355,7 +352,7 @@ await Promise.race([obj?.a.b.c?.()]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -368,7 +365,7 @@ await Promise.all([wrappedPromise, stdPromise]); errors: [ { line: 4, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -380,7 +377,7 @@ await foo.race([0]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -392,7 +389,7 @@ await Foo.all([0]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -404,7 +401,7 @@ await foo.all([0]); errors: [ { line: 3, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -413,7 +410,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -422,7 +419,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -431,7 +428,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -440,7 +437,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdInArray, + messageId: 'inArray', }, ], }, @@ -449,7 +446,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -458,7 +455,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -467,7 +464,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -476,7 +473,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -485,7 +482,7 @@ await foo.all([0]); errors: [ { line: 1, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -497,7 +494,7 @@ await Promise.all(promiseArr); errors: [ { line: 3, - messageId: messageIdNonArrayArg, + messageId: 'nonArrayArg', }, ], }, @@ -506,7 +503,7 @@ await Promise.all(promiseArr); errors: [ { line: 1, - messageId: messageIdArrayArg, + messageId: 'arrayArg', }, ], }, @@ -518,7 +515,7 @@ await Promise.all(promiseArr.map(v => await v)); errors: [ { line: 3, - messageId: messageIdArrayArg, + messageId: 'arrayArg', }, ], }, @@ -530,7 +527,7 @@ await Promise.all?.(arr); errors: [ { line: 3, - messageId: messageIdArrayArg, + messageId: 'arrayArg', }, ], }, @@ -542,7 +539,7 @@ await Promise.race(foo); errors: [ { line: 3, - messageId: messageIdArrayArg, + messageId: 'arrayArg', }, ], }, @@ -551,7 +548,7 @@ await Promise.race(foo); errors: [ { line: 1, - messageId: messageIdArrayArg, + messageId: 'arrayArg', }, ], }, From 9859e6d0eefaf3a89ded9f5569c9554ff689ac40 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:48:09 -0800 Subject: [PATCH 13/26] Split dual-purpose test cases --- .../thenable-in-promise-aggregators.test.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 841a1f28f102..abcd5a7ee8c9 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -78,7 +78,11 @@ async function test() { class Foo extends Promise {} const foo: Foo = Foo.resolve(2); await Promise.race([foo]); - +} + `, + ` +async function test() { + class Foo extends Promise {} class Bar extends Foo {} const bar: Bar = Bar.resolve(2); await Promise.race([bar]); @@ -86,10 +90,11 @@ async function test() { `, ` async function test() { - await Promise.race([Math.random() > 0.5 ? numberPromise : 0]); - await Promise.race([Math.random() > 0.5 ? foo : 0]); - await Promise.race([Math.random() > 0.5 ? bar : 0]); - + await Promise.race([Math.random() > 0.5 ? nonExistentSymbol : 0]); +} + `, + ` +async function test() { const intersectionPromise: Promise & number; await Promise.race([intersectionPromise]); } @@ -121,9 +126,8 @@ const doSomething = async ( obj4.a.b.c?.(), obj5.a?.().b?.c?.(), obj6?.a.b.c?.(), + callback(), ]); - - await Promise.allSettled([callback?.()]); }; `, ` From 2cfbf769b32ea93eb1a3faf39c00d75ddd0bb3eb Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 07:54:19 -0800 Subject: [PATCH 14/26] Tests for [] syntax instead of dot syntax --- .../thenable-in-promise-aggregators.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index abcd5a7ee8c9..eb66d4cfd1f6 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -37,6 +37,16 @@ async function test() { } `, ` +async function test() { + await Promise['all']([Promise.resolve(3)]); +} + `, + ` +async function test() { + await Promise.all([Promise['resolve'](3)]); +} + `, + ` async function test() { await Promise.race([(async () => true)()]); } @@ -445,6 +455,33 @@ await foo.all([0]); }, ], }, + { + code: "await Promise['all']([3]);", + errors: [ + { + line: 1, + messageId: 'inArray', + }, + ], + }, + { + code: "await Promise['race']([3]);", + errors: [ + { + line: 1, + messageId: 'inArray', + }, + ], + }, + { + code: "await Promise['allSettled']([3]);", + errors: [ + { + line: 1, + messageId: 'inArray', + }, + ], + }, { code: 'await Promise.race(3);', errors: [ @@ -490,6 +527,15 @@ await foo.all([0]); }, ], }, + { + code: "await Promise['all'](3);", + errors: [ + { + line: 1, + messageId: 'nonArrayArg', + }, + ], + }, { code: ` declare const promiseArr: Promise; @@ -556,5 +602,14 @@ await Promise.race(foo); }, ], }, + { + code: "await Promise['all']([0, 1].map(v => v));", + errors: [ + { + line: 1, + messageId: 'arrayArg', + }, + ], + }, ], }); From 3e0e4422f23410a58c8fc4555a19d7f7b98b6e2e Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 08:07:35 -0800 Subject: [PATCH 15/26] Explicit tests for union types --- .../thenable-in-promise-aggregators.test.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index eb66d4cfd1f6..3c2a99b20928 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -110,6 +110,12 @@ async function test() { } `, ` +async function test() { + const unionPromise: Promise | number; + await Promise.race([unionPromise]); +} + `, + ` async function test() { class Thenable { then(callback: () => {}) {} @@ -245,6 +251,18 @@ async function test() { async function test() { const arrOfUnknown: unknown[] = []; await Promise.all(arrOfAny); +} + `, + ` +async function test() { + const arrOfIntersection: (Promise & number)[] = []; + await Promise.all(arrOfIntersection); +} + `, + ` +async function test() { + const arrOfUnion: (Promise | number)[] = []; + await Promise.all(arrOfUnion); } `, ], @@ -482,6 +500,18 @@ await foo.all([0]); }, ], }, + { + code: ` +declare const badUnion: number | string; +await Promise.all([badUnion]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, { code: 'await Promise.race(3);', errors: [ @@ -611,5 +641,17 @@ await Promise.race(foo); }, ], }, + { + code: ` +declare const badUnionArr: (number | string)[]; +await Promise.all(badUnionArr); + `, + errors: [ + { + line: 3, + messageId: 'arrayArg', + }, + ], + }, ], }); From 20e15459224c964f8b5f38faade6dde81a118415 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 08:41:20 -0800 Subject: [PATCH 16/26] Use shorter valid testcase syntax --- .../thenable-in-promise-aggregators.test.ts | 250 ++++++------------ 1 file changed, 77 insertions(+), 173 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 3c2a99b20928..c2fbc20684d8 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -16,114 +16,62 @@ const ruleTester = new RuleTester({ ruleTester.run('thenable-in-promise-aggregators', rule, { valid: [ + 'await Promise.race([Promise.resolve(3)]);', + 'await Promise.all([Promise.resolve(3)]);', + 'await Promise.allSettled([Promise.resolve(3)]);', + 'await Promise.all([]);', + "await Promise['all']([Promise.resolve(3)]);", + "await Promise.all([Promise['resolve'](3)]);", + 'await Promise.race([(async () => true)()]);', ` -async function test() { - await Promise.race([Promise.resolve(3)]); -} - `, - ` -async function test() { - await Promise.all([Promise.resolve(3)]); -} - `, - ` -async function test() { - await Promise.allSettled([Promise.resolve(3)]); -} - `, - ` -async function test() { - await Promise.all([]); -} - `, - ` -async function test() { - await Promise['all']([Promise.resolve(3)]); +function returnsPromise() { + return Promise.resolve('value'); } +await Promise.race([returnsPromise()]); `, ` -async function test() { - await Promise.all([Promise['resolve'](3)]); -} +async function returnsPromiseAsync() {} +await Promise.race([returnsPromiseAsync()]); `, ` -async function test() { - await Promise.race([(async () => true)()]); -} +declare const anyValue: any; +await Promise.race([anyValue]); `, ` -async function test() { - function returnsPromise() { - return Promise.resolve('value'); - } - await Promise.race([returnsPromise()]); -} +declare const unknownValue: unknown; +await Promise.race([unknownValue]); `, ` -async function test() { - async function returnsPromiseAsync() {} - await Promise.race([returnsPromiseAsync()]); -} +declare const numberPromise: Promise; +await Promise.race([numberPromise]); `, ` -async function test() { - let anyValue: any; - await Promise.race([anyValue]); -} - `, - ` -async function test() { - let unknownValue: unknown; - await Promise.race([unknownValue]); -} - `, - ` -async function test() { - const numberPromise: Promise; - await Promise.race([numberPromise]); -} - `, - ` -async function test() { - class Foo extends Promise {} - const foo: Foo = Foo.resolve(2); - await Promise.race([foo]); -} +class Foo extends Promise {} +const foo: Foo = Foo.resolve(2); +await Promise.race([foo]); `, ` -async function test() { - class Foo extends Promise {} - class Bar extends Foo {} - const bar: Bar = Bar.resolve(2); - await Promise.race([bar]); -} +class Foo extends Promise {} +class Bar extends Foo {} +const bar: Bar = Bar.resolve(2); +await Promise.race([bar]); `, + 'await Promise.race([Math.random() > 0.5 ? nonExistentSymbol : 0]);', ` -async function test() { - await Promise.race([Math.random() > 0.5 ? nonExistentSymbol : 0]); -} +declare const intersectionPromise: Promise & number; +await Promise.race([intersectionPromise]); `, ` -async function test() { - const intersectionPromise: Promise & number; - await Promise.race([intersectionPromise]); -} +declare const unionPromise: Promise | number; +await Promise.race([unionPromise]); `, ` -async function test() { - const unionPromise: Promise | number; - await Promise.race([unionPromise]); +class Thenable { + then(callback: () => {}) {} } - `, - ` -async function test() { - class Thenable { - then(callback: () => {}) {} - } - const thenable = new Thenable(); - await Promise.race([thenable]); -} +const thenable = new Thenable(); +await Promise.race([thenable]); `, ` const doSomething = async ( @@ -147,123 +95,79 @@ const doSomething = async ( }; `, ` -async function test() { - const promiseArr: Promise[]; - await Promise.all(promiseArr); -} - `, - ` -async function test() { - const intersectionArr: (Promise & number)[]; - await Promise.all(intersectionArr); -} - `, - ` -async function test() { - const values = [1, 2, 3]; - await Promise.all(values.map(value => Promise.resolve(value))); -} - `, - ` -async function test() { - const values = [1, 2, 3]; - await Promise.all(values.map(async value => {})); -} +declare const promiseArr: Promise[]; +await Promise.all(promiseArr); `, ` -async function test() { - const foo = Promise; - await foo.all([Promise.resolve(3)]); -} +declare const intersectionArr: (Promise & number)[]; +await Promise.all(intersectionArr); `, ` -async function test() { - const foo = Promise; - await Promise.all([foo.resolve(3)]); -} +const values = [1, 2, 3]; +await Promise.all(values.map(value => Promise.resolve(value))); `, ` -async function test() { - class Foo extends Promise {} - await Foo.all([Foo.resolve(3)]); -} +const values = [1, 2, 3]; +await Promise.all(values.map(async value => {})); `, ` -async function test() { - const foo = Promise; - await Promise.all([ - new foo(resolve => { - resolve(); - }), - ]); -} +const foo = Promise; +await foo.all([Promise.resolve(3)]); `, ` -async function test() { - class Foo extends Promise {} - const myfn = () => - new Foo(resolve => { - resolve(3); - }); - await Promise.all([myfn()]); -} +const foo = Promise; +await Promise.all([foo.resolve(3)]); `, ` -async function test() { - await Promise.resolve?.([Promise.resolve(3)]); -} +class Foo extends Promise {} +await Foo.all([Foo.resolve(3)]); `, ` -async function test() { - await Promise?.resolve?.([Promise.resolve(3)]); -} +const foo = Promise; +await Promise.all([ + new foo(resolve => { + resolve(); + }), +]); `, ` -async function test() { - const foo = Promise; - await foo.resolve?.([foo.resolve(3)]); -} +class Foo extends Promise {} +const myfn = () => + new Foo(resolve => { + resolve(3); + }); +await Promise.all([myfn()]); `, + 'await Promise.resolve?.([Promise.resolve(3)]);', + 'await Promise?.resolve?.([Promise.resolve(3)]);', ` -async function test() { - const promisesTuple: [Promise] = [Promise.resolve(3)]; - await Promise.all(promisesTuple); -} +const foo = Promise; +await foo.resolve?.([foo.resolve(3)]); `, ` -async function test() { - await Promise.all([Promise.resolve(6)] as const); -} +const promisesTuple: [Promise] = [Promise.resolve(3)]; +await Promise.all(promisesTuple); `, + 'await Promise.all([Promise.resolve(6)] as const);', ` -async function test() { - const foo = Array(); - await Promise.all(foo); -} +const foo = Array(); +await Promise.all(foo); `, ` -async function test() { - const arrOfAny: any[] = []; - await Promise.all(arrOfAny); -} +declare const arrOfAny: any[]; +await Promise.all(arrOfAny); `, ` -async function test() { - const arrOfUnknown: unknown[] = []; - await Promise.all(arrOfAny); -} +declare const arrOfUnknown: unknown[] = []; +await Promise.all(arrOfAny); `, ` -async function test() { - const arrOfIntersection: (Promise & number)[] = []; - await Promise.all(arrOfIntersection); -} +declare const arrOfIntersection: (Promise & number)[] = []; +await Promise.all(arrOfIntersection); `, ` -async function test() { - const arrOfUnion: (Promise | number)[] = []; - await Promise.all(arrOfUnion); -} +declare const arrOfUnion: (Promise | number)[] = []; +await Promise.all(arrOfUnion); `, ], From 7a3cc01d7275d046cffcc762ddbb555215266b8c Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 20 Dec 2023 08:43:09 -0800 Subject: [PATCH 17/26] Fix including = in declare const --- .../tests/rules/thenable-in-promise-aggregators.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index c2fbc20684d8..ac99cee97c7f 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -158,15 +158,15 @@ declare const arrOfAny: any[]; await Promise.all(arrOfAny); `, ` -declare const arrOfUnknown: unknown[] = []; +declare const arrOfUnknown: unknown[]; await Promise.all(arrOfAny); `, ` -declare const arrOfIntersection: (Promise & number)[] = []; +declare const arrOfIntersection: (Promise & number)[]; await Promise.all(arrOfIntersection); `, ` -declare const arrOfUnion: (Promise | number)[] = []; +declare const arrOfUnion: (Promise | number)[]; await Promise.all(arrOfUnion); `, ], From 304356e5d270f491b5b54ab177b35d4d028a853c Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Sun, 24 Dec 2023 05:34:57 -0800 Subject: [PATCH 18/26] Use latest helpers from #8011 --- .../rules/thenable-in-promise-aggregators.ts | 63 ++++++++----------- 1 file changed, 25 insertions(+), 38 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 2b72c95cfdd0..04271845793c 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -63,47 +63,31 @@ export default createRule({ return false; } - // Is like Promise, or a class/interface extending from Promise - function isPromiseClassLike(program: ts.Program, type: ts.Type): boolean { - if (type.isIntersection()) { - return type.types.some(t => isPromiseLike(program, t)); - } - if (type.isUnion()) { - return type.types.every(t => isPromiseLike(program, t)); - } - - const symbol = type.getSymbol(); - if (!symbol) { - return false; - } - - if ( - symbol.getName() === 'Promise' && - isSymbolFromDefaultLibrary(program, symbol) - ) { - return true; - } - - if (symbol.flags & (ts.SymbolFlags.Class | ts.SymbolFlags.Interface)) { - const checker = program.getTypeChecker(); - - for (const baseType of checker.getBaseTypes(type as ts.InterfaceType)) { - if (isPromiseLike(program, baseType)) { - return true; - } - } - } + function isPromiseLike(program: ts.Program, type: ts.Type): boolean { + return isBuiltinSymbolLike(program, type, 'Promise'); + } - return false; + function isPromiseConstructorLike( + program: ts.Program, + type: ts.Type, + ): boolean { + return isBuiltinSymbolLike(program, type, 'PromiseConstructor'); } - // Is like PromiseConstructor, or a class/interface extending from Promise - function isPromiseLike(program: ts.Program, type: ts.Type): boolean { + function isBuiltinSymbolLike( + program: ts.Program, + type: ts.Type, + symbolName: string, + ): boolean { if (type.isIntersection()) { - return type.types.some(t => isPromiseLike(program, t)); + return type.types.some(t => + isBuiltinSymbolLike(program, t, symbolName), + ); } if (type.isUnion()) { - return type.types.every(t => isPromiseLike(program, t)); + return type.types.every(t => + isBuiltinSymbolLike(program, t, symbolName), + ); } const symbol = type.getSymbol(); @@ -112,7 +96,7 @@ export default createRule({ } if ( - symbol.getName() === 'PromiseConstructor' && + symbol.getName() === symbolName && isSymbolFromDefaultLibrary(program, symbol) ) { return true; @@ -122,7 +106,7 @@ export default createRule({ const checker = program.getTypeChecker(); for (const baseType of checker.getBaseTypes(type as ts.InterfaceType)) { - if (isPromiseClassLike(program, baseType)) { + if (isBuiltinSymbolLike(program, baseType, symbolName)) { return true; } } @@ -151,7 +135,10 @@ export default createRule({ } const callerType = services.getTypeAtLocation(callee.object); - if (!isPromiseLike(services.program, callerType)) { + if ( + !isPromiseConstructorLike(services.program, callerType) && + !isPromiseLike(services.program, callerType) + ) { return; } From d58111dc288aa01972851f6c7c53cbc1b9f97749 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Sun, 24 Dec 2023 06:37:00 -0800 Subject: [PATCH 19/26] Carefully handle complicated promise-likes, remove nonArrayArg --- .../rules/thenable-in-promise-aggregators.ts | 106 ++++----- .../thenable-in-promise-aggregators.test.ts | 206 +++++++++++++++--- 2 files changed, 226 insertions(+), 86 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 04271845793c..ad976bbde06f 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -23,7 +23,6 @@ export default createRule({ 'Unexpected non-Thenable value in array passed to promise aggregator.', arrayArg: 'Unexpected array of non-Thenable values passed to promise aggregator.', - nonArrayArg: 'Unexpected non-array passed to promise aggregator.', }, schema: [], type: 'problem', @@ -115,6 +114,52 @@ export default createRule({ return false; } + function isPartiallyLikeType( + type: ts.Type, + predicate: (type: ts.Type) => boolean, + ): boolean { + if (isTypeAnyType(type) || isTypeUnknownType(type)) { + return true; + } + if (type.isIntersection() || type.isUnion()) { + return type.types.some(t => isPartiallyLikeType(t, predicate)); + } + return predicate(type); + } + + function isIndexableWithSomeElementsLike( + type: ts.Type, + predicate: (type: ts.Type) => boolean, + ): boolean { + if (isTypeAnyType(type) || isTypeUnknownType(type)) { + return true; + } + + if (type.isIntersection() || type.isUnion()) { + return type.types.some(t => + isIndexableWithSomeElementsLike(t, predicate), + ); + } + + if (!checker.isArrayType(type) && !checker.isTupleType(type)) { + const indexType = checker.getIndexTypeOfType(type, ts.IndexKind.Number); + if (indexType === undefined) { + return false; + } + + return isPartiallyLikeType(indexType, predicate); + } + + const typeArgs = type.typeArguments; + if (typeArgs === undefined) { + throw new Error( + 'Expected to find type arguments for an array or tuple.', + ); + } + + return typeArgs.some(t => isPartiallyLikeType(t, predicate)); + } + return { CallExpression(node: TSESTree.CallExpression): void { const callee = skipChainExpression(node.callee); @@ -181,64 +226,19 @@ export default createRule({ } const argType = services.getTypeAtLocation(arg); - if (isTypeAnyType(argType) || isTypeUnknownType(argType)) { - return; - } - - if (checker.isArrayType(argType)) { - if ( - argType.typeArguments == null || - argType.typeArguments.length < 1 - ) { - throw new Error('Expected to find type arguments for an array.'); - } - - const typeArg = argType.typeArguments[0]; - if (isTypeAnyType(typeArg) || isTypeUnknownType(typeArg)) { - return; - } - - const originalNode = services.esTreeNodeToTSNodeMap.get(arg); - if (tsutils.isThenableType(checker, originalNode, typeArg)) { - return; - } - - context.report({ - messageId: 'arrayArg', - node: arg, - }); - return; - } - - if (checker.isTupleType(argType)) { - if (argType.typeArguments === undefined) { - return; - } - - const originalNode = services.esTreeNodeToTSNodeMap.get(arg); - for (const typeArg of argType.typeArguments) { - if (isTypeAnyType(typeArg) || isTypeUnknownType(typeArg)) { - continue; - } - - if (tsutils.isThenableType(checker, originalNode, typeArg)) { - continue; - } - - context.report({ - messageId: 'arrayArg', - node: arg, - }); - return; - } + const originalNode = services.esTreeNodeToTSNodeMap.get(arg); + if ( + isIndexableWithSomeElementsLike(argType, elementType => { + return tsutils.isThenableType(checker, originalNode, elementType); + }) + ) { return; } context.report({ - messageId: 'nonArrayArg', + messageId: 'arrayArg', node: arg, }); - return; }, }; }, diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index ac99cee97c7f..2f59f095731e 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -169,6 +169,94 @@ await Promise.all(arrOfIntersection); declare const arrOfUnion: (Promise | number)[]; await Promise.all(arrOfUnion); `, + ` +declare const unionOfArr: Promise[] | Promise[]; +await Promise.all(unionOfArr); + `, + ` +declare const unionOfTuple: [Promise] | [Promise]; +await Promise.all(unionOfTuple); + `, + ` +declare const intersectionOfArr: Promise[] & Promise[]; +await Promise.all(intersectionOfArr); + `, + ` +declare const intersectionOfTuple: [Promise] & [Promise]; +await Promise.all(intersectionOfTuple); + `, + ` +declare const readonlyArr: ReadonlyArray>; +await Promise.all(readonlyArr); + `, + ` +declare const unionOfPromiseArrAndArr: Promise[] | number[]; +await Promise.all(unionOfPromiseArrAndArr); + `, + ` +declare const readonlyTuple: readonly [Promise]; +await Promise.all(readonlyTuple); + `, + ` +declare const readonlyTupleWithOneValid: readonly [number, Promise]; +await Promise.all(readonlyTupleWithOneValid); + `, + ` +declare const unionOfReadonlyTuples: + | readonly [number] + | readonly [Promise]; +await Promise.all(unionOfReadonlyTuples); + `, + ` +declare const readonlyTupleOfUnion: readonly [Promise | number]; +await Promise.all(readonlyTupleOfUnion); + `, + ` +class Foo extends Array> {} +declare const foo: Foo; +await Promise.all(foo); + `, + ` +class Foo extends Array {} +declare const foo: Foo; +await Promise.all(foo); + `, + ` +class Foo extends Array {} +declare const foo: Foo; +await Promise.all(foo); + `, + ` +class Foo extends Array {} +declare const foo: Foo; +await Promise.all(foo); + `, + ` +class Foo extends Array> {} +declare const foo: Foo; +await Promise.all(foo); + `, + ` +type Foo = { new (): ReadonlyArray> }; +declare const foo: Foo; +class Baz extends foo {} +declare const baz: Baz; +await Promise.all(baz); + `, + ` +type Foo = { new (): [Promise] }; +declare const foo: Foo; +class Baz extends foo {} +declare const baz: Baz; +await Promise.all(baz); + `, + ` +type Foo = { new (): [number | Promise] }; +declare const foo: Foo; +class Baz extends foo {} +declare const baz: Baz; +await Promise.all(baz); + `, ], invalid: [ @@ -417,84 +505,96 @@ await Promise.all([badUnion]); ], }, { - code: 'await Promise.race(3);', + code: 'await Promise.all([0, 1].map(v => v));', errors: [ { line: 1, - messageId: 'nonArrayArg', + messageId: 'arrayArg', }, ], }, { - code: 'await Promise.all(3);', + code: ` +declare const promiseArr: Promise[]; +await Promise.all(promiseArr.map(v => await v)); + `, errors: [ { - line: 1, - messageId: 'nonArrayArg', + line: 3, + messageId: 'arrayArg', }, ], }, { - code: 'await Promise.allSettled({ foo: 3 });', + code: ` +declare const arr: number[]; +await Promise.all?.(arr); + `, errors: [ { - line: 1, - messageId: 'nonArrayArg', + line: 3, + messageId: 'arrayArg', }, ], }, { - code: 'await Promise.race(undefined);', + code: ` +declare const foo: [number]; +await Promise.race(foo); + `, errors: [ { - line: 1, - messageId: 'nonArrayArg', + line: 3, + messageId: 'arrayArg', }, ], }, { - code: 'await Promise.race?.(undefined);', + code: 'await Promise.race([0] as const);', errors: [ { line: 1, - messageId: 'nonArrayArg', + messageId: 'arrayArg', }, ], }, { - code: "await Promise['all'](3);", + code: "await Promise['all']([0, 1].map(v => v));", errors: [ { line: 1, - messageId: 'nonArrayArg', + messageId: 'arrayArg', }, ], }, { code: ` -declare const promiseArr: Promise; -await Promise.all(promiseArr); +declare const badUnionArr: (number | string)[]; +await Promise.all(badUnionArr); `, errors: [ { line: 3, - messageId: 'nonArrayArg', + messageId: 'arrayArg', }, ], }, { - code: 'await Promise.all([0, 1].map(v => v));', + code: ` +declare const badArrUnion: number[] | string[]; +await Promise.all(badArrUnion); + `, errors: [ { - line: 1, + line: 3, messageId: 'arrayArg', }, ], }, { code: ` -declare const promiseArr: Promise[]; -await Promise.all(promiseArr.map(v => await v)); +declare const badReadonlyArr: ReadonlyArray; +await Promise.all(badReadonlyArr); `, errors: [ { @@ -505,8 +605,8 @@ await Promise.all(promiseArr.map(v => await v)); }, { code: ` -declare const arr: number[]; -await Promise.all?.(arr); +declare const badArrIntersection: number[] & string[]; +await Promise.all(badArrIntersection); `, errors: [ { @@ -517,8 +617,8 @@ await Promise.all?.(arr); }, { code: ` -declare const foo: [number]; -await Promise.race(foo); +declare const badReadonlyTuple: readonly [number, string]; +await Promise.all(badReadonlyTuple); `, errors: [ { @@ -528,31 +628,71 @@ await Promise.race(foo); ], }, { - code: 'await Promise.race([0] as const);', + code: ` +class Foo extends Array {} +declare const foo: Foo; +await Promise.all(foo); + `, errors: [ { - line: 1, + line: 4, messageId: 'arrayArg', }, ], }, { - code: "await Promise['all']([0, 1].map(v => v));", + code: ` +class Foo extends Array {} +declare const foo: Foo; +await Promise.all(foo); + `, errors: [ { - line: 1, + line: 4, messageId: 'arrayArg', }, ], }, { code: ` -declare const badUnionArr: (number | string)[]; -await Promise.all(badUnionArr); +type Foo = [number]; +declare const foo: Foo; +await Promise.all(foo); `, errors: [ { - line: 3, + line: 4, + messageId: 'arrayArg', + }, + ], + }, + { + code: ` +class Bar {} +type Foo = { new (): Bar & [number] }; +declare const foo: Foo; +class Baz extends foo {} +declare const baz: Baz; +await Promise.all(baz); + `, + errors: [ + { + line: 7, + messageId: 'arrayArg', + }, + ], + }, + { + code: ` +type Foo = { new (): ReadonlyArray }; +declare const foo: Foo; +class Baz extends foo {} +declare const baz: Baz; +await Promise.all(baz); + `, + errors: [ + { + line: 6, messageId: 'arrayArg', }, ], From 609db97e91fb60bafa59acaed1f1fc9774f3e2c9 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Sun, 24 Dec 2023 06:38:22 -0800 Subject: [PATCH 20/26] Rename callerType calleeType for consistency --- .../src/rules/thenable-in-promise-aggregators.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index ad976bbde06f..4218e67fd290 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -179,10 +179,10 @@ export default createRule({ return; } - const callerType = services.getTypeAtLocation(callee.object); + const calleeType = services.getTypeAtLocation(callee.object); if ( - !isPromiseConstructorLike(services.program, callerType) && - !isPromiseLike(services.program, callerType) + !isPromiseConstructorLike(services.program, calleeType) && + !isPromiseLike(services.program, calleeType) ) { return; } From 698e57992e0e50d232f91a79f6dad349f190fa3f Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Sun, 24 Dec 2023 06:53:10 -0800 Subject: [PATCH 21/26] Add any to list of promise aggregators --- .../src/rules/thenable-in-promise-aggregators.ts | 2 +- .../rules/thenable-in-promise-aggregators.test.ts | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 4218e67fd290..7a7a6ad16d9c 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -33,7 +33,7 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); - const aggregateFunctionNames = ['all', 'race', 'allSettled']; + const aggregateFunctionNames = ['all', 'race', 'allSettled', 'any']; function skipChainExpression( node: T, diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 2f59f095731e..4461309d7079 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -19,6 +19,7 @@ ruleTester.run('thenable-in-promise-aggregators', rule, { 'await Promise.race([Promise.resolve(3)]);', 'await Promise.all([Promise.resolve(3)]);', 'await Promise.allSettled([Promise.resolve(3)]);', + 'await Promise.any([Promise.resolve(3)]);', 'await Promise.all([]);', "await Promise['all']([Promise.resolve(3)]);", "await Promise.all([Promise['resolve'](3)]);", @@ -287,6 +288,15 @@ await Promise.all(baz); }, ], }, + { + code: 'await Promise.any([0]);', + errors: [ + { + line: 1, + messageId: 'inArray', + }, + ], + }, { code: 'await Promise.race([Promise.resolve(3), 0]);', errors: [ From 4028718a29dddb75bbf66ddcf9bb17a8fd46bf1f Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 10 Jan 2024 05:37:25 -0800 Subject: [PATCH 22/26] Update to pull in type util functions --- .../rules/thenable-in-promise-aggregators.ts | 73 +------------------ 1 file changed, 2 insertions(+), 71 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index 7a7a6ad16d9c..c1b95c937e7d 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -1,4 +1,6 @@ import { + isPromiseConstructorLike, + isPromiseLike, isTypeAnyType, isTypeUnknownType, } from '@typescript-eslint/type-utils'; @@ -43,77 +45,6 @@ export default createRule({ : node; } - function isSymbolFromDefaultLibrary( - program: ts.Program, - symbol: ts.Symbol | undefined, - ): boolean { - if (!symbol) { - return false; - } - - const declarations = symbol.getDeclarations() ?? []; - for (const declaration of declarations) { - const sourceFile = declaration.getSourceFile(); - if (program.isSourceFileDefaultLibrary(sourceFile)) { - return true; - } - } - - return false; - } - - function isPromiseLike(program: ts.Program, type: ts.Type): boolean { - return isBuiltinSymbolLike(program, type, 'Promise'); - } - - function isPromiseConstructorLike( - program: ts.Program, - type: ts.Type, - ): boolean { - return isBuiltinSymbolLike(program, type, 'PromiseConstructor'); - } - - function isBuiltinSymbolLike( - program: ts.Program, - type: ts.Type, - symbolName: string, - ): boolean { - if (type.isIntersection()) { - return type.types.some(t => - isBuiltinSymbolLike(program, t, symbolName), - ); - } - if (type.isUnion()) { - return type.types.every(t => - isBuiltinSymbolLike(program, t, symbolName), - ); - } - - const symbol = type.getSymbol(); - if (!symbol) { - return false; - } - - if ( - symbol.getName() === symbolName && - isSymbolFromDefaultLibrary(program, symbol) - ) { - return true; - } - - if (symbol.flags & (ts.SymbolFlags.Class | ts.SymbolFlags.Interface)) { - const checker = program.getTypeChecker(); - - for (const baseType of checker.getBaseTypes(type as ts.InterfaceType)) { - if (isBuiltinSymbolLike(program, baseType, symbolName)) { - return true; - } - } - } - - return false; - } - function isPartiallyLikeType( type: ts.Type, predicate: (type: ts.Type) => boolean, From 82447b77fefbbe43ee160d4faac645166a395fde Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Fri, 12 Jan 2024 07:43:04 -0800 Subject: [PATCH 23/26] emptyArrayElement, refactoring, typed member names Also adds tests for rejected promises, calling via template literals, and `never` argument values --- .../rules/thenable-in-promise-aggregators.ts | 81 ++++++++++++----- .../thenable-in-promise-aggregators.test.ts | 88 ++++++++++++++++++- packages/type-utils/src/builtinSymbolLikes.ts | 22 +++-- 3 files changed, 164 insertions(+), 27 deletions(-) diff --git a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts index c1b95c937e7d..e6eeac2e882b 100644 --- a/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts +++ b/packages/eslint-plugin/src/rules/thenable-in-promise-aggregators.ts @@ -1,6 +1,5 @@ import { - isPromiseConstructorLike, - isPromiseLike, + isBuiltinSymbolLike, isTypeAnyType, isTypeUnknownType, } from '@typescript-eslint/type-utils'; @@ -11,6 +10,8 @@ import * as ts from 'typescript'; import { createRule, getParserServices } from '../util'; +const aggregateFunctionNames = new Set(['all', 'race', 'allSettled', 'any']); + export default createRule({ name: 'thenable-in-promise-aggregators', meta: { @@ -25,6 +26,8 @@ export default createRule({ 'Unexpected non-Thenable value in array passed to promise aggregator.', arrayArg: 'Unexpected array of non-Thenable values passed to promise aggregator.', + emptyArrayElement: + 'Unexpected empty element in array passed to promise aggregator (do you have an extra comma?).', }, schema: [], type: 'problem', @@ -35,8 +38,6 @@ export default createRule({ const services = getParserServices(context); const checker = services.program.getTypeChecker(); - const aggregateFunctionNames = ['all', 'race', 'allSettled', 'any']; - function skipChainExpression( node: T, ): T | TSESTree.ChainElement { @@ -91,6 +92,48 @@ export default createRule({ return typeArgs.some(t => isPartiallyLikeType(t, predicate)); } + function isStringLiteralMatching( + type: ts.Type, + predicate: (value: string) => boolean, + ): boolean { + if (type.isIntersection()) { + return type.types.some(t => isStringLiteralMatching(t, predicate)); + } + + if (type.isUnion()) { + return type.types.every(t => isStringLiteralMatching(t, predicate)); + } + + if (!type.isStringLiteral()) { + return false; + } + + return predicate(type.value); + } + + function isMemberName( + node: + | TSESTree.MemberExpressionComputedName + | TSESTree.MemberExpressionNonComputedName, + predicate: (name: string) => boolean, + ): boolean { + if (!node.computed) { + return predicate(node.property.name); + } + + if (node.property.type !== AST_NODE_TYPES.Literal) { + const typeOfProperty = services.getTypeAtLocation(node.property); + return isStringLiteralMatching(typeOfProperty, predicate); + } + + const { value } = node.property; + if (typeof value !== 'string') { + return false; + } + + return predicate(value); + } + return { CallExpression(node: TSESTree.CallExpression): void { const callee = skipChainExpression(node.callee); @@ -98,28 +141,24 @@ export default createRule({ return; } - if (callee.computed) { - if ( - callee.property.type !== AST_NODE_TYPES.Literal || - typeof callee.property.value !== 'string' || - !aggregateFunctionNames.includes(callee.property.value) - ) { - return; - } - } else if (!aggregateFunctionNames.includes(callee.property.name)) { + if (!isMemberName(callee, n => aggregateFunctionNames.has(n))) { return; } - const calleeType = services.getTypeAtLocation(callee.object); - if ( - !isPromiseConstructorLike(services.program, calleeType) && - !isPromiseLike(services.program, calleeType) - ) { + const args = node.arguments; + if (args.length < 1) { return; } - const args = node.arguments; - if (args.length < 1) { + const calleeType = services.getTypeAtLocation(callee.object); + + if ( + !isBuiltinSymbolLike( + services.program, + calleeType, + name => name === 'PromiseConstructor' || name === 'Promise', + ) + ) { return; } @@ -133,7 +172,7 @@ export default createRule({ for (const element of elements) { if (element == null) { context.report({ - messageId: 'inArray', + messageId: 'emptyArrayElement', node: arg, }); return; diff --git a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts index 4461309d7079..8c912b05a99a 100644 --- a/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts +++ b/packages/eslint-plugin/tests/rules/thenable-in-promise-aggregators.test.ts @@ -21,7 +21,9 @@ ruleTester.run('thenable-in-promise-aggregators', rule, { 'await Promise.allSettled([Promise.resolve(3)]);', 'await Promise.any([Promise.resolve(3)]);', 'await Promise.all([]);', + 'await Promise.race([Promise.reject(3)]);', "await Promise['all']([Promise.resolve(3)]);", + 'await Promise[`all`]([Promise.resolve(3)]);', "await Promise.all([Promise['resolve'](3)]);", 'await Promise.race([(async () => true)()]);', ` @@ -39,6 +41,30 @@ declare const anyValue: any; await Promise.race([anyValue]); `, ` +const key = 'all'; +await Promise[key]([Promise.resolve(3)]); + `, + ` +declare const key: 'race'; +await Promise[key]([Promise.resolve(3)]); + `, + ` +declare const key: 'race' | 'any'; +await Promise[key]([Promise.resolve(3)]); + `, + ` +declare const key: any; +await Promise[key]([3]); + `, + ` +declare const key: 'all' | 'unrelated'; +await Promise[key]([3]); + `, + ` +declare const key: [string] & 'all'; +await Promise[key]([Promise.resolve(3)]); + `, + ` declare const unknownValue: unknown; await Promise.race([unknownValue]); `, @@ -466,12 +492,24 @@ await foo.all([0]); }, ], }, + { + code: ` +declare const foo: never; +await Promise.all([foo]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, { code: 'await Promise.all([,]);', errors: [ { line: 1, - messageId: 'inArray', + messageId: 'emptyArrayElement', }, ], }, @@ -504,6 +542,54 @@ await foo.all([0]); }, { code: ` +const key = 'all'; +await Promise[key]([3]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, + { + code: ` +declare const key: 'all'; +await Promise[key]([3]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, + { + code: ` +declare const key: 'all' | 'race'; +await Promise[key]([3]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, + { + code: ` +declare const key: 'all' & Promise; +await Promise[key]([3]); + `, + errors: [ + { + line: 3, + messageId: 'inArray', + }, + ], + }, + { + code: ` declare const badUnion: number | string; await Promise.all([badUnion]); `, diff --git a/packages/type-utils/src/builtinSymbolLikes.ts b/packages/type-utils/src/builtinSymbolLikes.ts index 3443a0d0382e..8c0c7253008c 100644 --- a/packages/type-utils/src/builtinSymbolLikes.ts +++ b/packages/type-utils/src/builtinSymbolLikes.ts @@ -8,7 +8,11 @@ import { isSymbolFromDefaultLibrary } from './isSymbolFromDefaultLibrary'; * ^ PromiseLike */ export function isPromiseLike(program: ts.Program, type: ts.Type): boolean { - return isBuiltinSymbolLike(program, type, 'Promise'); + return isBuiltinSymbolLike( + program, + type, + symbolName => symbolName === 'Promise', + ); } /** @@ -20,7 +24,11 @@ export function isPromiseConstructorLike( program: ts.Program, type: ts.Type, ): boolean { - return isBuiltinSymbolLike(program, type, 'PromiseConstructor'); + return isBuiltinSymbolLike( + program, + type, + symbolName => symbolName === 'PromiseConstructor', + ); } /** @@ -29,7 +37,11 @@ export function isPromiseConstructorLike( * ^ ErrorLike */ export function isErrorLike(program: ts.Program, type: ts.Type): boolean { - return isBuiltinSymbolLike(program, type, 'Error'); + return isBuiltinSymbolLike( + program, + type, + symbolName => symbolName === 'Error', + ); } /** @@ -105,7 +117,7 @@ export function isBuiltinTypeAliasLike( export function isBuiltinSymbolLike( program: ts.Program, type: ts.Type, - symbolName: string, + predicate: (symbolName: string) => boolean, ): boolean { return isBuiltinSymbolLikeRecurser(program, type, subType => { const symbol = subType.getSymbol(); @@ -114,7 +126,7 @@ export function isBuiltinSymbolLike( } if ( - symbol.getName() === symbolName && + predicate(symbol.getName()) && isSymbolFromDefaultLibrary(program, symbol) ) { return true; From c5aef503447ff2d1dbff51d816bc71234a02ffe8 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Wed, 24 Jan 2024 08:42:56 -0800 Subject: [PATCH 24/26] Minor documentation cleanup --- .../docs/rules/thenable-in-promise-aggregators.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md index 0d18f0dfebe7..820a36e511a8 100644 --- a/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md +++ b/packages/eslint-plugin/docs/rules/thenable-in-promise-aggregators.md @@ -9,7 +9,7 @@ description: 'Disallow passing non-Thenable values to promise aggregators.' A "Thenable" value is an object which has a `then` method, such as a Promise. The `await` keyword is generally used to retrieve the result of calling a Thenable's `then` method. -When multiple Thenable's are running at the same time, it is sometimes desirable to wait until any one of them resolves (`Promise.race`), all of them resolve or any of them reject (`Promise.all`), or all of them resolve or reject (`Promise.allSettled`). +When multiple Thenables are running at the same time, it is sometimes desirable to wait until any one of them resolves (`Promise.race`), all of them resolve or any of them reject (`Promise.all`), or all of them resolve or reject (`Promise.allSettled`). Each of these functions accept an iterable of promises as input and return a single Promise. If a non-Thenable is passed, it is ignored. @@ -43,6 +43,6 @@ await Promise.race([ ## When Not To Use It -If you want to allow code to use `Promise.race`, `Promise.all`, or `Promise.allSettled` on arrays of non-promise values. +If you want to allow code to use `Promise.race`, `Promise.all`, or `Promise.allSettled` on arrays of non-Thenable values. This is generally not preferred but can sometimes be useful for visual consistency. You might consider using [ESLint disable comments](https://eslint.org/docs/latest/use/configure/rules#using-configuration-comments-1) for those specific situations instead of completely disabling this rule. From 2bbe8fa547b0fc5f9a64ff0e8c825bfa9979896f Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Fri, 2 Feb 2024 08:08:36 -0800 Subject: [PATCH 25/26] Avoid backward incompat lib signature change --- packages/type-utils/src/builtinSymbolLikes.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/type-utils/src/builtinSymbolLikes.ts b/packages/type-utils/src/builtinSymbolLikes.ts index 8c0c7253008c..284742917401 100644 --- a/packages/type-utils/src/builtinSymbolLikes.ts +++ b/packages/type-utils/src/builtinSymbolLikes.ts @@ -114,11 +114,30 @@ export function isBuiltinTypeAliasLike( }); } +/** + * Checks if the given type is an instance of a built-in type whose name matches + * the given predicate, i.e., it either is that type or extends it. + * + * This will return false if the type is _potentially_ an instance of the given + * type but might not be, e.g., if it's a union type where only some of the + * members are instances of a built-in type matching the predicate, this returns + * false. + * + * @param program The program the type is defined in + * @param type The type + * @param predicateOrSymbolName A predicate which returns true if the name of a + * symbol is a match and false otherwise, or the name of the symbol to match + */ export function isBuiltinSymbolLike( program: ts.Program, type: ts.Type, - predicate: (symbolName: string) => boolean, + predicateOrSymbolName: string | ((symbolName: string) => boolean), ): boolean { + const predicate = + typeof predicateOrSymbolName === 'string' + ? (symbolName: string) => symbolName === predicateOrSymbolName + : predicateOrSymbolName; + return isBuiltinSymbolLikeRecurser(program, type, subType => { const symbol = subType.getSymbol(); if (!symbol) { From ea652123afad3e4692d9ad6e3fcedb9659ccb0f1 Mon Sep 17 00:00:00 2001 From: Timothy Moore Date: Fri, 2 Feb 2024 08:11:07 -0800 Subject: [PATCH 26/26] Satisfy linter --- packages/type-utils/src/builtinSymbolLikes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/type-utils/src/builtinSymbolLikes.ts b/packages/type-utils/src/builtinSymbolLikes.ts index 284742917401..14e5f5c4480b 100644 --- a/packages/type-utils/src/builtinSymbolLikes.ts +++ b/packages/type-utils/src/builtinSymbolLikes.ts @@ -135,7 +135,7 @@ export function isBuiltinSymbolLike( ): boolean { const predicate = typeof predicateOrSymbolName === 'string' - ? (symbolName: string) => symbolName === predicateOrSymbolName + ? (symbolName: string): boolean => symbolName === predicateOrSymbolName : predicateOrSymbolName; return isBuiltinSymbolLikeRecurser(program, type, subType => {