build(lint): enable no-array-for-each #6281

## Problem Many bugs, and confusing behavior stem from developers using an async function within a forEach. This is almost always not desired. Additionally, `forEach` can lead to bloated implementations when other methods like `some`, `find`, `reduce`, or `flatMap` are more applicable. According to https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-for-each.md It is also faster with `for ... of`. ## Solution - Add lint rule: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-for-each.md - Migrate existing cases with `eslint CLI --fix` (75% of cases). - For remaining cases, mark as exception.
aws · Dec 23, 2024 · 5ef59ca · 5ef59ca
1 parent 745306d
commit 5ef59ca
Show file tree

Hide file tree

Showing 180 changed files with 777 additions and 616 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -158,6 +158,8 @@ module.exports = {
         'unicorn/prefer-reflect-apply': 'error',
         'unicorn/prefer-string-trim-start-end': 'error',
         'unicorn/prefer-type-error': 'error',
+        // Discourage `.forEach` because it can lead to accidental, incorrect use of async callbacks.
+        'unicorn/no-array-for-each': 'error',
         'security-node/detect-child-process': 'error',
 
         'header/header': [

diff --git a/packages/amazonq/src/inlineChat/controller/inlineChatController.ts b/packages/amazonq/src/inlineChat/controller/inlineChatController.ts
@@ -156,7 +156,9 @@ export class InlineChatController {
     }
 
     private async reset() {
-        this.listeners.forEach((listener) => listener.dispose())
+        for (const listener of this.listeners) {
+            listener.dispose()
+        }
         this.listeners = []
 
         this.task = undefined

diff --git 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/e2e/amazonq/explore.test.ts b/packages/amazonq/test/e2e/amazonq/explore.test.ts
@@ -36,10 +36,10 @@ describe('Amazon Q Explore page', function () {
     it('should have correct button IDs', async () => {
         const features = ['featuredev', 'testgen', 'doc', 'review', 'gumby']
 
-        features.forEach((feature, index) => {
+        for (const [index, feature] of features.entries()) {
             const buttons = (tab.getStore().chatItems ?? [])[index].buttons ?? []
             assert.deepStrictEqual(buttons[0].id, `user-guide-${feature}`)
             assert.deepStrictEqual(buttons[1].id, `quick-start-${feature}`)
-        })
+        }
     })
 })
diff --git a/packages/amazonq/test/e2e/amazonq/welcome.test.ts b/packages/amazonq/test/e2e/amazonq/welcome.test.ts
@@ -51,11 +51,11 @@ describe('Amazon Q Welcome page', function () {
             framework.createTab()
 
             let welcomeCount = 0
-            framework.getTabs().forEach((tab) => {
+            for (const tab of framework.getTabs()) {
                 if (tab.getStore().tabTitle === 'Welcome to Q') {
                     welcomeCount++
                 }
-            })
+            }
             // 3 welcome tabs
             assert.deepStrictEqual(welcomeCount, 3)
 

diff --git 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<vscode.TextDocumentContentChangeEvent> {
             return [

diff --git a/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts b/packages/amazonq/test/unit/codewhisperer/util/crossFileContextUtil.test.ts
@@ -251,11 +251,11 @@ describe('crossFileContextUtil', function () {
             const actual = await crossFile.getCrossFileCandidates(editor)
 
             assert.ok(actual.length === 5)
-            actual.forEach((actualFile, index) => {
+            for (const [index, actualFile] of actual.entries()) {
                 const expectedFile = path.join(tempFolder, expectedFilePaths[index])
                 assert.strictEqual(normalize(expectedFile), normalize(actualFile))
                 assert.ok(areEqual(tempFolder, actualFile, expectedFile))
-            })
+            }
         })
     })
 
@@ -274,7 +274,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 })
@@ -287,7 +287,7 @@ describe('crossFileContextUtil', function () {
 
                 assert.ok(actual && actual.supplementalContextItems.length === 0)
             })
-        })
+        }
     })
 
     describe.skip('partial support - crossfile group', function () {
@@ -305,7 +305,7 @@ describe('crossFileContextUtil', 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 })
@@ -318,7 +318,7 @@ describe('crossFileContextUtil', function () {
 
                 assert.ok(actual && actual.supplementalContextItems.length !== 0)
             })
-        })
+        }
     })
 
     describe('full support', function () {
@@ -337,7 +337,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
@@ -361,7 +361,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
@@ -95,12 +95,12 @@ describe('editorContext', function () {
                 ['c', 'c'],
             ])
 
-            languageToExtension.forEach((extension, language) => {
+            for (const [language, extension] of languageToExtension.entries()) {
                 const editor = createMockTextEditor('', 'test.ipynb', language, 1, 17)
                 const actual = EditorContext.getFileRelativePath(editor)
                 const expected = 'test.' + extension
                 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
@@ -52,15 +52,15 @@ describe('runtimeLanguageContext', function () {
             await resetCodeWhispererGlobalVariables()
         })
 
-        cases.forEach((tuple) => {
+        for (const tuple of cases) {
             const languageId = tuple[0]
             const expected = tuple[1]
 
             it(`should ${expected ? '' : 'not'} support ${languageId}`, 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 +105,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 +114,7 @@ describe('runtimeLanguageContext', function () {
                     const actual = languageContext.isLanguageSupported(doc)
                     assert.strictEqual(actual, expected)
                 })
-            })
+            }
         })
     })
 
@@ -148,14 +148,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
@@ -51,15 +51,15 @@ describe('securityScanLanguageContext', function () {
             await resetCodeWhispererGlobalVariables()
         })
 
-        cases.forEach((tuple) => {
+        for (const tuple of cases) {
             const languageId = tuple[0]
             const expected = tuple[1]
 
             it(`should ${expected ? '' : 'not'} support ${languageId}`, 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
@@ -35,6 +35,7 @@ function setupMocha() {
 
 function gatherTestFiles() {
     // Bundles all files in the current directory matching `*.test`
+    // eslint-disable-next-line unicorn/no-array-for-each
     const importAll = (r: __WebpackModuleApi.RequireContext) => r.keys().forEach(r)
     importAll(require.context('.', true, /\.test$/))
 }

diff --git 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<void> {
-    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(
@@ -151,9 +151,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
@@ -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/amazonq/commons/controllers/contentController.ts b/packages/core/src/amazonq/commons/controllers/contentController.ts
@@ -50,13 +50,13 @@ export class EditorContentController {
                 indent = ' '.repeat(indent.length - indent.trimStart().length)
             }
             let textWithIndent = ''
-            text.split('\n').forEach((line, index) => {
+            for (const [index, line] of text.split('\n').entries()) {
                 if (index === 0) {
                     textWithIndent += line
                 } else {
                     textWithIndent += '\n' + indent + line
                 }
-            })
+            }
             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
@@ -46,7 +46,7 @@ export async function computeDiff(leftPath: string, rightPath: string, tabId: st
     let charsRemoved = 0
     let linesAdded = 0
     let linesRemoved = 0
-    changes.forEach((change) => {
+    for (const change of changes) {
         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
@@ -57,6 +57,6 @@ export async function computeDiff(leftPath: string, rightPath: string, tabId: st
             charsRemoved += charCount
             linesRemoved += lineCount
         }
-    })
+    }
     return { changes, charsAdded, linesAdded, charsRemoved, linesRemoved }
 }
diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts
@@ -280,23 +280,25 @@ export class LspController {
     async query(s: string): Promise<RelevantTextDocument[]> {
         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),
-                })
+        if (chunks) {
+            for (const chunk of chunks) {
+                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
     }
 

diff --git a/packages/core/src/amazonq/webview/ui/main.ts b/packages/core/src/amazonq/webview/ui/main.ts
@@ -663,10 +663,10 @@ export const createMynahUI = (
                     ideApi.postMessage(createClickTelemetry('amazonq-disclaimer-acknowledge-button'))
 
                     // remove all disclaimer cards from all tabs
-                    Object.keys(mynahUI.getAllTabs()).forEach((storeTabKey) => {
+                    for (const storeTabKey of Object.keys(mynahUI.getAllTabs())) {
                         // eslint-disable-next-line unicorn/no-null
                         mynahUI.updateStore(storeTabKey, { promptInputStickyCard: null })
-                    })
+                    }
                     return
                 }
                 case 'quick-start': {