refactor(general): avoid async method inside forEach (WIP)

.eslintrc.js

@@ -178,6 +178,7 @@ module.exports = {
         'aws-toolkits/no-console-log': 'error',
         'aws-toolkits/no-json-stringify-in-log': 'error',
         'aws-toolkits/no-printf-mismatch': 'error',
+        'aws-toolkits/no-async-in-foreach': 'error',
         'no-restricted-imports': [
             'error',
             {

packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts

@@ -269,15 +269,7 @@ function closeSessionWhenAllEditorsClosed(
 }
 
 function isLiveTailSessionOpenInAnyTab(liveTailSession: LiveTailSession) {
-    let isOpen = false
-    vscode.window.tabGroups.all.forEach(async (tabGroup) => {
-        tabGroup.tabs.forEach((tab) => {
-            if (tab.input instanceof vscode.TabInputText) {
-                if (liveTailSession.uri.toString() === tab.input.uri.toString()) {
-                    isOpen = true
-                }
-            }
-        })
-    })
-    return isOpen
+    const isOpen = (tab: vscode.Tab) =>
+        tab.input instanceof vscode.TabInputText && tab.input.uri.toString() === liveTailSession.uri.toString()
+    return vscode.window.tabGroups.all.some((tabGroup) => tabGroup.tabs.some(isOpen))
 }

packages/core/src/awsService/ec2/activation.ts

@@ -21,6 +21,7 @@ import {
 import { Ec2ConnecterMap } from './connectionManagerMap'
 import { ec2LogsScheme } from '../../shared/constants'
 import { Ec2LogDocumentProvider } from './ec2LogDocumentProvider'
+import { mapOverMap } from '../../shared/utilities/collectionUtils'
 
 const connectionManagers = new Ec2ConnecterMap()
 
@@ -83,5 +84,5 @@ export async function activate(ctx: ExtContext): Promise<void> {
 }
 
 export async function deactivate(): Promise<void> {
-    connectionManagers.forEach(async (manager) => await manager.dispose())
+    await mapOverMap(connectionManagers, async (_, manager) => await manager.dispose())
 }

packages/core/src/awsService/ec2/remoteSessionManager.ts

@@ -6,6 +6,7 @@
 import { EC2, SSM } from 'aws-sdk'
 import { SsmClient } from '../../shared/clients/ssmClient'
 import { Disposable } from 'vscode'
+import { mapOverMap } from '../../shared/utilities/collectionUtils'
 
 export class Ec2SessionTracker extends Map<EC2.InstanceId, SSM.SessionId> implements Disposable {
     public constructor(
@@ -31,7 +32,7 @@ export class Ec2SessionTracker extends Map<EC2.InstanceId, SSM.SessionId> implem
     }
 
     public async dispose(): Promise<void> {
-        this.forEach(async (_sessionId, instanceId) => await this.disconnectEnv(instanceId))
+        await mapOverMap(this, async (instanceId, _sessionId) => await this.disconnectEnv(instanceId))
     }
 
     public isConnectedTo(instanceId: EC2.InstanceId): boolean {

packages/core/src/codewhisperer/service/importAdderProvider.ts

@@ -55,7 +55,7 @@ export class ImportAdderProvider implements vscode.CodeLensProvider {
         ) {
             const line = findLineToInsertImportStatement(editor, firstLineOfRecommendation)
             let mergedStatements = ``
-            r.mostRelevantMissingImports?.forEach(async (i) => {
+            r.mostRelevantMissingImports?.forEach((i) => {
                 // trust service response that this to-be-added import is necessary
                 if (i.statement) {
                     mergedStatements += i.statement + '\n'

packages/core/src/shared/utilities/collectionUtils.ts

@@ -566,3 +566,7 @@ export function createCollectionFromPages<T>(...pages: T[]): AsyncCollection<T>
 export function isPresent<T>(value: T | undefined): value is T {
     return value !== undefined
 }
+
+export async function mapOverMap<K, V, O>(m: Map<K, V>, f: (k: K, v: V) => Promise<O>): Promise<O[]> {
+    return await Promise.all(Array.from(m).map(async ([k, v]) => await f(k, v)))
+}

packages/core/src/test/lambda/commands/listSamResources.test.ts

@@ -62,7 +62,7 @@ describe('listSamResources', () => {
             { name: 'stringify array', value: '[]' },
             { name: 'array object', value: [] },
         ]
-        testcases.forEach(async ({ name, value }) => {
+        testcases.forEach(({ name, value }) => {
             it(`returns empty array given SAM CLI return ${name} given any issue`, async () => {
                 runSamCliListResourceStub.resolves(value)
 

packages/core/src/test/shared/fs/fs.test.ts

@@ -218,15 +218,15 @@ describe('FileSystem', function () {
     describe('mkdir()', function () {
         const paths = ['a', 'a/b', 'a/b/c', 'a/b/c/d/']
 
-        paths.forEach(async function (p) {
+        paths.forEach(function (p) {
             it(`creates folder: '${p}'`, async function () {
                 const dirPath = testFolder.pathFrom(p)
                 await fs.mkdir(dirPath)
                 assert(existsSync(dirPath))
             })
         })
 
-        paths.forEach(async function (p) {
+        paths.forEach(function (p) {
             it(`creates folder but uses the "fs" module if in Cloud9: '${p}'`, async function () {
                 sandbox.stub(extensionUtilities, 'isCloud9').returns(true)
                 const dirPath = testFolder.pathFrom(p)

plugins/eslint-plugin-aws-toolkits/index.ts

@@ -11,6 +11,7 @@ import NoStringExecForChildProcess from './lib/rules/no-string-exec-for-child-pr
 import NoConsoleLog from './lib/rules/no-console-log'
 import noJsonStringifyInLog from './lib/rules/no-json-stringify-in-log'
 import noPrintfMismatch from './lib/rules/no-printf-mismatch'
+import noAsyncInForEach from './lib/rules/no-async-in-foreach'
 
 const rules = {
     'no-await-on-vscode-msg': NoAwaitOnVscodeMsg,
@@ -21,6 +22,7 @@ const rules = {
     'no-console-log': NoConsoleLog,
     'no-json-stringify-in-log': noJsonStringifyInLog,
     'no-printf-mismatch': noPrintfMismatch,
+    'no-async-in-foreach': noAsyncInForEach,
 }
 
 export { rules }

plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts

@@ -0,0 +1,87 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { ESLintUtils, TSESTree } from '@typescript-eslint/utils'
+import { AST_NODE_TYPES } from '@typescript-eslint/types'
+import { Rule } from 'eslint'
+import { RuleContext } from '@typescript-eslint/utils/ts-eslint'
+
+export const errMsg = 'Avoid using async methods with .forEach as it leads to race conditions'
+
+function isFunctionExpression(
+    node: TSESTree.Node
+): node is TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression {
+    return node.type === AST_NODE_TYPES.ArrowFunctionExpression || node.type === AST_NODE_TYPES.FunctionExpression
+}
+
+function isAsyncFunction<T extends string, T2 extends readonly unknown[]>(
+    context: RuleContext<T, T2>,
+    funcNode: TSESTree.CallExpressionArgument
+) {
+    if (funcNode.type === AST_NODE_TYPES.Identifier) {
+        const scope = context.sourceCode.getScope(funcNode)
+        const maybeFNode =
+            scope.variables.find((v) => v.name === funcNode.name)?.defs.find((d) => !!d)?.node ?? undefined
+        // function declartions Ex. async function f() {}
+        if (
+            maybeFNode &&
+            (isFunctionExpression(maybeFNode) || maybeFNode.type === AST_NODE_TYPES.FunctionDeclaration) &&
+            maybeFNode.async
+        ) {
+            return true
+        }
+        // variable-style function declarations Ex. const f = async function () {}
+        if (
+            maybeFNode &&
+            maybeFNode.type === AST_NODE_TYPES.VariableDeclarator &&
+            maybeFNode.init &&
+            isFunctionExpression(maybeFNode.init) &&
+            maybeFNode.init.async
+        ) {
+            return true
+        }
+        return false
+    }
+    return (
+        (funcNode.type === AST_NODE_TYPES.ArrowFunctionExpression ||
+            funcNode.type === AST_NODE_TYPES.FunctionExpression) &&
+        funcNode.async
+    )
+}
+
+export default ESLintUtils.RuleCreator.withoutDocs({
+    meta: {
+        docs: {
+            description: 'disallow inlining async functions with .forEach',
+            recommended: 'recommended',
+        },
+        messages: {
+            errMsg,
+        },
+        type: 'problem',
+        fixable: 'code',
+        schema: [],
+    },
+    defaultOptions: [],
+    create(context) {
+        return {
+            CallExpression(node: TSESTree.CallExpression) {
+                if (
+                    node.callee.type === AST_NODE_TYPES.MemberExpression &&
+                    node.callee.property.type === AST_NODE_TYPES.Identifier &&
+                    node.callee.property.name === 'forEach' &&
+                    node.arguments.length >= 1
+                ) {
+                    if (isAsyncFunction(context, node.arguments[0])) {
+                        return context.report({
+                            node: node,
+                            messageId: 'errMsg',
+                        })
+                    }
+                }
+            },
+        }
+    },
+}) as unknown as Rule.RuleModule

plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts

@@ -0,0 +1,43 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { rules } from '../../index'
+import { errMsg } from '../../lib/rules/no-async-in-foreach'
+import { getRuleTester } from '../testUtil'
+
+getRuleTester().run('no-async-in-foreach', rules['no-async-in-foreach'], {
+    valid: [
+        'list.forEach((a) => a * a)',
+        'list.forEach(asyncFunctionOrNot)',
+        'list.forEach(() => console.log(x))',
+        'list.forEach(function () {})',
+        'function f(){} \n list.forEach(f)',
+        'const f = function () {} \n list.forEach(f)',
+    ],
+
+    invalid: [
+        { code: 'list.forEach(async (a) => await Promise.resolve(a * a))', errors: [errMsg] },
+        { code: 'list.forEach(async (a: any) => console.log(x))', errors: [errMsg] },
+        { code: 'list.forEach((a) => a.forEach(async (b) => a * b))', errors: [errMsg] },
+        { code: 'list.forEach(async function () {})', errors: [errMsg] },
+        { code: 'async function f(){} \n list.forEach(f)', errors: [errMsg] },
+        { code: 'const f = async () => {} \n list.forEach(f)', errors: [errMsg] },
+        { code: 'const f = async function () {} \n list.forEach(f)', errors: [errMsg] },
+        { code: 'class c { \n public async f() {} \n } \n [].forEach((new c().f))', errors: [errMsg] },
+        {
+            code: 'class c { \n public async f() {} \n } \n const c2 = new c() \n list.forEach(c2.f)',
+            errors: [errMsg],
+        },
+        { code: 'function f() { \n return async function () {}} \n [].forEach(f())', errors: [errMsg] },
+        {
+            code: 'function f() { \n return new (class c { \n public async f2() {} \n })().f2 \n } \n list.forEach(f())',
+            errors: [errMsg],
+        },
+        {
+            code: 'function f() { \n return function f2() { \n return function f3() { \n return function f4() { \n return function f5() { \n return async function f6() { \n \n } \n } \n } \n } \n } \n } \n list.forEach(f()()()()())',
+            errors: [errMsg],
+        },
+    ],
+})