From 91ba8b1c500bcd8ae642fadf6362335f90b26e34 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 17 Dec 2024 13:56:20 -0500 Subject: [PATCH 01/19] remove async within forEach --- .../cloudWatchLogs/commands/tailLogGroup.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts b/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts index 4aa1feb272a..7f656a6d061 100644 --- a/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts +++ b/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)) } From 890de9e999b3fd1582acecd70e3c3daa0e61b9f4 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 17 Dec 2024 14:28:10 -0500 Subject: [PATCH 02/19] fully remove forEach async --- packages/core/src/awsService/ec2/activation.ts | 3 ++- packages/core/src/awsService/ec2/remoteSessionManager.ts | 3 ++- .../core/src/codewhisperer/service/importAdderProvider.ts | 2 +- packages/core/src/shared/utilities/collectionUtils.ts | 4 ++++ .../core/src/test/lambda/commands/listSamResources.test.ts | 2 +- packages/core/src/test/shared/fs/fs.test.ts | 4 ++-- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/core/src/awsService/ec2/activation.ts b/packages/core/src/awsService/ec2/activation.ts index f8881af8347..604fc4b10ce 100644 --- a/packages/core/src/awsService/ec2/activation.ts +++ b/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 { } export async function deactivate(): Promise { - connectionManagers.forEach(async (manager) => await manager.dispose()) + await mapOverMap(connectionManagers, async (_, manager) => await manager.dispose()) } diff --git a/packages/core/src/awsService/ec2/remoteSessionManager.ts b/packages/core/src/awsService/ec2/remoteSessionManager.ts index 4c1843aabdb..238d66f9c3d 100644 --- a/packages/core/src/awsService/ec2/remoteSessionManager.ts +++ b/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 implements Disposable { public constructor( @@ -31,7 +32,7 @@ export class Ec2SessionTracker extends Map implem } public async dispose(): Promise { - this.forEach(async (_sessionId, instanceId) => await this.disconnectEnv(instanceId)) + await mapOverMap(this, async (_, instanceId) => await this.disconnectEnv(instanceId)) } public isConnectedTo(instanceId: EC2.InstanceId): boolean { diff --git a/packages/core/src/codewhisperer/service/importAdderProvider.ts b/packages/core/src/codewhisperer/service/importAdderProvider.ts index de16365713e..0c2f0bd3dc8 100644 --- a/packages/core/src/codewhisperer/service/importAdderProvider.ts +++ b/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' diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 3776d7fac7b..ba7e97e2020 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -566,3 +566,7 @@ export function createCollectionFromPages(...pages: T[]): AsyncCollection export function isPresent(value: T | undefined): value is T { return value !== undefined } + +export async function mapOverMap(m: Map, f: (k: K, v: V) => Promise): Promise { + return await Promise.all(Array.from(m).map(async ([k, v]) => await f(k, v))) +} diff --git a/packages/core/src/test/lambda/commands/listSamResources.test.ts b/packages/core/src/test/lambda/commands/listSamResources.test.ts index 37a7d227f10..3fc6c7cd12b 100644 --- a/packages/core/src/test/lambda/commands/listSamResources.test.ts +++ b/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) diff --git a/packages/core/src/test/shared/fs/fs.test.ts b/packages/core/src/test/shared/fs/fs.test.ts index 5fa4428559f..fe0342bf043 100644 --- a/packages/core/src/test/shared/fs/fs.test.ts +++ b/packages/core/src/test/shared/fs/fs.test.ts @@ -218,7 +218,7 @@ 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) @@ -226,7 +226,7 @@ describe('FileSystem', function () { }) }) - 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) From f7b6d12738e9f4feddaedd1b95fd951c90a8a135 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 17 Dec 2024 14:56:08 -0500 Subject: [PATCH 03/19] fix parameter mismatch --- packages/core/src/awsService/ec2/remoteSessionManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/awsService/ec2/remoteSessionManager.ts b/packages/core/src/awsService/ec2/remoteSessionManager.ts index 238d66f9c3d..c59c3b4d454 100644 --- a/packages/core/src/awsService/ec2/remoteSessionManager.ts +++ b/packages/core/src/awsService/ec2/remoteSessionManager.ts @@ -32,7 +32,7 @@ export class Ec2SessionTracker extends Map implem } public async dispose(): Promise { - await mapOverMap(this, async (_, instanceId) => await this.disconnectEnv(instanceId)) + await mapOverMap(this, async (instanceId, _sessionId) => await this.disconnectEnv(instanceId)) } public isConnectedTo(instanceId: EC2.InstanceId): boolean { From ec20f3a03584104aa953b66817ff805662c20dad Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 17 Dec 2024 16:09:22 -0500 Subject: [PATCH 04/19] implement lint rule for inline async --- plugins/eslint-plugin-aws-toolkits/index.ts | 2 + .../lib/rules/no-inline-async-foreach.ts | 50 +++++++++++++++++++ .../rules/no-inline-async-foreach.test.ts | 24 +++++++++ 3 files changed, 76 insertions(+) create mode 100644 plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts create mode 100644 plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index 87ee111c095..6c015370385 100644 --- a/plugins/eslint-plugin-aws-toolkits/index.ts +++ b/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 noInlineAsyncForEach from './lib/rules/no-inline-async-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-inline-async-foreach': noInlineAsyncForEach, } export { rules } diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts new file mode 100644 index 00000000000..a1221dfe290 --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts @@ -0,0 +1,50 @@ +/*! + * 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' + +export const errMsg = 'Avoid using async methods with .forEach as it leads to race conditions and confusing behavior' + +function isAsyncFunction(node: TSESTree.CallExpressionArgument): boolean { + return ( + (node.type === AST_NODE_TYPES.ArrowFunctionExpression || node.type === AST_NODE_TYPES.FunctionExpression) && + node.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 && + isAsyncFunction(node.arguments[0]) + ) { + return context.report({ + node: node, + messageId: 'errMsg', + }) + } + }, + } + }, +}) as unknown as Rule.RuleModule diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts new file mode 100644 index 00000000000..5541cd20e2b --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts @@ -0,0 +1,24 @@ +/*! + * 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-inline-async-foreach' +import { getRuleTester } from '../testUtil' + +getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], { + valid: [ + 'list.forEach((a) => a * a)', + 'list.forEach(asyncFunctionOrNot)', + 'list.forEach(() => console.log(x))', + 'list.forEach(function () {})', + ], + + 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] }, + ], +}) From 25c42c70bc71694acb0f1e68d7fb66929aaa4210 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 17 Dec 2024 16:11:15 -0500 Subject: [PATCH 05/19] add to eslint --- .eslintrc.js | 1 + .../lib/rules/no-inline-async-foreach.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index b3e5e39dc9f..7d1acd8b805 100644 --- a/.eslintrc.js +++ b/.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-inline-async-foreach': 'error', 'no-restricted-imports': [ 'error', { diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts index a1221dfe290..9d8c06f5408 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts @@ -7,7 +7,7 @@ import { ESLintUtils, TSESTree } from '@typescript-eslint/utils' import { AST_NODE_TYPES } from '@typescript-eslint/types' import { Rule } from 'eslint' -export const errMsg = 'Avoid using async methods with .forEach as it leads to race conditions and confusing behavior' +export const errMsg = 'Avoid using async methods with .forEach as it leads to race conditions' function isAsyncFunction(node: TSESTree.CallExpressionArgument): boolean { return ( From 669a9cd460a7b804ab2d650592b6307c8f7a26a2 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 18 Dec 2024 13:24:16 -0500 Subject: [PATCH 06/19] catch async defined functions --- .../lib/rules/no-inline-async-foreach.ts | 42 +++++++++++++++---- .../rules/no-inline-async-foreach.test.ts | 6 +++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts index 9d8c06f5408..1b45acd9847 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts @@ -6,15 +6,38 @@ 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 isAsyncFunction(node: TSESTree.CallExpressionArgument): boolean { +function isAsyncFunction( + context: RuleContext, + funcNode: TSESTree.CallExpressionArgument +) { + if (funcNode.type === AST_NODE_TYPES.Identifier) { + console.log('is identifier') + const scope = context.sourceCode.getScope(funcNode) + const maybeFNode = + scope.variables.find((v) => v.name === funcNode.name)?.defs.find((d) => !!d)?.node ?? undefined + console.log(maybeFNode) + if ( + maybeFNode && + (maybeFNode.type === AST_NODE_TYPES.ArrowFunctionExpression || + maybeFNode.type === AST_NODE_TYPES.FunctionExpression || + maybeFNode.type === AST_NODE_TYPES.FunctionDeclaration) && + maybeFNode.async + ) { + return true + } + return false + } return ( - (node.type === AST_NODE_TYPES.ArrowFunctionExpression || node.type === AST_NODE_TYPES.FunctionExpression) && - node.async + (funcNode.type === AST_NODE_TYPES.ArrowFunctionExpression || + funcNode.type === AST_NODE_TYPES.FunctionExpression) && + funcNode.async ) } + export default ESLintUtils.RuleCreator.withoutDocs({ meta: { docs: { @@ -36,13 +59,14 @@ export default ESLintUtils.RuleCreator.withoutDocs({ node.callee.type === AST_NODE_TYPES.MemberExpression && node.callee.property.type === AST_NODE_TYPES.Identifier && node.callee.property.name === 'forEach' && - node.arguments.length >= 1 && - isAsyncFunction(node.arguments[0]) + node.arguments.length >= 1 ) { - return context.report({ - node: node, - messageId: 'errMsg', - }) + if (isAsyncFunction(context, node.arguments[0])) { + return context.report({ + node: node, + messageId: 'errMsg', + }) + } } }, } diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts index 5541cd20e2b..973c5b90a5d 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts @@ -13,6 +13,8 @@ getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], '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: [ @@ -20,5 +22,9 @@ getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], { 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: 'const f = function () {} \n list.forEach(f)', errors: [errMsg] }, ], }) From 0cc790684f86c27e29b1c42b9b57243f72d106ce Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 18 Dec 2024 13:57:13 -0500 Subject: [PATCH 07/19] clean up helper functions --- .../lib/rules/no-inline-async-foreach.ts | 23 +++++++++++++++---- .../rules/no-inline-async-foreach.test.ts | 1 - 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts index 1b45acd9847..75764d733c3 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts @@ -10,25 +10,38 @@ 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( context: RuleContext, funcNode: TSESTree.CallExpressionArgument ) { if (funcNode.type === AST_NODE_TYPES.Identifier) { - console.log('is identifier') const scope = context.sourceCode.getScope(funcNode) const maybeFNode = scope.variables.find((v) => v.name === funcNode.name)?.defs.find((d) => !!d)?.node ?? undefined - console.log(maybeFNode) + // function declartions Ex. async function f() {} if ( maybeFNode && - (maybeFNode.type === AST_NODE_TYPES.ArrowFunctionExpression || - maybeFNode.type === AST_NODE_TYPES.FunctionExpression || - maybeFNode.type === AST_NODE_TYPES.FunctionDeclaration) && + (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 ( diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts index 973c5b90a5d..99672e162c2 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts @@ -25,6 +25,5 @@ getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], { 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: 'const f = function () {} \n list.forEach(f)', errors: [errMsg] }, ], }) From 104a85f855b23f4fb30dcf67565a5d8f67cbf4e6 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 18 Dec 2024 14:19:14 -0500 Subject: [PATCH 08/19] add more test cases --- .../test/rules/no-inline-async-foreach.test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts index 99672e162c2..42b065cae97 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts @@ -25,5 +25,15 @@ getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], { 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], + }, ], }) From a0e47137e789d3ce50e7a92009515c8f4954a574 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 18 Dec 2024 14:40:36 -0500 Subject: [PATCH 09/19] add test cases for remaining cases --- .eslintrc.js | 2 +- plugins/eslint-plugin-aws-toolkits/index.ts | 4 ++-- ...{no-inline-async-foreach.ts => no-async-in-foreach.ts} | 0 ...-async-foreach.test.ts => no-async-in-foreach.test.ts} | 8 ++++++-- 4 files changed, 9 insertions(+), 5 deletions(-) rename plugins/eslint-plugin-aws-toolkits/lib/rules/{no-inline-async-foreach.ts => no-async-in-foreach.ts} (100%) rename plugins/eslint-plugin-aws-toolkits/test/rules/{no-inline-async-foreach.test.ts => no-async-in-foreach.test.ts} (79%) diff --git a/.eslintrc.js b/.eslintrc.js index 7d1acd8b805..02a6f8f6608 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -178,7 +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-inline-async-foreach': 'error', + 'aws-toolkits/no-async-in-foreach': 'error', 'no-restricted-imports': [ 'error', { diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index 6c015370385..7634dc095bb 100644 --- a/plugins/eslint-plugin-aws-toolkits/index.ts +++ b/plugins/eslint-plugin-aws-toolkits/index.ts @@ -11,7 +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 noInlineAsyncForEach from './lib/rules/no-inline-async-foreach' +import noAsyncInForEach from './lib/rules/no-async-in-foreach' const rules = { 'no-await-on-vscode-msg': NoAwaitOnVscodeMsg, @@ -22,7 +22,7 @@ const rules = { 'no-console-log': NoConsoleLog, 'no-json-stringify-in-log': noJsonStringifyInLog, 'no-printf-mismatch': noPrintfMismatch, - 'no-inline-async-foreach': noInlineAsyncForEach, + 'no-async-in-foreach': noAsyncInForEach, } export { rules } diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts similarity index 100% rename from plugins/eslint-plugin-aws-toolkits/lib/rules/no-inline-async-foreach.ts rename to plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts similarity index 79% rename from plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts rename to plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts index 42b065cae97..ca3600ddbb1 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-inline-async-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts @@ -4,10 +4,10 @@ */ import { rules } from '../../index' -import { errMsg } from '../../lib/rules/no-inline-async-foreach' +import { errMsg } from '../../lib/rules/no-async-in-foreach' import { getRuleTester } from '../testUtil' -getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], { +getRuleTester().run('no-async-in-foreach', rules['no-async-in-foreach'], { valid: [ 'list.forEach((a) => a * a)', 'list.forEach(asyncFunctionOrNot)', @@ -35,5 +35,9 @@ getRuleTester().run('no-inline-async-foreach', rules['no-inline-async-foreach'], 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], + }, ], }) From 7a10045fcd2a939797fd951f005c7393d5ffce50 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 11:28:29 -0500 Subject: [PATCH 10/19] solve simple member expression case --- .../lib/rules/no-async-in-foreach.ts | 102 ++++++++++++++---- 1 file changed, 79 insertions(+), 23 deletions(-) diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts index 75764d733c3..0fca557af32 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts @@ -16,33 +16,89 @@ function isFunctionExpression( return node.type === AST_NODE_TYPES.ArrowFunctionExpression || node.type === AST_NODE_TYPES.FunctionExpression } +function isAsyncFuncIdentifier( + context: RuleContext, + funcNode: TSESTree.Identifier +): boolean { + 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 +} + +function getClassDeclarationNode( + context: RuleContext, + node: TSESTree.Node +): TSESTree.ClassDeclaration | undefined { + console.log('in getObjectNode with %O', node) + if (node.type === AST_NODE_TYPES.MemberExpression) { + return getClassDeclarationNode(context, node.object) + } + if (node.type === AST_NODE_TYPES.NewExpression && node.callee.type === AST_NODE_TYPES.Identifier) { + const className = node.callee.name + const scope = context.sourceCode.getScope(node) + const maybeDefNode = + (scope.variables + .find((v) => v.name === className) + ?.defs.find((d) => !!d && d.node.type === AST_NODE_TYPES.ClassDeclaration) + ?.node as TSESTree.ClassDeclaration) ?? undefined + return maybeDefNode + } +} + +function isAsyncClassMethod(defObjNode: TSESTree.ClassDeclaration, functionName: string): boolean { + return defObjNode.body.body.some( + (n) => + n.type === AST_NODE_TYPES.MethodDefinition && + n.kind === 'method' && + n.key.type === AST_NODE_TYPES.Identifier && + n.key.name === functionName && + n.value.async + ) +} + +function isAsyncFuncMemExpression( + context: RuleContext, + funcNode: TSESTree.MemberExpression +): boolean { + console.log('in isAsyncMemExp') + if (funcNode.object.type === AST_NODE_TYPES.MemberExpression) { + return isAsyncFuncMemExpression(context, funcNode.object) + } + if (funcNode.property.type === AST_NODE_TYPES.Identifier) { + const fName = funcNode.property.name + const defObjNode = getClassDeclarationNode(context, funcNode.object) + return isAsyncClassMethod(defObjNode!, fName) + } + return false +} + function isAsyncFunction( context: RuleContext, 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 isAsyncFuncIdentifier(context, funcNode) + } + if (funcNode.type === AST_NODE_TYPES.MemberExpression) { + return isAsyncFuncMemExpression(context, funcNode) } return ( (funcNode.type === AST_NODE_TYPES.ArrowFunctionExpression || @@ -54,7 +110,7 @@ function isAsyncFunction( export default ESLintUtils.RuleCreator.withoutDocs({ meta: { docs: { - description: 'disallow inlining async functions with .forEach', + description: 'disallow async functions with .forEach', recommended: 'recommended', }, messages: { From 9a22040e47efd2d23a3693a24ef4c9112a8b7dd8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 11:44:27 -0500 Subject: [PATCH 11/19] start of work on next case --- .../lib/rules/no-async-in-foreach.ts | 6 ++++++ .../test/rules/no-async-in-foreach.test.ts | 18 +++++++++--------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts index 0fca557af32..7271420b9b8 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts @@ -61,6 +61,12 @@ function getClassDeclarationNode v.name === node.name)?.defs.find((d) => !!d)?.node ?? undefined + console.log('got maybeDefNode %O', maybeDefNode) + return undefined + } } function isAsyncClassMethod(defObjNode: TSESTree.ClassDeclaration, functionName: string): boolean { diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts index ca3600ddbb1..45d76c70c18 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts @@ -30,14 +30,14 @@ getRuleTester().run('no-async-in-foreach', rules['no-async-in-foreach'], { 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], - }, + // { 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], + // }, ], }) From 492ee73e05c1fd94d5e82acd294b5539a2be5d91 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 11:53:50 -0500 Subject: [PATCH 12/19] switch to banning general usage --- .eslintrc.js | 2 +- plugins/eslint-plugin-aws-toolkits/index.ts | 4 +- .../lib/rules/no-async-in-foreach.ts | 149 ------------------ .../lib/rules/no-foreach.ts | 43 +++++ .../test/rules/no-async-in-foreach.test.ts | 43 ----- .../test/rules/no-foreach.test.ts | 36 +++++ 6 files changed, 82 insertions(+), 195 deletions(-) delete mode 100644 plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts create mode 100644 plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts delete mode 100644 plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts create mode 100644 plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index 02a6f8f6608..0063785c769 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -178,7 +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', + 'aws-toolkits/no-foreach': 'error', 'no-restricted-imports': [ 'error', { diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index 7634dc095bb..23f5dae0610 100644 --- a/plugins/eslint-plugin-aws-toolkits/index.ts +++ b/plugins/eslint-plugin-aws-toolkits/index.ts @@ -11,7 +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' +import noForeach from './lib/rules/no-foreach' const rules = { 'no-await-on-vscode-msg': NoAwaitOnVscodeMsg, @@ -22,7 +22,7 @@ const rules = { 'no-console-log': NoConsoleLog, 'no-json-stringify-in-log': noJsonStringifyInLog, 'no-printf-mismatch': noPrintfMismatch, - 'no-async-in-foreach': noAsyncInForEach, + 'no-foreach': noForeach, } export { rules } diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts deleted file mode 100644 index 7271420b9b8..00000000000 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-async-in-foreach.ts +++ /dev/null @@ -1,149 +0,0 @@ -/*! - * 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 isAsyncFuncIdentifier( - context: RuleContext, - funcNode: TSESTree.Identifier -): boolean { - 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 -} - -function getClassDeclarationNode( - context: RuleContext, - node: TSESTree.Node -): TSESTree.ClassDeclaration | undefined { - console.log('in getObjectNode with %O', node) - if (node.type === AST_NODE_TYPES.MemberExpression) { - return getClassDeclarationNode(context, node.object) - } - if (node.type === AST_NODE_TYPES.NewExpression && node.callee.type === AST_NODE_TYPES.Identifier) { - const className = node.callee.name - const scope = context.sourceCode.getScope(node) - const maybeDefNode = - (scope.variables - .find((v) => v.name === className) - ?.defs.find((d) => !!d && d.node.type === AST_NODE_TYPES.ClassDeclaration) - ?.node as TSESTree.ClassDeclaration) ?? undefined - return maybeDefNode - } - if (node.type === AST_NODE_TYPES.Identifier) { - const scope = context.sourceCode.getScope(node) - const maybeDefNode = scope.variables.find((v) => v.name === node.name)?.defs.find((d) => !!d)?.node ?? undefined - console.log('got maybeDefNode %O', maybeDefNode) - return undefined - } -} - -function isAsyncClassMethod(defObjNode: TSESTree.ClassDeclaration, functionName: string): boolean { - return defObjNode.body.body.some( - (n) => - n.type === AST_NODE_TYPES.MethodDefinition && - n.kind === 'method' && - n.key.type === AST_NODE_TYPES.Identifier && - n.key.name === functionName && - n.value.async - ) -} - -function isAsyncFuncMemExpression( - context: RuleContext, - funcNode: TSESTree.MemberExpression -): boolean { - console.log('in isAsyncMemExp') - if (funcNode.object.type === AST_NODE_TYPES.MemberExpression) { - return isAsyncFuncMemExpression(context, funcNode.object) - } - if (funcNode.property.type === AST_NODE_TYPES.Identifier) { - const fName = funcNode.property.name - const defObjNode = getClassDeclarationNode(context, funcNode.object) - return isAsyncClassMethod(defObjNode!, fName) - } - return false -} - -function isAsyncFunction( - context: RuleContext, - funcNode: TSESTree.CallExpressionArgument -) { - if (funcNode.type === AST_NODE_TYPES.Identifier) { - return isAsyncFuncIdentifier(context, funcNode) - } - if (funcNode.type === AST_NODE_TYPES.MemberExpression) { - return isAsyncFuncMemExpression(context, funcNode) - } - return ( - (funcNode.type === AST_NODE_TYPES.ArrowFunctionExpression || - funcNode.type === AST_NODE_TYPES.FunctionExpression) && - funcNode.async - ) -} - -export default ESLintUtils.RuleCreator.withoutDocs({ - meta: { - docs: { - description: 'disallow 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 diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts new file mode 100644 index 00000000000..fd56eae85ce --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts @@ -0,0 +1,43 @@ +/*! + * 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' + +export const errMsg = 'Avoid using .forEach as it can lead to race conditions' + +export default ESLintUtils.RuleCreator.withoutDocs({ + meta: { + docs: { + description: 'disallow 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 + ) { + return context.report({ + node: node, + messageId: 'errMsg', + }) + } + }, + } + }, +}) as unknown as Rule.RuleModule diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts deleted file mode 100644 index 45d76c70c18..00000000000 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-async-in-foreach.test.ts +++ /dev/null @@ -1,43 +0,0 @@ -/*! - * 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], - // }, - ], -}) diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts new file mode 100644 index 00000000000..1e7c894581e --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts @@ -0,0 +1,36 @@ +/*! + * 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-foreach' +import { getRuleTester } from '../testUtil' + +getRuleTester().run('no-foreach', rules['no-foreach'], { + valid: ['list.map(f)', 'list.find(f)', 'list.forAll(f)', 'o.forEachItem(f)', ''], + + invalid: [ + { code: 'list.forEach((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: '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], + }, + ], +}) From fcfc106b0a915c102c87bfa80a2cb18b9282143c Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 12:00:22 -0500 Subject: [PATCH 13/19] update comment --- plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts index fd56eae85ce..f30f89812e5 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-foreach.ts @@ -12,7 +12,7 @@ export const errMsg = 'Avoid using .forEach as it can lead to race conditions' export default ESLintUtils.RuleCreator.withoutDocs({ meta: { docs: { - description: 'disallow async functions with .forEach', + description: 'disallow .forEach', recommended: 'recommended', }, messages: { From 13353dadf857064a2797cd9d17bdd19bf6d32b51 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 13:28:50 -0500 Subject: [PATCH 14/19] start migration --- .../scripts/build/generateServiceClient.ts | 9 +- packages/core/scripts/lint/testLint.ts | 4 +- packages/core/src/extensionNode.ts | 20 +-- .../src/shared/utilities/collectionUtils.ts | 6 + packages/toolkit/package.json | 127 +++++++++++------- packages/toolkit/src/api.ts | 25 ++-- .../lib/rules/no-incorrect-once-usage.ts | 13 +- .../rules/no-string-exec-for-child-process.ts | 4 +- .../test/rules/no-foreach.test.ts | 2 +- scripts/generateSettings.ts | 16 +-- 10 files changed, 135 insertions(+), 91 deletions(-) diff --git a/packages/core/scripts/build/generateServiceClient.ts b/packages/core/scripts/build/generateServiceClient.ts index 4c6fb730b19..154cd8b9842 100644 --- a/packages/core/scripts/build/generateServiceClient.ts +++ b/packages/core/scripts/build/generateServiceClient.ts @@ -106,7 +106,7 @@ async function insertServiceClientsIntoJsSdk( jsSdkPath: string, serviceClientDefinitions: ServiceClientDefinition[] ): Promise { - serviceClientDefinitions.forEach((serviceClientDefinition) => { + for (const serviceClientDefinition of serviceClientDefinitions) { const apiVersion = getApiVersion(serviceClientDefinition.serviceJsonPath) // Copy the Service Json into the JS SDK for generation @@ -116,7 +116,7 @@ async function insertServiceClientsIntoJsSdk( `${serviceClientDefinition.serviceName.toLowerCase()}-${apiVersion}.normal.json` ) nodefs.copyFileSync(serviceClientDefinition.serviceJsonPath, jsSdkServiceJsonPath) - }) + } const apiMetadataPath = path.join(jsSdkPath, 'apis', 'metadata.json') await patchServicesIntoApiMetadata( @@ -150,10 +150,9 @@ async function patchServicesIntoApiMetadata(apiMetadataPath: string, serviceName const apiMetadataJson = nodefs.readFileSync(apiMetadataPath).toString() const apiMetadata = JSON.parse(apiMetadataJson) as ApiMetadata - - serviceNames.forEach((serviceName) => { + for (const serviceName of serviceNames) { apiMetadata[serviceName.toLowerCase()] = { name: serviceName } - }) + } nodefs.writeFileSync(apiMetadataPath, JSON.stringify(apiMetadata, undefined, 4)) } diff --git a/packages/core/scripts/lint/testLint.ts b/packages/core/scripts/lint/testLint.ts index 43bafb6ae00..d215e57f675 100644 --- a/packages/core/scripts/lint/testLint.ts +++ b/packages/core/scripts/lint/testLint.ts @@ -12,9 +12,9 @@ void (async () => { const mocha = new Mocha() const testFiles = await glob('dist/src/testLint/**/*.test.js') - testFiles.forEach((file) => { + for (const file of testFiles) { mocha.addFile(file) - }) + } mocha.run((failures) => { const exitCode = failures ? 1 : 0 diff --git a/packages/core/src/extensionNode.ts b/packages/core/src/extensionNode.ts index b6d4a599ce1..c0dcca1a9e2 100644 --- a/packages/core/src/extensionNode.ts +++ b/packages/core/src/extensionNode.ts @@ -60,6 +60,7 @@ import { activate as activateThreatComposerEditor } from './threatComposer/activ import { isSsoConnection, hasScopes } from './auth/connection' import { CrashMonitoring, setContext } from './shared' import { AuthFormId } from './login/webview/vue/types' +import { addAll } from './shared/utilities/collectionUtils' let localize: nls.LocalizeFunc @@ -85,10 +86,11 @@ export async function activate(context: vscode.ExtensionContext) { const toolkitEnvDetails = getExtEnvironmentDetails() // Splits environment details by new line, filter removes the empty string - toolkitEnvDetails - .split(/\r?\n/) - .filter(Boolean) - .forEach((line) => getLogger().info(line)) + const logger = getLogger() + const linesToLog = toolkitEnvDetails.split(/\r?\n/).filter(Boolean) + for (const line of linesToLog) { + logger.info(line) + } globals.awsContextCommands = new AwsContextCommands(globals.regionProvider, Auth.instance) globals.schemaService = new SchemaService() @@ -343,17 +345,19 @@ async function getAuthState(): Promise> { const enabledScopes: Set = new Set() if (Auth.instance.hasConnections) { authStatus = 'expired' - ;(await Auth.instance.listConnections()).forEach((conn) => { + const connections = await Auth.instance.listConnections() + for (const conn of connections) { const state = Auth.instance.getConnectionState(conn) if (state === 'valid') { authStatus = 'connected' } + const connectionsIds = getAuthFormIdsFromConnection(conn) + addAll(enabledConnections, connectionsIds) - getAuthFormIdsFromConnection(conn).forEach((id) => enabledConnections.add(id)) if (isSsoConnection(conn)) { - conn.scopes?.forEach((s) => enabledScopes.add(s)) + addAll(enabledScopes, conn.scopes ?? []) } - }) + } } // There may be other SSO connections in toolkit, but there is no use case for diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index ba7e97e2020..eaf6265c7fd 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -570,3 +570,9 @@ export function isPresent(value: T | undefined): value is T { export async function mapOverMap(m: Map, f: (k: K, v: V) => Promise): Promise { return await Promise.all(Array.from(m).map(async ([k, v]) => await f(k, v))) } + +export function addAll(s: Set, items: T[]) { + for (const item of items) { + s.add(item) + } +} diff --git a/packages/toolkit/package.json b/packages/toolkit/package.json index 0344109e11d..a70784cd74e 100644 --- a/packages/toolkit/package.json +++ b/packages/toolkit/package.json @@ -4037,327 +4037,362 @@ "fontCharacter": "\\f1ac" } }, - "aws-amazonq-transform-arrow-dark": { + "aws-amazonq-severity-critical": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1ad" } }, - "aws-amazonq-transform-arrow-light": { + "aws-amazonq-severity-high": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1ae" } }, - "aws-amazonq-transform-default-dark": { + "aws-amazonq-severity-info": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1af" } }, - "aws-amazonq-transform-default-light": { + "aws-amazonq-severity-low": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b0" } }, - "aws-amazonq-transform-dependencies-dark": { + "aws-amazonq-severity-medium": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b1" } }, - "aws-amazonq-transform-dependencies-light": { + "aws-amazonq-transform-arrow-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b2" } }, - "aws-amazonq-transform-file-dark": { + "aws-amazonq-transform-arrow-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b3" } }, - "aws-amazonq-transform-file-light": { + "aws-amazonq-transform-default-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b4" } }, - "aws-amazonq-transform-logo": { + "aws-amazonq-transform-default-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b5" } }, - "aws-amazonq-transform-step-into-dark": { + "aws-amazonq-transform-dependencies-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b6" } }, - "aws-amazonq-transform-step-into-light": { + "aws-amazonq-transform-dependencies-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b7" } }, - "aws-amazonq-transform-variables-dark": { + "aws-amazonq-transform-file-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b8" } }, - "aws-amazonq-transform-variables-light": { + "aws-amazonq-transform-file-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1b9" } }, - "aws-applicationcomposer-icon": { + "aws-amazonq-transform-logo": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1ba" } }, - "aws-applicationcomposer-icon-dark": { + "aws-amazonq-transform-step-into-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1bb" } }, - "aws-apprunner-service": { + "aws-amazonq-transform-step-into-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1bc" } }, - "aws-cdk-logo": { + "aws-amazonq-transform-variables-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1bd" } }, - "aws-cloudformation-stack": { + "aws-amazonq-transform-variables-light": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1be" } }, - "aws-cloudwatch-log-group": { + "aws-applicationcomposer-icon": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1bf" } }, - "aws-codecatalyst-logo": { + "aws-applicationcomposer-icon-dark": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c0" } }, - "aws-codewhisperer-icon-black": { + "aws-apprunner-service": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c1" } }, - "aws-codewhisperer-icon-white": { + "aws-cdk-logo": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c2" } }, - "aws-codewhisperer-learn": { + "aws-cloudformation-stack": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c3" } }, - "aws-ecr-registry": { + "aws-cloudwatch-log-group": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c4" } }, - "aws-ecs-cluster": { + "aws-codecatalyst-logo": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c5" } }, - "aws-ecs-container": { + "aws-codewhisperer-icon-black": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c6" } }, - "aws-ecs-service": { + "aws-codewhisperer-icon-white": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c7" } }, - "aws-generic-attach-file": { + "aws-codewhisperer-learn": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c8" } }, - "aws-iot-certificate": { + "aws-ecr-registry": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1c9" } }, - "aws-iot-policy": { + "aws-ecs-cluster": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1ca" } }, - "aws-iot-thing": { + "aws-ecs-container": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1cb" } }, - "aws-lambda-function": { + "aws-ecs-service": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1cc" } }, - "aws-mynah-MynahIconBlack": { + "aws-generic-attach-file": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1cd" } }, - "aws-mynah-MynahIconWhite": { + "aws-iot-certificate": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1ce" } }, - "aws-mynah-logo": { + "aws-iot-policy": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1cf" } }, - "aws-redshift-cluster": { + "aws-iot-thing": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d0" } }, - "aws-redshift-cluster-connected": { + "aws-lambda-function": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d1" } }, - "aws-redshift-database": { + "aws-mynah-MynahIconBlack": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d2" } }, - "aws-redshift-redshift-cluster-connected": { + "aws-mynah-MynahIconWhite": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d3" } }, - "aws-redshift-schema": { + "aws-mynah-logo": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d4" } }, - "aws-redshift-table": { + "aws-redshift-cluster": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d5" } }, - "aws-s3-bucket": { + "aws-redshift-cluster-connected": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d6" } }, - "aws-s3-create-bucket": { + "aws-redshift-database": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d7" } }, - "aws-schemas-registry": { + "aws-redshift-redshift-cluster-connected": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d8" } }, - "aws-schemas-schema": { + "aws-redshift-schema": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1d9" } }, - "aws-stepfunctions-preview": { + "aws-redshift-table": { "description": "AWS Contributed Icon", "default": { "fontPath": "./resources/fonts/aws-toolkit-icons.woff", "fontCharacter": "\\f1da" } + }, + "aws-s3-bucket": { + "description": "AWS Contributed Icon", + "default": { + "fontPath": "./resources/fonts/aws-toolkit-icons.woff", + "fontCharacter": "\\f1db" + } + }, + "aws-s3-create-bucket": { + "description": "AWS Contributed Icon", + "default": { + "fontPath": "./resources/fonts/aws-toolkit-icons.woff", + "fontCharacter": "\\f1dc" + } + }, + "aws-schemas-registry": { + "description": "AWS Contributed Icon", + "default": { + "fontPath": "./resources/fonts/aws-toolkit-icons.woff", + "fontCharacter": "\\f1dd" + } + }, + "aws-schemas-schema": { + "description": "AWS Contributed Icon", + "default": { + "fontPath": "./resources/fonts/aws-toolkit-icons.woff", + "fontCharacter": "\\f1de" + } + }, + "aws-stepfunctions-preview": { + "description": "AWS Contributed Icon", + "default": { + "fontPath": "./resources/fonts/aws-toolkit-icons.woff", + "fontCharacter": "\\f1df" + } } }, "notebooks": [ diff --git a/packages/toolkit/src/api.ts b/packages/toolkit/src/api.ts index 1cead54d6a5..15b81d43338 100644 --- a/packages/toolkit/src/api.ts +++ b/packages/toolkit/src/api.ts @@ -20,24 +20,25 @@ export const awsToolkitApi = { async listConnections(): Promise { getLogger().debug(`listConnections: extension ${extensionId}`) const connections = await Auth.instance.listConnections() - const exposedConnections: AwsConnection[] = [] - connections.forEach((x: Connection) => { + return connections.flatMap((x: Connection) => { if (x.type === 'sso') { const connState = Auth.instance.getConnectionState(x) if (connState) { - exposedConnections.push({ - id: x.id, - label: x.label, - type: x.type, - ssoRegion: x.ssoRegion, - startUrl: x.startUrl, - scopes: x.scopes, - state: connState, - }) + return [ + { + id: x.id, + label: x.label, + type: x.type, + ssoRegion: x.ssoRegion, + startUrl: x.startUrl, + scopes: x.scopes, + state: connState, + }, + ] } } + return [] }) - return exposedConnections }, /** diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-incorrect-once-usage.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-incorrect-once-usage.ts index 9a8a4853af7..a464056f504 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-incorrect-once-usage.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-incorrect-once-usage.ts @@ -114,8 +114,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({ ) { return } - - node.declarations.forEach((declaration) => { + for (const declaration of node.declarations) { if ( declaration.init?.type === AST_NODE_TYPES.CallExpression && declaration.id.type === AST_NODE_TYPES.Identifier && @@ -134,11 +133,11 @@ export default ESLintUtils.RuleCreator.withoutDocs({ // Check if it is being referenced multiple times // TODO: expand to check if it is being referenced inside nested scopes only? (currently checks current scope as well) if (refs.length > 1) { - return + continue } // Check if it is being referenced once, but inside a loop. - refs.forEach((ref) => { + for (const ref of refs) { let currNode: TSESTree.Node | undefined = ref.identifier while (currNode && currNode !== scope.block) { @@ -154,18 +153,18 @@ export default ESLintUtils.RuleCreator.withoutDocs({ } currNode = currNode.parent } - }) + } } // If the variable is somehow not assigned? or only used once and not in a loop. if (variable === undefined || !isUsedInLoopScope) { - return context.report({ + context.report({ node: declaration.init.callee, messageId: 'notReusableErr', }) } } - }) + } }, } }, diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-exec-for-child-process.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-exec-for-child-process.ts index 769b1157cfa..7313ceed61e 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-exec-for-child-process.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-exec-for-child-process.ts @@ -45,7 +45,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({ ImportDeclaration(node: TSESTree.ImportDeclaration) { // Detect imports for child_process if (node.source.value === 'child_process') { - node.specifiers.forEach((specifier) => { + for (const specifier of node.specifiers) { if (specifier.type === AST_NODE_TYPES.ImportNamespaceSpecifier) { // Detect the name of the import, e.g. "proc" from "import * as proc from child_process" libImportName = specifier.local.name @@ -59,7 +59,7 @@ export default ESLintUtils.RuleCreator.withoutDocs({ messageId: 'errMsg', }) } - }) + } } }, CallExpression(node: TSESTree.CallExpression) { diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts index 1e7c894581e..aba2547dce2 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-foreach.test.ts @@ -13,7 +13,7 @@ getRuleTester().run('no-foreach', rules['no-foreach'], { invalid: [ { code: 'list.forEach((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((a) => a.forEach(async (b) => a * b))', errors: [errMsg, errMsg] }, { code: 'list.forEach(async function () {})', errors: [errMsg] }, { code: 'function f(){} \n list.forEach(f)', errors: [errMsg] }, { code: 'const f = async () => {} \n list.forEach(f)', errors: [errMsg] }, diff --git a/scripts/generateSettings.ts b/scripts/generateSettings.ts index 5fe210f9ed3..54f03734e5c 100644 --- a/scripts/generateSettings.ts +++ b/scripts/generateSettings.ts @@ -23,16 +23,16 @@ function main() { type Configuration = { [key: string]: { [key: string]: {} } } const settings: Configuration = {} - Object.entries(packageJson.contributes.configuration.properties as { [key: string]: { properties?: {} } }).forEach( - ([k, v]) => { - settings[k] = {} - if (v.properties !== undefined) { - for (const inner of Object.keys(v.properties)) { - settings[k][inner] = {} - } + for (const [k, v] of Object.entries( + packageJson.contributes.configuration.properties as { [key: string]: { properties?: {} } } + )) { + settings[k] = {} + if (v.properties !== undefined) { + for (const inner of Object.keys(v.properties)) { + settings[k][inner] = {} } } - ) + } const contents = `/*! * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. From 7c09eb81a71fc8f9c2e6b059dd5cd7c7cf515efd Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 14:38:55 -0500 Subject: [PATCH 15/19] refactor textUtilities tests --- .../commons/controllers/contentController.ts | 17 +++-- packages/core/src/amazonq/commons/diff.ts | 45 +++++++----- .../src/shared/utilities/collectionUtils.ts | 4 +- .../src/shared/utilities/textUtilities.ts | 5 ++ .../shared/utilities/textUtilities.test.ts | 68 +++++++++++++++---- 5 files changed, 98 insertions(+), 41 deletions(-) diff --git a/packages/core/src/amazonq/commons/controllers/contentController.ts b/packages/core/src/amazonq/commons/controllers/contentController.ts index 0af3b317025..08b3985a69d 100644 --- a/packages/core/src/amazonq/commons/controllers/contentController.ts +++ b/packages/core/src/amazonq/commons/controllers/contentController.ts @@ -15,7 +15,13 @@ import { getIndentedCode, getSelectionFromRange, } from '../../../shared/utilities/textDocumentUtilities' -import { extractFileAndCodeSelectionFromMessage, fs, getErrorMsg, ToolkitError } from '../../../shared' +import { + extractFileAndCodeSelectionFromMessage, + formatTextWithIndent, + fs, + getErrorMsg, + ToolkitError, +} from '../../../shared' export class ContentProvider implements vscode.TextDocumentContentProvider { constructor(private uri: vscode.Uri) {} @@ -49,14 +55,7 @@ export class EditorContentController { if (indent.trim().length !== 0) { indent = ' '.repeat(indent.length - indent.trimStart().length) } - let textWithIndent = '' - text.split('\n').forEach((line, index) => { - if (index === 0) { - textWithIndent += line - } else { - textWithIndent += '\n' + indent + line - } - }) + const textWithIndent = formatTextWithIndent(text, indent) editor .edit((editBuilder) => { editBuilder.insert(cursorStart, textWithIndent) diff --git a/packages/core/src/amazonq/commons/diff.ts b/packages/core/src/amazonq/commons/diff.ts index beb45d88096..661ada38ea6 100644 --- a/packages/core/src/amazonq/commons/diff.ts +++ b/packages/core/src/amazonq/commons/diff.ts @@ -5,7 +5,7 @@ import * as vscode from 'vscode' import { fs } from '../../shared' -import { diffLines } from 'diff' +import { Change, diffLines } from 'diff' export async function openDiff(leftPath: string, rightPath: string, tabId: string, scheme: string) { const { left, right } = await getFileDiffUris(leftPath, rightPath, tabId, scheme) @@ -42,21 +42,30 @@ export async function computeDiff(leftPath: string, rightPath: string, tabId: st ignoreWhitespace: true, }) - let charsAdded = 0 - let charsRemoved = 0 - let linesAdded = 0 - let linesRemoved = 0 - changes.forEach((change) => { - const lines = change.value.split('\n') - const charCount = lines.reduce((sum, line) => sum + line.length, 0) - const lineCount = change.count ?? lines.length - 1 // ignoring end-of-file empty line - if (change.added) { - charsAdded += charCount - linesAdded += lineCount - } else if (change.removed) { - charsRemoved += charCount - linesRemoved += lineCount - } - }) - return { changes, charsAdded, linesAdded, charsRemoved, linesRemoved } + interface Result { + charsAdded: number + linesAdded: number + charsRemoved: number + linesRemoved: number + } + + const changeDetails = changes.reduce( + (curResult: Result, change: Change) => { + const lines = change.value.split('\n') + const charCount = lines.reduce((sum, line) => sum + line.length, 0) + const lineCount = change.count ?? lines.length - 1 // ignoring end-of-file empty line + + if (change.added) { + curResult.charsAdded += charCount + curResult.linesAdded += lineCount + } else if (change.removed) { + curResult.charsRemoved += charCount + curResult.linesRemoved += lineCount + } + return curResult + }, + { charsAdded: 0, linesAdded: 0, charsRemoved: 0, linesRemoved: 0 } + ) + + return { changes, ...changeDetails } } diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index eaf6265c7fd..3ee3f3d274f 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -346,13 +346,13 @@ export function inspect(obj: any, opt?: { depth: number }): string { export function stripUndefined>( obj: T ): asserts obj is { [P in keyof T]-?: NonNullable } { - Object.keys(obj).forEach((key) => { + for (const key of Object.keys(obj)) { if (obj[key] === undefined) { delete obj[key] } else if (typeof obj[key] === 'object') { stripUndefined(obj[key]) } - }) + } } export function isAsyncIterable(obj: any): obj is AsyncIterable { diff --git a/packages/core/src/shared/utilities/textUtilities.ts b/packages/core/src/shared/utilities/textUtilities.ts index 95bfabe37d3..023ccdac81a 100644 --- a/packages/core/src/shared/utilities/textUtilities.ts +++ b/packages/core/src/shared/utilities/textUtilities.ts @@ -274,3 +274,8 @@ export function extractFileAndCodeSelectionFromMessage(message: any) { const selection = message?.context?.focusAreaContext?.selectionInsideExtendedCodeBlock as vscode.Selection return { filePath, selection } } + +export function formatTextWithIndent(text: string, indent: string): string { + // TODO: shouldn't this trim the resulting string? + return text.split('\n').reduce((prev, curr) => (prev === '' ? prev + curr : prev + '\n' + indent + curr)) +} diff --git a/packages/core/src/test/shared/utilities/textUtilities.test.ts b/packages/core/src/test/shared/utilities/textUtilities.test.ts index 8a62de243df..90b0eb52f67 100644 --- a/packages/core/src/test/shared/utilities/textUtilities.test.ts +++ b/packages/core/src/test/shared/utilities/textUtilities.test.ts @@ -13,6 +13,8 @@ import { sanitizeFilename, toSnakeCase, undefinedIfEmpty, + formatTextWithIndent, + formatTextWithIndent2, } from '../../../shared/utilities/textUtilities' describe('textUtilities', async function () { @@ -131,21 +133,35 @@ describe('toSnakeCase', function () { }) }) +interface TestCase { + input: Input + params?: any[] + output: Output + case: string +} + +function generateTestCases( + cases: TestCase[], + f: (t: Input, ...p: any[]) => Output +): void { + for (const c of cases) { + it(c.case, function () { + assert.strictEqual(c.params ? f(c.input, ...c.params) : f(c.input), c.output) + }) + } +} + describe('sanitizeFilename', function () { - const cases: { input: string; output: string; case: string; replaceString?: string }[] = [ + const cases = [ { input: 'foo🤷', output: 'foo_', case: 'removes emojis' }, { input: 'foo/zub', output: 'foo_zub', case: 'replaces slash with underscore' }, { input: 'foo zub', output: 'foo_zub', case: 'replaces space with underscore' }, - { input: 'foo:bar', output: 'fooXbar', replaceString: 'X', case: 'replaces dot with replaceString' }, + { input: 'foo:bar', output: 'fooXbar', params: ['X'], case: 'replaces dot with replaceString' }, { input: 'foo🤷bar/zu b.txt', output: 'foo_bar_zu_b.txt', case: 'docstring example' }, { input: 'foo.txt', output: 'foo.txt', case: 'keeps dot' }, { input: 'züb', output: 'züb', case: 'keeps special chars' }, ] - cases.forEach((testCase) => { - it(testCase.case, function () { - assert.strictEqual(sanitizeFilename(testCase.input, testCase.replaceString), testCase.output) - }) - }) + generateTestCases(cases, sanitizeFilename) }) describe('undefinedIfEmpty', function () { @@ -157,9 +173,37 @@ describe('undefinedIfEmpty', function () { { input: ' foo ', output: ' foo ', case: 'return original str without trim' }, ] - cases.forEach((testCases) => { - it(testCases.case, function () { - assert.strictEqual(undefinedIfEmpty(testCases.input), testCases.output) - }) - }) + generateTestCases(cases, undefinedIfEmpty) +}) + +describe('formatStringWithIndent', function () { + const cases = [ + { input: 'foo', output: 'foo', params: [' '], case: 'return str if input is not multiline' }, + { + input: 'foo\nbar', + output: 'foo\n bar', + params: [' '], + case: 'return str with indent if input is multiline', + }, + { + input: 'foo\n\nbar', + output: 'foo\n \n bar', + params: [' '], + case: 'return str with indent if input is multiline with empty line', + }, + { + input: 'foo\nbar\nfoo\nbar\n', + output: 'foo\n bar\n foo\n bar\n ', + params: [' '], + case: 'return str with indent if input is multiline with empty line at the end', + }, + { + input: 'foo\nbar\nfoo\nbar\n', + output: 'foo\nbar\nfoo\nbar\n', + params: [''], + case: 'returns original string with empty indent', + }, + ] + + generateTestCases(cases, formatTextWithIndent) }) From 09141a70b8b0e54f92f2ce54e64e08fc23937a3e Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 14:39:12 -0500 Subject: [PATCH 16/19] remove dead import --- packages/core/src/test/shared/utilities/textUtilities.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/test/shared/utilities/textUtilities.test.ts b/packages/core/src/test/shared/utilities/textUtilities.test.ts index 90b0eb52f67..640d615eff4 100644 --- a/packages/core/src/test/shared/utilities/textUtilities.test.ts +++ b/packages/core/src/test/shared/utilities/textUtilities.test.ts @@ -14,7 +14,6 @@ import { toSnakeCase, undefinedIfEmpty, formatTextWithIndent, - formatTextWithIndent2, } from '../../../shared/utilities/textUtilities' describe('textUtilities', async function () { From 87dc53352dc907c437ad261f18865e07ffe5699d Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 15:16:30 -0500 Subject: [PATCH 17/19] finish migrating Q --- .../controller/inlineChatController.ts | 2 +- .../src/inlineChat/output/computeDiff.ts | 7 +++---- .../service/keyStrokeHandler.test.ts | 4 ++-- .../util/crossFileContextUtil.test.ts | 18 +++++++++--------- .../codewhisperer/util/editorContext.test.ts | 7 +++---- .../util/runtimeLanguageContext.test.ts | 13 ++++++------- .../util/securityScanLanguageContext.test.ts | 4 ++-- packages/amazonq/test/web/testRunner.ts | 1 + 8 files changed, 27 insertions(+), 29 deletions(-) diff --git a/packages/amazonq/src/inlineChat/controller/inlineChatController.ts b/packages/amazonq/src/inlineChat/controller/inlineChatController.ts index ce0df6a0878..dcc7400b0a3 100644 --- a/packages/amazonq/src/inlineChat/controller/inlineChatController.ts +++ b/packages/amazonq/src/inlineChat/controller/inlineChatController.ts @@ -156,7 +156,7 @@ export class InlineChatController { } private async reset() { - this.listeners.forEach((listener) => listener.dispose()) + await Promise.all(this.listeners.map((l) => l.dispose())) this.listeners = [] this.task = undefined diff --git a/packages/amazonq/src/inlineChat/output/computeDiff.ts b/packages/amazonq/src/inlineChat/output/computeDiff.ts index 9b599b2eff3..a17a7914cf6 100644 --- a/packages/amazonq/src/inlineChat/output/computeDiff.ts +++ b/packages/amazonq/src/inlineChat/output/computeDiff.ts @@ -2,7 +2,7 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { type LinesOptions, diffLines, Change } from 'diff' +import { type LinesOptions, diffLines } from 'diff' import * as vscode from 'vscode' import { InlineTask, TextDiff } from '../controller/inlineTask' @@ -24,8 +24,7 @@ export function computeDiff(response: string, inlineTask: InlineTask, isPartialD const textDiff: TextDiff[] = [] let startLine = selectedRange.start.line - - diffs.forEach((part: Change) => { + for (const part of diffs) { const count = part.count ?? 0 if (part.removed) { if (part.value !== '\n') { @@ -49,7 +48,7 @@ export function computeDiff(response: string, inlineTask: InlineTask, isPartialD } } startLine += count - }) + } inlineTask.diff = textDiff return textDiff } diff --git a/packages/amazonq/test/unit/codewhisperer/service/keyStrokeHandler.test.ts b/packages/amazonq/test/unit/codewhisperer/service/keyStrokeHandler.test.ts index 075dc769b0e..4b6a5291f22 100644 --- a/packages/amazonq/test/unit/codewhisperer/service/keyStrokeHandler.test.ts +++ b/packages/amazonq/test/unit/codewhisperer/service/keyStrokeHandler.test.ts @@ -212,7 +212,7 @@ describe('keyStrokeHandler', function () { ['function suggestedByIntelliSense():', DocumentChangedSource.Unknown], ] - cases.forEach((tuple) => { + for (const tuple of cases) { const input = tuple[0] const expected = tuple[1] it(`test input ${input} should return ${expected}`, function () { @@ -221,7 +221,7 @@ describe('keyStrokeHandler', function () { ).checkChangeSource() assert.strictEqual(actual, expected) }) - }) + } function createFakeDocumentChangeEvent(str: string): ReadonlyArray { return [ diff --git a/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts b/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts index 5203159eb58..8bbf3fd3312 100644 --- a/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts +++ b/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts @@ -241,11 +241,12 @@ describe('crossFileContextUtil', function () { const actual = await crossFile.getCrossFileCandidates(editor) assert.ok(actual.length === 5) - actual.forEach((actualFile, index) => { + for (const item of actual.map((v: string, i: number) => [i, v] as const)) { + const [index, actualFile] = item const expectedFile = path.join(tempFolder, expectedFilePaths[index]) assert.strictEqual(normalize(expectedFile), normalize(actualFile)) assert.ok(areEqual(tempFolder, actualFile, expectedFile)) - }) + } }) }) @@ -264,7 +265,7 @@ describe('crossFileContextUtil', function () { await closeAllEditors() }) - fileExtLists.forEach((fileExt) => { + for (const fileExt of fileExtLists) { it('should be empty if userGroup is control', async function () { const editor = await toTextEditor('content-1', `file-1.${fileExt}`, tempFolder) await toTextEditor('content-2', `file-2.${fileExt}`, tempFolder, { preview: false }) @@ -277,7 +278,7 @@ describe('crossFileContextUtil', function () { assert.ok(actual && actual.supplementalContextItems.length === 0) }) - }) + } }) describe.skip('partial support - crossfile group', function () { @@ -294,8 +295,7 @@ describe('crossFileContextUtil', function () { afterEach(async function () { await closeAllEditors() }) - - fileExtLists.forEach((fileExt) => { + for (const fileExt of fileExtLists) { it('should be non empty if usergroup is Crossfile', async function () { const editor = await toTextEditor('content-1', `file-1.${fileExt}`, tempFolder) await toTextEditor('content-2', `file-2.${fileExt}`, tempFolder, { preview: false }) @@ -308,7 +308,7 @@ describe('crossFileContextUtil', function () { assert.ok(actual && actual.supplementalContextItems.length !== 0) }) - }) + } }) describe('full support', function () { @@ -329,7 +329,7 @@ describe('crossFileContextUtil', function () { await closeAllEditors() }) - fileExtLists.forEach((fileExt) => { + for (const fileExt of fileExtLists) { it(`supplemental context for file ${fileExt} should be non empty`, async function () { sinon.stub(FeatureConfigProvider.instance, 'getProjectContextGroup').returns('control') sinon @@ -353,7 +353,7 @@ describe('crossFileContextUtil', function () { assert.ok(actual && actual.supplementalContextItems.length !== 0) }) - }) + } }) describe('splitFileToChunks', function () { diff --git a/packages/amazonq/test/unit/codewhisperer/util/editorContext.test.ts b/packages/amazonq/test/unit/codewhisperer/util/editorContext.test.ts index 22be4199375..302df61533c 100644 --- a/packages/amazonq/test/unit/codewhisperer/util/editorContext.test.ts +++ b/packages/amazonq/test/unit/codewhisperer/util/editorContext.test.ts @@ -94,13 +94,12 @@ describe('editorContext', function () { ['typescript', 'ts'], ['c', 'c'], ]) - - languageToExtension.forEach((extension, language) => { + for (const language of languageToExtension.keys()) { const editor = createMockTextEditor('', 'test.ipynb', language, 1, 17) const actual = EditorContext.getFileRelativePath(editor) - const expected = 'test.' + extension + const expected = 'test.' + languageToExtension.get(language) assert.strictEqual(actual, expected) - }) + } }) it('Should return relative path', async function () { diff --git a/packages/amazonq/test/unit/codewhisperer/util/runtimeLanguageContext.test.ts b/packages/amazonq/test/unit/codewhisperer/util/runtimeLanguageContext.test.ts index 653505d6cf9..8a9517ca42a 100644 --- a/packages/amazonq/test/unit/codewhisperer/util/runtimeLanguageContext.test.ts +++ b/packages/amazonq/test/unit/codewhisperer/util/runtimeLanguageContext.test.ts @@ -51,8 +51,7 @@ describe('runtimeLanguageContext', function () { beforeEach(async function () { await resetCodeWhispererGlobalVariables() }) - - cases.forEach((tuple) => { + for (const tuple of cases) { const languageId = tuple[0] const expected = tuple[1] @@ -60,7 +59,7 @@ describe('runtimeLanguageContext', function () { const actual = languageContext.isLanguageSupported(languageId) assert.strictEqual(actual, expected) }) - }) + } describe('test isLanguageSupported with document as the argument', function () { const cases: [string, boolean][] = [ @@ -105,7 +104,7 @@ describe('runtimeLanguageContext', function () { ['helloFoo.foo', false], ] - cases.forEach((tuple) => { + for (const tuple of cases) { const fileName = tuple[0] const expected = tuple[1] @@ -114,7 +113,7 @@ describe('runtimeLanguageContext', function () { const actual = languageContext.isLanguageSupported(doc) assert.strictEqual(actual, expected) }) - }) + } }) }) @@ -148,14 +147,14 @@ describe('runtimeLanguageContext', function () { [undefined, 'plaintext'], ] - cases.forEach((tuple) => { + for (const tuple of cases) { const vscLanguageId = tuple[0] const expectedCwsprLanguageId = tuple[1] it(`given vscLanguage ${vscLanguageId} should return ${expectedCwsprLanguageId}`, function () { const result = runtimeLanguageContext.getLanguageContext(vscLanguageId) assert.strictEqual(result.language as string, expectedCwsprLanguageId) }) - }) + } }) describe('normalizeLanguage', function () { diff --git a/packages/amazonq/test/unit/codewhisperer/util/securityScanLanguageContext.test.ts b/packages/amazonq/test/unit/codewhisperer/util/securityScanLanguageContext.test.ts index cb0a51fdad8..e1d2cf91189 100644 --- a/packages/amazonq/test/unit/codewhisperer/util/securityScanLanguageContext.test.ts +++ b/packages/amazonq/test/unit/codewhisperer/util/securityScanLanguageContext.test.ts @@ -51,7 +51,7 @@ describe('securityScanLanguageContext', function () { await resetCodeWhispererGlobalVariables() }) - cases.forEach((tuple) => { + for (const tuple of cases) { const languageId = tuple[0] const expected = tuple[1] @@ -59,7 +59,7 @@ describe('securityScanLanguageContext', function () { const actual = languageContext.isLanguageSupported(languageId) assert.strictEqual(actual, expected) }) - }) + } }) describe('normalizeLanguage', function () { diff --git a/packages/amazonq/test/web/testRunner.ts b/packages/amazonq/test/web/testRunner.ts index a2c8f8e90cc..7be8fcd2197 100644 --- a/packages/amazonq/test/web/testRunner.ts +++ b/packages/amazonq/test/web/testRunner.ts @@ -35,6 +35,7 @@ function setupMocha() { function gatherTestFiles() { // Bundles all files in the current directory matching `*.test` + // eslint-disable-next-line aws-toolkits/no-foreach const importAll = (r: __WebpackModuleApi.RequireContext) => r.keys().forEach(r) importAll(require.context('.', true, /\.test$/)) } From 264354a8aff84dd529cfec7e88e765cb571bdf1e Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 16:34:54 -0500 Subject: [PATCH 18/19] partial migration --- .../core/src/amazonq/lsp/lspController.ts | 40 ++++++++-------- .../webview/ui/quickActions/handler.ts | 21 +++------ .../webview/ui/storages/tabsStorage.ts | 7 +++ packages/core/src/amazonqDoc/app.ts | 4 +- packages/core/src/amazonqFeatureDev/app.ts | 4 +- .../chat/controller/messenger/messenger.ts | 27 ++++------- .../amazonqTest/chat/controller/controller.ts | 8 ++-- .../applicationcomposer/composerWebview.ts | 6 +-- .../src/auth/credentials/sharedCredentials.ts | 6 ++- .../core/src/auth/credentials/validation.ts | 10 ++-- packages/core/src/auth/sso/cache.ts | 4 +- packages/core/src/auth/sso/server.ts | 4 +- .../accessanalyzer/vue/iamPolicyChecks.ts | 47 +++++++++---------- .../appBuilder/explorer/detectSamProjects.ts | 4 +- .../wizards/templateParametersWizard.ts | 4 +- .../src/awsService/apprunner/activation.ts | 10 ++-- .../apprunner/explorer/apprunnerNode.ts | 8 ++-- .../cdk/explorer/detectCdkProjects.ts | 4 +- .../cloudWatchLogs/commands/tailLogGroup.ts | 14 ++++-- .../document/logStreamsCodeLensProvider.ts | 6 +-- packages/core/src/awsexplorer/toolView.ts | 4 +- packages/core/src/codecatalyst/explorer.ts | 4 +- .../codecatalyst/wizards/devenvSettings.ts | 1 + packages/core/src/codewhisperer/activation.ts | 1 + .../src/codewhisperer/client/codewhisperer.ts | 12 ++--- .../codewhisperer/commands/basicCommands.ts | 4 +- .../service/completionProvider.ts | 9 ++-- .../service/diagnosticsProvider.ts | 18 +++---- .../service/importAdderProvider.ts | 13 +++-- .../service/recommendationHandler.ts | 13 ++--- .../src/shared/utilities/collectionUtils.ts | 4 ++ 31 files changed, 169 insertions(+), 152 deletions(-) diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index bb5cc88f5a2..6d0ae70683d 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -279,25 +279,27 @@ export class LspController { async query(s: string): Promise { const chunks: Chunk[] | undefined = await LspClient.instance.queryVectorIndex(s) - const resp: RelevantTextDocument[] = [] - chunks?.forEach((chunk) => { - const text = chunk.context ? chunk.context : chunk.content - if (chunk.programmingLanguage && chunk.programmingLanguage !== 'unknown') { - resp.push({ - text: text, - relativeFilePath: chunk.relativePath ? chunk.relativePath : path.basename(chunk.filePath), - programmingLanguage: { - languageName: chunk.programmingLanguage, - }, - }) - } else { - resp.push({ - text: text, - relativeFilePath: chunk.relativePath ? chunk.relativePath : path.basename(chunk.filePath), - }) - } - }) - return resp + return ( + chunks?.flatMap((chunk) => { + const text = chunk.context ? chunk.context : chunk.content + return chunk.programmingLanguage && chunk.programmingLanguage !== 'unknown' + ? [ + { + text: text, + relativeFilePath: chunk.relativePath ? chunk.relativePath : path.basename(chunk.filePath), + programmingLanguage: { + languageName: chunk.programmingLanguage, + }, + }, + ] + : [ + { + text: text, + relativeFilePath: chunk.relativePath ? chunk.relativePath : path.basename(chunk.filePath), + }, + ] + }) ?? [] + ) } async queryInlineProjectContext(query: string, path: string, target: 'bm25' | 'codemap' | 'default') { diff --git a/packages/core/src/amazonq/webview/ui/quickActions/handler.ts b/packages/core/src/amazonq/webview/ui/quickActions/handler.ts index 78ef3d0e7ec..3f8426fbc4c 100644 --- a/packages/core/src/amazonq/webview/ui/quickActions/handler.ts +++ b/packages/core/src/amazonq/webview/ui/quickActions/handler.ts @@ -107,13 +107,10 @@ export class QuickActionHandler { if (!this.isScanEnabled) { return } - let scanTabId: string | undefined = undefined - - this.tabsStorage.getTabs().forEach((tab) => { - if (tab.type === 'review') { - scanTabId = tab.id - } - }) + const scanTabId = this.tabsStorage + .getTabs() + .reverse() + .find((tab) => tab.type === 'review')?.id if (scanTabId !== undefined) { this.mynahUI.selectTab(scanTabId, eventId || '') @@ -161,7 +158,7 @@ export class QuickActionHandler { if (!this.isTestEnabled) { return } - const testTabId = this.tabsStorage.getTabs().find((tab) => tab.type === 'testgen')?.id + const testTabId = this.tabsStorage.getTabByType('testgen')?.id const realPromptText = chatPrompt.escapedPrompt?.trim() ?? '' if (testTabId !== undefined) { @@ -290,13 +287,7 @@ export class QuickActionHandler { return } - let gumbyTabId: string | undefined = undefined - - this.tabsStorage.getTabs().forEach((tab) => { - if (tab.type === 'gumby') { - gumbyTabId = tab.id - } - }) + const gumbyTabId = this.tabsStorage.getTabByType('gumby')?.id if (gumbyTabId !== undefined) { this.mynahUI.selectTab(gumbyTabId, eventId || '') diff --git a/packages/core/src/amazonq/webview/ui/storages/tabsStorage.ts b/packages/core/src/amazonq/webview/ui/storages/tabsStorage.ts index f9a419fed96..52fcf354e5a 100644 --- a/packages/core/src/amazonq/webview/ui/storages/tabsStorage.ts +++ b/packages/core/src/amazonq/webview/ui/storages/tabsStorage.ts @@ -91,6 +91,13 @@ export class TabsStorage { return Array.from(this.tabs.values()) } + public getTabByType(type: string): Tab | undefined { + // TODO: Is there ever multiple tabs with the same type or is taking the last one unnecesary? + return this.getTabs() + .reverse() + .find((tab) => tab.type === type) + } + public isTabDead(tabID: string): boolean { return this.tabs.get(tabID)?.status === 'dead' } diff --git a/packages/core/src/amazonqDoc/app.ts b/packages/core/src/amazonqDoc/app.ts index 4aba1b9e9bc..1847da2e168 100644 --- a/packages/core/src/amazonqDoc/app.ts +++ b/packages/core/src/amazonqDoc/app.ts @@ -89,7 +89,9 @@ export function init(appContext: AmazonQAppInitContext) { authenticatingSessionIDs = authenticatingSessions.map((session: any) => session.tabID) // We've already authenticated these sessions - authenticatingSessions.forEach((session: any) => (session.isAuthenticating = false)) + for (const session of authenticatingSessions) { + session.isAuthenticating = false + } } messenger.sendAuthenticationUpdate(authenticated, authenticatingSessionIDs) diff --git a/packages/core/src/amazonqFeatureDev/app.ts b/packages/core/src/amazonqFeatureDev/app.ts index 99164f417be..a72851e8ac7 100644 --- a/packages/core/src/amazonqFeatureDev/app.ts +++ b/packages/core/src/amazonqFeatureDev/app.ts @@ -95,7 +95,9 @@ export function init(appContext: AmazonQAppInitContext) { authenticatingSessionIDs = authenticatingSessions.map((session) => session.tabID) // We've already authenticated these sessions - authenticatingSessions.forEach((session) => (session.isAuthenticating = false)) + for (const session of authenticatingSessions) { + session.isAuthenticating = false + } } messenger.sendAuthenticationUpdate(authenticated, authenticatingSessionIDs) diff --git a/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts b/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts index b120aae986b..045c38c52ea 100644 --- a/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts +++ b/packages/core/src/amazonqGumby/chat/controller/messenger/messenger.ts @@ -194,16 +194,13 @@ export class Messenger { } public async sendLanguageUpgradeProjectPrompt(projects: TransformationCandidateProject[], tabID: string) { - const projectFormOptions: { value: any; label: string }[] = [] - const detectedJavaVersions = new Array() - - projects.forEach((candidateProject) => { - projectFormOptions.push({ + const projectFormOptions: { value: any; label: string }[] = projects.map((candidateProject) => { + return { value: candidateProject.path, label: candidateProject.name, - }) - detectedJavaVersions.push(candidateProject.JDKVersion) + } }) + const detectedJavaVersions = projects.map((candidateProject) => candidateProject.JDKVersion) const formItems: ChatItemFormItem[] = [] formItems.push({ @@ -277,13 +274,11 @@ export class Messenger { } public async sendSQLConversionProjectPrompt(projects: TransformationCandidateProject[], tabID: string) { - const projectFormOptions: { value: any; label: string }[] = [] - - projects.forEach((candidateProject) => { - projectFormOptions.push({ + const projectFormOptions: { value: any; label: string }[] = projects.map((candidateProject) => { + return { value: candidateProject.path, label: candidateProject.name, - }) + } }) const formItems: ChatItemFormItem[] = [] @@ -659,14 +654,12 @@ ${codeSnippet} public sendDependencyVersionsFoundMessage(versions: DependencyVersions, tabID: string) { const message = MessengerUtils.createAvailableDependencyVersionString(versions) - const valueFormOptions: { value: any; label: string }[] = [] - - versions.allVersions.forEach((version) => { - valueFormOptions.push({ + const valueFormOptions: { value: any; label: string }[] = Object.entries(versions.allVersions).map( + ([version, _]) => ({ value: version, label: version, }) - }) + ) const formItems: ChatItemFormItem[] = [] formItems.push({ diff --git a/packages/core/src/amazonqTest/chat/controller/controller.ts b/packages/core/src/amazonqTest/chat/controller/controller.ts index 79d4d117057..8c99e7a2dcb 100644 --- a/packages/core/src/amazonqTest/chat/controller/controller.ts +++ b/packages/core/src/amazonqTest/chat/controller/controller.ts @@ -667,7 +667,7 @@ export class TestController { const fileName = path.basename(session.generatedFilePath) const time = new Date().toLocaleString() // TODO: this is duplicated in basicCommands.ts for scan (codewhisperer). Fix this later. - session.references.forEach((reference) => { + for (const reference of session.references) { getLogger().debug('Processing reference: %O', reference) // Log values for debugging getLogger().debug('updatedContent: %s', updatedContent) @@ -702,7 +702,7 @@ export class TestController { '
' getLogger().debug('Adding reference log: %s', referenceLog) ReferenceLogViewProvider.instance.addReferenceLog(referenceLog) - }) + } // TODO: see if there's a better way to check if active file is a diff if (vscode.window.tabGroups.activeTabGroup.activeTab?.label.includes(amazonQTabSuffix)) { @@ -1241,7 +1241,7 @@ export class TestController { groupName ) if (session.listOfTestGenerationJobId.length && groupName) { - session.listOfTestGenerationJobId.forEach((id) => { + for (const id of session.listOfTestGenerationJobId) { if (id === session.acceptedJobId) { TelemetryHelper.instance.sendTestGenerationEvent( groupName, @@ -1267,7 +1267,7 @@ export class TestController { 0 ) } - }) + } } session.listOfTestGenerationJobId = [] session.testGenerationJobGroupName = undefined diff --git a/packages/core/src/applicationcomposer/composerWebview.ts b/packages/core/src/applicationcomposer/composerWebview.ts index 94493091c3a..546dee4fdff 100644 --- a/packages/core/src/applicationcomposer/composerWebview.ts +++ b/packages/core/src/applicationcomposer/composerWebview.ts @@ -104,9 +104,9 @@ export class ApplicationComposer { } this.isPanelDisposed = true this.onVisualizationDisposeEmitter.fire() - this.disposables.forEach((disposable) => { - disposable.dispose() - }) + for (const d of this.disposables) { + d.dispose() + } this.onVisualizationDisposeEmitter.dispose() } diff --git a/packages/core/src/auth/credentials/sharedCredentials.ts b/packages/core/src/auth/credentials/sharedCredentials.ts index 1b7ea038653..d26661b7cd1 100644 --- a/packages/core/src/auth/credentials/sharedCredentials.ts +++ b/packages/core/src/auth/credentials/sharedCredentials.ts @@ -10,6 +10,7 @@ import { assertHasProps } from '../../shared/utilities/tsUtils' import { getConfigFilename, getCredentialsFilename } from './sharedCredentialsFile' import { SectionName, StaticProfile } from './types' import { UserCredentialsUtils } from '../../shared/credentials/userCredentialsUtils' +import { enumerate } from '../../shared/utilities/collectionUtils' export async function updateAwsSdkLoadConfigEnvVar(): Promise { const configFileExists = await fs.exists(getConfigFilename()) @@ -176,7 +177,8 @@ export function mergeAndValidateSections(data: BaseSection[]): ParseResult { export function parseIni(iniData: string, source: vscode.Uri): BaseSection[] { const sections = [] as BaseSection[] const lines = iniData.split(/\r?\n/).map((l) => l.split(/(^|\s)[;#]/)[0]) // remove comments - lines.forEach((line, lineNumber) => { + for (const item of enumerate(lines)) { + const [line, lineNumber] = item const section = line.match(/^\s*\[([^\[\]]+)]\s*$/) const currentSection: BaseSection | undefined = sections[sections.length - 1] if (section) { @@ -195,7 +197,7 @@ export function parseIni(iniData: string, source: vscode.Uri): BaseSection[] { }) } } - }) + } return sections } diff --git a/packages/core/src/auth/credentials/validation.ts b/packages/core/src/auth/credentials/validation.ts index e1bee0b8c08..856d51a459a 100644 --- a/packages/core/src/auth/credentials/validation.ts +++ b/packages/core/src/auth/credentials/validation.ts @@ -36,12 +36,12 @@ export function getCredentialsErrors( validateFunc: GetCredentialError = getCredentialError ): CredentialsErrors | undefined { const errors: CredentialsData = {} - Object.entries(data).forEach(([key, value]) => { - if (!isCredentialsKey(key)) { - return + for (const item of Object.entries(data)) { + const [key, value] = item + if (isCredentialsKey(key)) { + errors[key] = validateFunc(key, value) } - errors[key] = validateFunc(key, value) - }) + } const hasErrors = Object.values(errors).some(Boolean) if (!hasErrors) { diff --git a/packages/core/src/auth/sso/cache.ts b/packages/core/src/auth/sso/cache.ts index a9cdfa43ce0..7d43c07da35 100644 --- a/packages/core/src/auth/sso/cache.ts +++ b/packages/core/src/auth/sso/cache.ts @@ -149,7 +149,9 @@ function getRegistrationCacheFile(ssoCacheDir: string, key: RegistrationKey): st const hash = (startUrl: string, scopes: string[]) => { const shasum = crypto.createHash('sha256') shasum.update(startUrl) - scopes.forEach((s) => shasum.update(s)) + for (const s of scopes) { + shasum.update(s) + } return shasum.digest('hex') } diff --git a/packages/core/src/auth/sso/server.ts b/packages/core/src/auth/sso/server.ts index 7c95b5ce016..cf25c1456d5 100644 --- a/packages/core/src/auth/sso/server.ts +++ b/packages/core/src/auth/sso/server.ts @@ -163,9 +163,9 @@ export class AuthSSOServer { getLogger().debug('AuthSSOServer: Attempting to close server.') - this.connections.forEach((connection) => { + for (const connection of this.connections) { connection.destroy() - }) + } this.server.close((err) => { if (err) { diff --git a/packages/core/src/awsService/accessanalyzer/vue/iamPolicyChecks.ts b/packages/core/src/awsService/accessanalyzer/vue/iamPolicyChecks.ts index da9c0738819..576bf6ce3a9 100644 --- a/packages/core/src/awsService/accessanalyzer/vue/iamPolicyChecks.ts +++ b/packages/core/src/awsService/accessanalyzer/vue/iamPolicyChecks.ts @@ -203,7 +203,7 @@ export class IamPolicyChecksWebview extends VueWebview { span.record({ findingsCount: data.findings.length, }) - data.findings.forEach((finding: AccessAnalyzer.ValidatePolicyFinding) => { + for (const finding of data.findings) { const message = `${finding.findingType}: ${finding.issueCode} - ${finding.findingDetails} Learn more: ${finding.learnMoreLink}` if ((finding.findingType as ValidatePolicyFindingType) === 'ERROR') { diagnostics.push( @@ -240,7 +240,7 @@ export class IamPolicyChecksWebview extends VueWebview { diagnostics ) } - }) + } this.onValidatePolicyResponse.fire([ IamPolicyChecksConstants.ValidatePolicySuccessWithFindings, getResultCssColor('Warning'), @@ -607,7 +607,6 @@ export class IamPolicyChecksWebview extends VueWebview { } public handleValidatePolicyCliResponse(response: string): number { - let findingsCount = 0 const diagnostics: vscode.Diagnostic[] = [] const jsonOutput = JSON.parse(response) if (jsonOutput.BlockingFindings.length === 0 && jsonOutput.NonBlockingFindings.length === 0) { @@ -616,21 +615,20 @@ export class IamPolicyChecksWebview extends VueWebview { getResultCssColor('Success'), ]) } else { - jsonOutput.BlockingFindings.forEach((finding: any) => { + for (const finding of jsonOutput.BlockingFindings) { this.pushValidatePolicyDiagnostic(diagnostics, finding, true) - findingsCount++ - }) - jsonOutput.NonBlockingFindings.forEach((finding: any) => { + } + for (const finding of jsonOutput.NonBlockingFindings) { this.pushValidatePolicyDiagnostic(diagnostics, finding, false) - findingsCount++ - }) + } + this.onValidatePolicyResponse.fire([ IamPolicyChecksConstants.ValidatePolicySuccessWithFindings, getResultCssColor('Warning'), ]) void vscode.commands.executeCommand('workbench.actions.view.problems') } - return findingsCount + return jsonOutput.BlockingFindings.length + jsonOutput.NonBlockingFindings.length } public async executeCustomPolicyChecksCommand( @@ -682,15 +680,15 @@ export class IamPolicyChecksWebview extends VueWebview { getResultCssColor('Success'), ]) } else { - jsonOutput.BlockingFindings.forEach((finding: any) => { + findingsCount = jsonOutput.BlockingFindings.length + jsonOutput.NonBlockingFindings.length + for (const finding of jsonOutput.BlockingFindings) { this.pushCustomCheckDiagnostic(diagnostics, finding, true) errorMessage = getCheckNoNewAccessErrorMessage(finding) - findingsCount++ - }) - jsonOutput.NonBlockingFindings.forEach((finding: any) => { + } + for (const finding of jsonOutput.NonBlockingFindings) { this.pushCustomCheckDiagnostic(diagnostics, finding, false) - findingsCount++ - }) + } + if (errorMessage) { this.onCustomPolicyCheckResponse.fire([errorMessage, getResultCssColor('Error')]) } else { @@ -724,15 +722,16 @@ export class IamPolicyChecksWebview extends VueWebview { : finding.message const message = `${finding.findingType}: ${findingMessage} - Resource name: ${finding.resourceName}, Policy name: ${finding.policyName}` if (finding.details.reasons) { - finding.details.reasons.forEach((reason: any) => { - diagnostics.push( - new vscode.Diagnostic( - new vscode.Range(0, 0, 0, 0), - message + ` - ${reason.description}`, - isBlocking ? vscode.DiagnosticSeverity.Error : vscode.DiagnosticSeverity.Warning - ) + diagnostics.push( + ...finding.details.reasons.map( + (reason: any) => + new vscode.Diagnostic( + new vscode.Range(0, 0, 0, 0), + message + ` - ${reason.description}`, + isBlocking ? vscode.DiagnosticSeverity.Error : vscode.DiagnosticSeverity.Warning + ) ) - }) + ) } else { diagnostics.push( new vscode.Diagnostic( diff --git a/packages/core/src/awsService/appBuilder/explorer/detectSamProjects.ts b/packages/core/src/awsService/appBuilder/explorer/detectSamProjects.ts index cb179d94f0d..f17bec9213a 100644 --- a/packages/core/src/awsService/appBuilder/explorer/detectSamProjects.ts +++ b/packages/core/src/awsService/appBuilder/explorer/detectSamProjects.ts @@ -21,7 +21,9 @@ export async function detectSamProjects(): Promise { [] ) - projects.forEach((p) => results.set(p.samTemplateUri.toString(), p)) + for (const p of projects) { + results.set(p.samTemplateUri.toString(), p) + } return Array.from(results.values()) } diff --git a/packages/core/src/awsService/appBuilder/wizards/templateParametersWizard.ts b/packages/core/src/awsService/appBuilder/wizards/templateParametersWizard.ts index 75c80c4eda9..9641562d6f0 100644 --- a/packages/core/src/awsService/appBuilder/wizards/templateParametersWizard.ts +++ b/packages/core/src/awsService/appBuilder/wizards/templateParametersWizard.ts @@ -35,7 +35,7 @@ export class TemplateParametersWizard extends Wizard { this.preloadedTemplate = await CloudFormation.load(this.template.fsPath) const samTemplateNames = new Set(this.samTemplateParameters?.keys() ?? []) - samTemplateNames.forEach((name) => { + for (const name of samTemplateNames) { if (this.preloadedTemplate) { const defaultValue = this.preloadedTemplate.Parameters ? (this.preloadedTemplate.Parameters[name]?.Default as string) @@ -47,7 +47,7 @@ export class TemplateParametersWizard extends Wizard { }) ) } - }) + } return this } diff --git a/packages/core/src/awsService/apprunner/activation.ts b/packages/core/src/awsService/apprunner/activation.ts index 14cfb167f75..4ac98feb5fb 100644 --- a/packages/core/src/awsService/apprunner/activation.ts +++ b/packages/core/src/awsService/apprunner/activation.ts @@ -79,16 +79,16 @@ commandMap.set(['aws.apprunner.deleteService', deleteServiceFailed], deleteServi * Activates App Runner */ export async function activate(context: ExtContext): Promise { - commandMap.forEach((command, tuple) => { + for (const [[commandName, errorMsg], command] of commandMap.entries()) { context.extensionContext.subscriptions.push( - Commands.register(tuple[0], async (...args: any) => { + Commands.register(commandName, async (...args: any) => { try { await command(...args) } catch (err) { - getLogger().error(`${tuple[1]}: %s`, err) - await showViewLogsMessage(tuple[1]) + getLogger().error(`${errorMsg}: %s`, err) + await showViewLogsMessage(errorMsg) } }) ) - }) + } } diff --git a/packages/core/src/awsService/apprunner/explorer/apprunnerNode.ts b/packages/core/src/awsService/apprunner/explorer/apprunnerNode.ts index 889f94a51cc..cebe4f8f796 100644 --- a/packages/core/src/awsService/apprunner/explorer/apprunnerNode.ts +++ b/packages/core/src/awsService/apprunner/explorer/apprunnerNode.ts @@ -57,8 +57,7 @@ export class AppRunnerNode extends AWSTreeNodeBase { while (true) { const next = await iterator.next() - - next.value.ServiceSummaryList.forEach((summary: AppRunner.Service) => services.push(summary)) + services.push(...next.value.ServiceSummaryList) if (next.done) { break @@ -86,8 +85,9 @@ export class AppRunnerNode extends AWSTreeNodeBase { deletedNodeArns.delete(summary.ServiceArn) }) ) - - deletedNodeArns.forEach(this.deleteNode.bind(this)) + for (const deletedNodeArn of deletedNodeArns) { + this.deleteNode.bind(this)(deletedNodeArn) + } } public startPollingNode(id: string): void { diff --git a/packages/core/src/awsService/cdk/explorer/detectCdkProjects.ts b/packages/core/src/awsService/cdk/explorer/detectCdkProjects.ts index 9a3fd363d1a..55f47f5938f 100644 --- a/packages/core/src/awsService/cdk/explorer/detectCdkProjects.ts +++ b/packages/core/src/awsService/cdk/explorer/detectCdkProjects.ts @@ -21,7 +21,9 @@ export async function detectCdkProjects( [] ) - projects.forEach((p) => results.set(p.cdkJsonUri.toString(), p)) + for (const p of projects) { + results.set(p.cdkJsonUri.toString(), p) + } return Array.from(results.values()) } diff --git a/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts b/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts index 7f656a6d061..03e196f6925 100644 --- a/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts +++ b/packages/core/src/awsService/cloudWatchLogs/commands/tailLogGroup.ts @@ -77,7 +77,9 @@ export async function tailLogGroup( }) await handleSessionStream(stream, document, session) } finally { - disposables.forEach((disposable) => disposable.dispose()) + for (const d of disposables) { + d.dispose() + } } }) } @@ -138,7 +140,9 @@ async function handleSessionStream( // amount of new lines can push bottom of file out of view before scrolling. const editorsToScroll = getTextEditorsToScroll(document) await updateTextDocumentWithNewLogEvents(formattedLogEvents, document, session.maxLines) - editorsToScroll.forEach(scrollTextEditorToBottom) + for (const e of editorsToScroll) { + scrollTextEditorToBottom(e) + } } session.eventRate = eventRate(event.sessionUpdate) session.isSampled = isSampled(event.sessionUpdate) @@ -200,9 +204,9 @@ async function updateTextDocumentWithNewLogEvents( maxLines: number ) { const edit = new vscode.WorkspaceEdit() - formattedLogEvents.forEach((formattedLogEvent) => - edit.insert(document.uri, new vscode.Position(document.lineCount, 0), formattedLogEvent) - ) + for (const event of formattedLogEvents) { + edit.insert(document.uri, new vscode.Position(document.lineCount, 0), event) + } if (document.lineCount + formattedLogEvents.length > maxLines) { trimOldestLines(formattedLogEvents.length, maxLines, document, edit) } diff --git a/packages/core/src/awsService/cloudWatchLogs/document/logStreamsCodeLensProvider.ts b/packages/core/src/awsService/cloudWatchLogs/document/logStreamsCodeLensProvider.ts index 9870ff009d2..9a8b06a99f1 100644 --- a/packages/core/src/awsService/cloudWatchLogs/document/logStreamsCodeLensProvider.ts +++ b/packages/core/src/awsService/cloudWatchLogs/document/logStreamsCodeLensProvider.ts @@ -50,9 +50,9 @@ export class LogStreamCodeLensProvider implements vscode.CodeLensProvider { const linesToGenerateCodeLens = await this.getStartingLineOfEachStreamId(document) // Create a code lens at the start of each Log Stream in the document - linesToGenerateCodeLens.forEach((idWithLine) => { - codelenses.push(this.createLogStreamCodeLens(logGroupInfo, idWithLine)) - }) + codelenses.push( + ...linesToGenerateCodeLens.map((idWithLine) => this.createLogStreamCodeLens(logGroupInfo, idWithLine)) + ) return codelenses } diff --git a/packages/core/src/awsexplorer/toolView.ts b/packages/core/src/awsexplorer/toolView.ts index 6153e8a1ab3..e3417f25521 100644 --- a/packages/core/src/awsexplorer/toolView.ts +++ b/packages/core/src/awsexplorer/toolView.ts @@ -30,13 +30,13 @@ export function createToolView(viewNode: ToolView): vscode.TreeView { // Cloud9 will only refresh when refreshing the entire tree if (isCloud9()) { - viewNode.nodes.forEach((node) => { + for (const node of viewNode.nodes) { // Refreshes are delayed to guard against excessive calls to `getTreeItem` and `getChildren` // The 10ms delay is arbitrary. A single event loop may be good enough in many scenarios. const refresh = debounce(() => treeDataProvider.refresh(node), 10) node.onDidChangeTreeItem?.(() => refresh()) node.onDidChangeChildren?.(() => refresh()) - }) + } } return view diff --git a/packages/core/src/codecatalyst/explorer.ts b/packages/core/src/codecatalyst/explorer.ts index da41ceede27..a2239d41d89 100644 --- a/packages/core/src/codecatalyst/explorer.ts +++ b/packages/core/src/codecatalyst/explorer.ts @@ -141,7 +141,9 @@ export class CodeCatalystRootNode implements TreeNode { this.addRefreshEmitter(() => this.onDidChangeEmitter.fire()) this.authProvider.onDidChange(() => { - this.refreshEmitters.forEach((fire) => fire()) + for (const fire of this.refreshEmitters) { + fire() + } }) } diff --git a/packages/core/src/codecatalyst/wizards/devenvSettings.ts b/packages/core/src/codecatalyst/wizards/devenvSettings.ts index 590b936bfb2..add70b81648 100644 --- a/packages/core/src/codecatalyst/wizards/devenvSettings.ts +++ b/packages/core/src/codecatalyst/wizards/devenvSettings.ts @@ -50,6 +50,7 @@ export function getInstanceDescription(type: InstanceType): InstanceDescription export function getAllInstanceDescriptions(): { [key: string]: InstanceDescription } { const desc: { [key: string]: InstanceDescription } = {} + // eslint-disable-next-line aws-toolkits/no-foreach entries(devenvOptions.instanceType).forEach(([k]) => (desc[k] = getInstanceDescription(k))) return desc } diff --git a/packages/core/src/codewhisperer/activation.ts b/packages/core/src/codewhisperer/activation.ts index 4034c6397f4..38b00d43f83 100644 --- a/packages/core/src/codewhisperer/activation.ts +++ b/packages/core/src/codewhisperer/activation.ts @@ -721,6 +721,7 @@ function toggleIssuesVisibility(visibleCondition: (issue: CodeScanIssue, filePat issues: group.issues.map((issue) => ({ ...issue, visible: visibleCondition(issue, group.filePath) })), })) securityScanRender.securityDiagnosticCollection?.clear() + // eslint-disable-next-line aws-toolkits/no-foreach updatedIssues.forEach((issue) => updateSecurityDiagnosticCollection(issue)) SecurityIssueProvider.instance.issues = updatedIssues SecurityIssueTreeViewProvider.instance.refresh() diff --git a/packages/core/src/codewhisperer/client/codewhisperer.ts b/packages/core/src/codewhisperer/client/codewhisperer.ts index c5a89b36e0c..6df09db61e3 100644 --- a/packages/core/src/codewhisperer/client/codewhisperer.ts +++ b/packages/core/src/codewhisperer/client/codewhisperer.ts @@ -14,7 +14,7 @@ import { CodeWhispererSettings } from '../util/codewhispererSettings' import { PromiseResult } from 'aws-sdk/lib/request' import { AuthUtil } from '../util/authUtil' import { isSsoConnection } from '../../auth/connection' -import { pageableToCollection } from '../../shared/utilities/collectionUtils' +import { enumerate, pageableToCollection } from '../../shared/utilities/collectionUtils' import apiConfig = require('./service-2.json') import userApiConfig = require('./user-service-2.json') import { session } from '../util/codeWhispererSession' @@ -227,18 +227,18 @@ export class DefaultCodeWhispererClient { const client = await this.createUserSdkClient() const requester = async (request: CodeWhispererUserClient.ListAvailableCustomizationsRequest) => client.listAvailableCustomizations(request).promise() - return pageableToCollection(requester, {}, 'nextToken') + return pageableToCollection(requester, {}, 'nextToken' as never) .promise() .then((resps) => { let logStr = 'amazonq: listAvailableCustomizations API request:' - resps.forEach((resp) => { + for (const resp of resps) { const requestId = resp.$response.requestId logStr += `\n${indent('RequestID: ', 4)}${requestId},\n${indent('Customizations:', 4)}` - resp.customizations.forEach((c, index) => { + for (const [c, index] of enumerate(resp.customizations)) { const entry = `${index.toString().padStart(2, '0')}: ${c.name?.trim()}` logStr += `\n${indent(entry, 8)}` - }) - }) + } + } getLogger().debug(logStr) return resps }) diff --git a/packages/core/src/codewhisperer/commands/basicCommands.ts b/packages/core/src/codewhisperer/commands/basicCommands.ts index 8ec54d02a2a..00fcedff27c 100644 --- a/packages/core/src/codewhisperer/commands/basicCommands.ts +++ b/packages/core/src/codewhisperer/commands/basicCommands.ts @@ -489,7 +489,7 @@ export const applySecurityFix = Commands.declare( const fileName = path.basename(targetFilePath) const time = new Date().toLocaleString() // TODO: this is duplicated in controller.ts for test. Fix this later. - suggestedFix.references?.forEach((reference) => { + for (const reference of suggestedFix.references ?? []) { getLogger().debug('Processing reference: %O', reference) // Log values for debugging getLogger().debug('suggested fix code: %s', suggestedFix.code) @@ -524,7 +524,7 @@ export const applySecurityFix = Commands.declare( '
' getLogger().debug('Adding reference log: %s', referenceLog) ReferenceLogViewProvider.instance.addReferenceLog(referenceLog) - }) + } removeDiagnostic(document.uri, targetIssue) SecurityIssueProvider.instance.removeIssue(document.uri, targetIssue) diff --git a/packages/core/src/codewhisperer/service/completionProvider.ts b/packages/core/src/codewhisperer/service/completionProvider.ts index 10b90372cb4..c66800383a3 100644 --- a/packages/core/src/codewhisperer/service/completionProvider.ts +++ b/packages/core/src/codewhisperer/service/completionProvider.ts @@ -15,11 +15,12 @@ import path from 'path' * completion provider for intelliSense popup */ export function getCompletionItems(document: vscode.TextDocument, position: vscode.Position) { - const completionItems: vscode.CompletionItem[] = [] - session.recommendations.forEach((recommendation, index) => { - completionItems.push(getCompletionItem(document, position, recommendation, index)) + const completionItems: vscode.CompletionItem[] = session.recommendations.map((recommendation, index) => + getCompletionItem(document, position, recommendation, index) + ) + for (let index: number = 0; index < session.recommendations.length; index++) { session.setSuggestionState(index, 'Showed') - }) + } return completionItems } diff --git a/packages/core/src/codewhisperer/service/diagnosticsProvider.ts b/packages/core/src/codewhisperer/service/diagnosticsProvider.ts index 3fcc9cb8b7d..4a40e22cbbd 100644 --- a/packages/core/src/codewhisperer/service/diagnosticsProvider.ts +++ b/packages/core/src/codewhisperer/service/diagnosticsProvider.ts @@ -35,10 +35,10 @@ export function initSecurityScanRender( } else if (scope === CodeAnalysisScope.PROJECT) { securityScanRender.securityDiagnosticCollection?.clear() } - securityRecommendationList.forEach((securityRecommendation) => { + for (const securityRecommendation of securityRecommendationList) { updateSecurityDiagnosticCollection(securityRecommendation) updateSecurityIssuesForProviders(securityRecommendation) - }) + } securityScanRender.initialized = true } @@ -58,11 +58,11 @@ export function updateSecurityDiagnosticCollection(securityRecommendation: Aggre const securityDiagnostics: vscode.Diagnostic[] = vscode.languages .getDiagnostics(uri) .filter((diagnostic) => diagnostic.source === codewhispererDiagnosticSourceLabel) - securityRecommendation.issues - .filter((securityIssue) => securityIssue.visible) - .forEach((securityIssue) => { - securityDiagnostics.push(createSecurityDiagnostic(securityIssue)) - }) + securityDiagnostics.push( + ...securityRecommendation.issues + .filter((securityIssue) => securityIssue.visible) + .map((securityIssue) => createSecurityDiagnostic(securityIssue)) + ) securityDiagnosticCollection.set(uri, securityDiagnostics) } @@ -108,7 +108,7 @@ export function disposeSecurityDiagnostic(event: vscode.TextDocumentChangeEvent) } ) - currentSecurityDiagnostics?.forEach((issue) => { + for (const issue of currentSecurityDiagnostics ?? []) { const intersection = changedRange.intersection(issue.range) if ( issue.severity === vscode.DiagnosticSeverity.Warning && @@ -128,7 +128,7 @@ export function disposeSecurityDiagnostic(event: vscode.TextDocumentChangeEvent) ) } newSecurityDiagnostics.push(issue) - }) + } securityScanRender.securityDiagnosticCollection?.set(uri, newSecurityDiagnostics) } diff --git a/packages/core/src/codewhisperer/service/importAdderProvider.ts b/packages/core/src/codewhisperer/service/importAdderProvider.ts index 0c2f0bd3dc8..276736d3c1c 100644 --- a/packages/core/src/codewhisperer/service/importAdderProvider.ts +++ b/packages/core/src/codewhisperer/service/importAdderProvider.ts @@ -54,13 +54,12 @@ export class ImportAdderProvider implements vscode.CodeLensProvider { r.mostRelevantMissingImports!.length > 0 ) { const line = findLineToInsertImportStatement(editor, firstLineOfRecommendation) - let mergedStatements = `` - r.mostRelevantMissingImports?.forEach((i) => { - // trust service response that this to-be-added import is necessary - if (i.statement) { - mergedStatements += i.statement + '\n' - } - }) + // trust service response that this to-be-added import is necessary + const mergedStatements = r.mostRelevantMissingImports + ?.filter((i) => i.statement) + .reduce((merged, i) => { + return merged + i.statement + '\n' + }, '') await editor.edit( (builder) => { builder.insert(new vscode.Position(line, 0), mergedStatements) diff --git a/packages/core/src/codewhisperer/service/recommendationHandler.ts b/packages/core/src/codewhisperer/service/recommendationHandler.ts index bbcd177a03b..67f55929694 100644 --- a/packages/core/src/codewhisperer/service/recommendationHandler.ts +++ b/packages/core/src/codewhisperer/service/recommendationHandler.ts @@ -44,6 +44,7 @@ import { openUrl } from '../../shared/utilities/vsCodeUtils' import { indent } from '../../shared/utilities/textUtilities' import path from 'path' import { isIamConnection } from '../../auth/connection' +import { enumerate } from '../../shared/utilities/collectionUtils' /** * This class is for getRecommendation/listRecommendation API calls and its states @@ -309,10 +310,10 @@ export class RecommendationHandler { 4, true ).trimStart() - recommendations.forEach((item, index) => { + for (const [index, item] of recommendations.entries()) { msg += `\n ${index.toString().padStart(2, '0')}: ${indent(item.content, 8, true).trim()}` session.requestIdList.push(requestId) - }) + } getLogger().debug(msg) if (invocationResult === 'Succeeded') { CodeWhispererCodeCoverageTracker.getTracker(session.language)?.incrementServiceInvocationCount() @@ -368,7 +369,7 @@ export class RecommendationHandler { TelemetryHelper.instance.setTypeAheadLength(typedPrefix.length) // mark suggestions that does not match typeahead when arrival as Discard // these suggestions can be marked as Showed if typeahead can be removed with new inline API - recommendations.forEach((r, i) => { + for (const [r, i] of enumerate(recommendations)) { const recommendationIndex = i + session.recommendations.length if ( !r.content.startsWith(typedPrefix) && @@ -377,7 +378,7 @@ export class RecommendationHandler { session.setSuggestionState(recommendationIndex, 'Discard') } session.setCompletionType(recommendationIndex, r) - }) + } session.recommendations = pagination ? session.recommendations.concat(recommendations) : recommendations if (isInlineCompletionEnabled() && this.hasAtLeastOneValidSuggestion(typedPrefix)) { this._onDidReceiveRecommendation.fire() @@ -472,9 +473,9 @@ export class RecommendationHandler { } reportDiscardedUserDecisions() { - session.recommendations.forEach((r, i) => { + for (const [i, _] of session.suggestionStates.entries()) { session.setSuggestionState(i, 'Discard') - }) + } this.reportUserDecisions(-1) } diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 3ee3f3d274f..e7ff2d89004 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -576,3 +576,7 @@ export function addAll(s: Set, items: T[]) { s.add(item) } } + +export function enumerate(a: T[]) { + return a.map((value, index) => [value, index] as const) +} From e0447dbbb2029f05f302a4dd024f78338051a7a8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Dec 2024 18:25:09 -0500 Subject: [PATCH 19/19] continued migration efforts --- .../service/recommendationHandler.ts | 18 ++++++------ .../service/securityScanHandler.ts | 17 +++++------ .../transformByQ/transformApiHandler.ts | 29 ++++++++++--------- .../transformByQ/transformFileHandler.ts | 8 ++--- .../transformationResultsViewProvider.ts | 12 ++++---- .../codewhispererCodeCoverageTracker.ts | 4 +-- .../core/src/codewhisperer/util/authUtil.ts | 4 ++- .../codewhisperer/util/closingBracketUtil.ts | 13 +++++---- .../codewhisperer/util/customizationUtil.ts | 1 + .../src/codewhisperer/util/editorContext.ts | 4 +-- .../src/codewhisperer/util/licenseUtil.ts | 8 ++--- 11 files changed, 60 insertions(+), 58 deletions(-) diff --git a/packages/core/src/codewhisperer/service/recommendationHandler.ts b/packages/core/src/codewhisperer/service/recommendationHandler.ts index 67f55929694..fed47e76bad 100644 --- a/packages/core/src/codewhisperer/service/recommendationHandler.ts +++ b/packages/core/src/codewhisperer/service/recommendationHandler.ts @@ -527,9 +527,7 @@ export class RecommendationHandler { // do not show recommendation if cursor is before invocation position // also mark as Discard if (editor.selection.active.isBefore(session.startPos)) { - session.recommendations.forEach((r, i) => { - session.setSuggestionState(i, 'Discard') - }) + this.discardSuggestions() reject() return false } @@ -545,9 +543,7 @@ export class RecommendationHandler { ) ) if (!session.recommendations[0].content.startsWith(typedPrefix.trimStart())) { - session.recommendations.forEach((r, i) => { - session.setSuggestionState(i, 'Discard') - }) + this.discardSuggestions() reject() return false } @@ -577,6 +573,12 @@ export class RecommendationHandler { this.next.dispose() } + private discardSuggestions() { + for (const [i, _] of session.suggestionStates.entries()) { + session.setSuggestionState(i, 'Discard') + } + } + // These commands override the vs code inline completion commands // They are subscribed when suggestion starts and disposed when suggestion is accepted/rejected // to avoid impacting other plugins or user who uses this API @@ -669,9 +671,7 @@ export class RecommendationHandler { editor.selection.active.isBefore(session.startPos) || editor.document.uri.fsPath !== this.documentUri?.fsPath ) { - session.recommendations.forEach((r, i) => { - session.setSuggestionState(i, 'Discard') - }) + this.discardSuggestions() this.reportUserDecisions(-1) } else if (session.recommendations.length > 0) { await this.showRecommendation(0, true) diff --git a/packages/core/src/codewhisperer/service/securityScanHandler.ts b/packages/core/src/codewhisperer/service/securityScanHandler.ts index d328034d560..37fd2353b9d 100644 --- a/packages/core/src/codewhisperer/service/securityScanHandler.ts +++ b/packages/core/src/codewhisperer/service/securityScanHandler.ts @@ -68,13 +68,13 @@ export async function listScanResults( return resp.codeAnalysisFindings }) .promise() - issues.forEach((issue) => { + for (const issue of issues) { mapToAggregatedList(codeScanIssueMap, issue, editor, scope) - }) - codeScanIssueMap.forEach((issues, key) => { + } + for (const [key, issues] of codeScanIssueMap.entries()) { // Project path example: /Users/username/project // Key example: project/src/main/java/com/example/App.java - projectPaths.forEach((projectPath) => { + for (const projectPath of projectPaths) { // We need to remove the project path from the key to get the absolute path to the file // Do not use .. in between because there could be multiple project paths in the same parent dir. const filePath = path.join(projectPath, key.split('/').slice(1).join('/')) @@ -85,7 +85,7 @@ export async function listScanResults( } aggregatedCodeScanIssueList.push(aggregatedCodeScanIssue) } - }) + } const maybeAbsolutePath = `/${key}` if (existsSync(maybeAbsolutePath) && statSync(maybeAbsolutePath).isFile()) { const aggregatedCodeScanIssue: AggregatedCodeScanIssue = { @@ -94,7 +94,7 @@ export async function listScanResults( } aggregatedCodeScanIssueList.push(aggregatedCodeScanIssue) } - }) + } return aggregatedCodeScanIssueList } @@ -157,8 +157,7 @@ export function mapToAggregatedList( } return true }) - - filteredIssues.forEach((issue) => { + for (const issue of filteredIssues) { const filePath = issue.filePath if (codeScanIssueMap.has(filePath)) { if (!isExistingIssue(issue, codeScanIssueMap)) { @@ -169,7 +168,7 @@ export function mapToAggregatedList( } else { codeScanIssueMap.set(filePath, [issue]) } - }) + } } function isDuplicateIssue(issueA: RawCodeScanIssue, issueB: RawCodeScanIssue) { diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts b/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts index fb90241ff68..6a39dd5b4f1 100644 --- a/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts +++ b/packages/core/src/codewhisperer/service/transformByQ/transformApiHandler.ts @@ -332,6 +332,7 @@ export async function zipCode( }, } // TO-DO: later consider making this add to path.join(zipManifest.dependenciesRoot, 'qct-sct-metadata', entry.entryName) so that it's more organized + // eslint-disable-next-line aws-toolkits/no-foreach metadataZip .getEntries() .forEach((entry) => zip.addFile(path.join(zipManifest.dependenciesRoot, entry.name), entry.getData())) @@ -487,34 +488,34 @@ export function addTableMarkdown(plan: string, stepId: string, tableMapping: { [ const table = JSON.parse(tableObj) plan += `\n\n\n${table.name}\n|` const columns = table.columnNames - columns.forEach((columnName: string) => { + for (const columnName of columns) { plan += ` ${getFormattedString(columnName)} |` - }) + } plan += '\n|' - columns.forEach((_: any) => { + for (const _ of columns) { plan += '-----|' - }) - table.rows.forEach((row: any) => { + } + for (const row of table.rows) { plan += '\n|' - columns.forEach((columnName: string) => { + for (const columnName of columns) { if (columnName === 'relativePath') { plan += ` [${row[columnName]}](${row[columnName]}) |` // add MD link only for files } else { plan += ` ${row[columnName]} |` } - }) - }) + } + } plan += '\n\n' return plan } export function getTableMapping(stepZeroProgressUpdates: ProgressUpdates) { const map: { [key: string]: string } = {} - stepZeroProgressUpdates.forEach((update) => { + for (const update of stepZeroProgressUpdates) { // description should never be undefined since even if no data we show an empty table // but just in case, empty string allows us to skip this table without errors when rendering map[update.name] = update.description ?? '' - }) + } return map } @@ -524,11 +525,11 @@ export function getJobStatisticsHtml(jobStatistics: any) { return htmlString } htmlString += `
` - jobStatistics.forEach((stat: { name: string; value: string }) => { + for (const stat of jobStatistics) { htmlString += `

${getFormattedString(stat.name)}: ${stat.value}

` - }) + } htmlString += `
` return htmlString } @@ -573,11 +574,11 @@ export async function getTransformationPlan(jobId: string) { CodeWhispererConstants.planIntroductionMessage }

${getJobStatisticsHtml(jobStatistics)}` plan += `

${CodeWhispererConstants.planHeaderMessage}

${CodeWhispererConstants.planDisclaimerMessage} Read more.

` - response.transformationPlan.transformationSteps.slice(1).forEach((step) => { + for (const step of response.transformationPlan.transformationSteps.slice(1)) { plan += `

${step.name}

Scroll to top

${step.description}

` plan = addTableMarkdown(plan, step.id, tableMapping) plan += `

` - }) + } plan += `

` plan += `

Appendix
Scroll to top


` plan = addTableMarkdown(plan, '-1', tableMapping) // ID of '-1' reserved for appendix table diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformFileHandler.ts b/packages/core/src/codewhisperer/service/transformByQ/transformFileHandler.ts index 516471fc078..17c33b1b272 100644 --- a/packages/core/src/codewhisperer/service/transformByQ/transformFileHandler.ts +++ b/packages/core/src/codewhisperer/service/transformByQ/transformFileHandler.ts @@ -97,14 +97,14 @@ export async function validateSQLMetadataFile(fileContents: string, message: any const serverNodeLocations = sctData['tree']['instances'][0]['ProjectModel'][0]['relations'][0]['server-node-location'] const schemaNames = new Set() - serverNodeLocations.forEach((serverNodeLocation: any) => { + for (const serverNodeLocation of serverNodeLocations) { const schemaNodes = serverNodeLocation['FullNameNodeInfoList'][0]['nameParts'][0][ 'FullNameNodeInfo' ].filter((node: any) => node['$']['typeNode'].toLowerCase() === 'schema') - schemaNodes.forEach((node: any) => { + for (const node of schemaNodes) { schemaNames.add(node['$']['nameNode'].toUpperCase()) - }) - }) + } + } transformByQState.setSchemaOptions(schemaNames) // user will choose one of these getLogger().info( `CodeTransformation: Parsed .sct file with source DB: ${sourceDB}, target DB: ${targetDB}, source host name: ${sourceServerName}, and schema names: ${Array.from(schemaNames)}` diff --git a/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts b/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts index 1f4058a54dc..caa3f9f77d4 100644 --- a/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts +++ b/packages/core/src/codewhisperer/service/transformByQ/transformationResultsViewProvider.ts @@ -145,7 +145,7 @@ export class DiffModel { public copyProject(pathToWorkspace: string, changedFiles: ParsedDiff[]) { const pathToTmpSrcDir = path.join(os.tmpdir(), `project-copy-${Date.now()}`) fs.mkdirSync(pathToTmpSrcDir) - changedFiles.forEach((file) => { + for (const file of changedFiles) { const pathToTmpFile = path.join(pathToTmpSrcDir, file.oldFileName!.substring(2)) // use mkdirsSync to create parent directories in pathToTmpFile too fs.mkdirSync(path.dirname(pathToTmpFile), { recursive: true }) @@ -154,7 +154,7 @@ export class DiffModel { if (fs.existsSync(pathToOldFile)) { fs.copyFileSync(pathToOldFile, pathToTmpFile) } - }) + } return pathToTmpSrcDir } @@ -243,11 +243,11 @@ export class DiffModel { } public saveChanges() { - this.patchFileNodes.forEach((patchFileNode) => { - patchFileNode.children.forEach((changeNode) => { + for (const parentFileNode of this.patchFileNodes) { + for (const changeNode of parentFileNode.children) { changeNode.saveChange() - }) - }) + } + } } public rejectChanges() { diff --git a/packages/core/src/codewhisperer/tracker/codewhispererCodeCoverageTracker.ts b/packages/core/src/codewhisperer/tracker/codewhispererCodeCoverageTracker.ts index 925609ce185..d0ad76c26da 100644 --- a/packages/core/src/codewhisperer/tracker/codewhispererCodeCoverageTracker.ts +++ b/packages/core/src/codewhisperer/tracker/codewhispererCodeCoverageTracker.ts @@ -104,12 +104,12 @@ export class CodeWhispererCodeCoverageTracker { // the accepted characters after calculating user modification let unmodifiedAcceptedTokens = 0 for (const filename in this._acceptedTokens) { - this._acceptedTokens[filename].forEach((v) => { + for (const v of this._acceptedTokens[filename]) { if (filename in this._totalTokens && this._totalTokens[filename] >= v.accepted) { unmodifiedAcceptedTokens += v.accepted acceptedTokens += v.text.length } - }) + } } const percentCount = ((acceptedTokens / totalTokens) * 100).toFixed(2) const percentage = Math.round(parseInt(percentCount)) diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index ed2dfd66e6c..ebaa42003f7 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -457,7 +457,9 @@ export class AuthUtil { state[Features.codewhispererCore] = AuthStates.connected } if (isValidAmazonQConnection(conn)) { - Object.values(Features).forEach((v) => (state[v as Feature] = AuthStates.connected)) + for (const v of Object.values(Features)) { + state[v as Feature] = AuthStates.connected + } } } diff --git a/packages/core/src/codewhisperer/util/closingBracketUtil.ts b/packages/core/src/codewhisperer/util/closingBracketUtil.ts index 4273094b58a..c3011fa1df5 100644 --- a/packages/core/src/codewhisperer/util/closingBracketUtil.ts +++ b/packages/core/src/codewhisperer/util/closingBracketUtil.ts @@ -111,13 +111,14 @@ const removeBracketsFromRightContext = async ( } else { await editor.edit( (editBuilder) => { - idxToRemove.forEach((idx) => { - const range = new vscode.Range( - editor.document.positionAt(offset + idx), - editor.document.positionAt(offset + idx + 1) + for (const idx of idxToRemove) { + editBuilder.delete( + new vscode.Range( + editor.document.positionAt(offset + idx), + editor.document.positionAt(offset + idx + 1) + ) ) - editBuilder.delete(range) - }) + } }, { undoStopAfter: false, undoStopBefore: false } ) diff --git a/packages/core/src/codewhisperer/util/customizationUtil.ts b/packages/core/src/codewhisperer/util/customizationUtil.ts index cfaf68b1afd..3c4ffe635e5 100644 --- a/packages/core/src/codewhisperer/util/customizationUtil.ts +++ b/packages/core/src/codewhisperer/util/customizationUtil.ts @@ -328,6 +328,7 @@ export const selectCustomization = async (customization: Customization) => { export const getAvailableCustomizationsList = async () => { const items: Customization[] = [] const response = await codeWhispererClient.listAvailableCustomizations() + // eslint-disable-next-line aws-toolkits/no-foreach response .map((listAvailableCustomizationsResponse) => listAvailableCustomizationsResponse.customizations) .forEach((customizations) => { diff --git a/packages/core/src/codewhisperer/util/editorContext.ts b/packages/core/src/codewhisperer/util/editorContext.ts index 4b58cb4f848..99a15fd1f02 100644 --- a/packages/core/src/codewhisperer/util/editorContext.ts +++ b/packages/core/src/codewhisperer/util/editorContext.ts @@ -216,7 +216,7 @@ function logSupplementalContext(supplementalContext: CodeWhispererSupplementalCo true ).trimStart() - supplementalContext.supplementalContextItems.forEach((context, index) => { + for (const [index, context] of supplementalContext.supplementalContextItems.entries()) { logString += indent(`\nChunk ${index}:\n`, 4, true) logString += indent( `Path: ${context.filePath} @@ -225,7 +225,7 @@ function logSupplementalContext(supplementalContext: CodeWhispererSupplementalCo 8, true ) - }) + } getLogger().debug(logString) } diff --git a/packages/core/src/codewhisperer/util/licenseUtil.ts b/packages/core/src/codewhisperer/util/licenseUtil.ts index 3a49adc6b4a..4e55dbbebb5 100644 --- a/packages/core/src/codewhisperer/util/licenseUtil.ts +++ b/packages/core/src/codewhisperer/util/licenseUtil.ts @@ -471,11 +471,9 @@ export class LicenseUtil { public static getUniqueLicenseNames(references: References | undefined): Set { const n = new Set() - references?.forEach((r) => { - if (r.licenseName) { - n.add(r.licenseName) - } - }) + // eslint-disable-next-line aws-toolkits/no-foreach + references?.filter((r) => r.licenseName).forEach((r) => n.add(r.licenseName!)) + return n } }