Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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-foreach': 'error',
Copy link
Contributor

@justinmk3 justinmk3 Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We depend on eslint-plugin-unicorn so I think we can use this rule: https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-for-each.md

in .eslintrc.js where we enable the rule, let's put a comment like:

// Discourage `.forEach` because it can lead to accidental, incorrect use of async callbacks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! That rule also comes with a way to auto-fix most cases so I switched over to it completely here: #6281

'no-restricted-imports': [
'error',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions packages/amazonq/src/inlineChat/output/computeDiff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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') {
Expand All @@ -49,7 +48,7 @@ export function computeDiff(response: string, inlineTask: InlineTask, isPartialD
}
}
startLine += count
})
}
inlineTask.diff = textDiff
return textDiff
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -221,7 +221,7 @@ describe('keyStrokeHandler', function () {
).checkChangeSource()
assert.strictEqual(actual, expected)
})
})
}

function createFakeDocumentChangeEvent(str: string): ReadonlyArray<vscode.TextDocumentContentChangeEvent> {
return [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
}
})
})

Expand All @@ -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 })
Expand All @@ -277,7 +278,7 @@ describe('crossFileContextUtil', function () {

assert.ok(actual && actual.supplementalContextItems.length === 0)
})
})
}
})

describe.skip('partial support - crossfile group', function () {
Expand All @@ -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 })
Expand All @@ -308,7 +308,7 @@ describe('crossFileContextUtil', function () {

assert.ok(actual && actual.supplementalContextItems.length !== 0)
})
})
}
})

describe('full support', function () {
Expand All @@ -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
Expand All @@ -353,7 +353,7 @@ describe('crossFileContextUtil', function () {

assert.ok(actual && actual.supplementalContextItems.length !== 0)
})
})
}
})

describe('splitFileToChunks', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,15 @@ describe('runtimeLanguageContext', function () {
beforeEach(async 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][] = [
Expand Down Expand Up @@ -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]

Expand All @@ -114,7 +113,7 @@ describe('runtimeLanguageContext', function () {
const actual = languageContext.isLanguageSupported(doc)
assert.strictEqual(actual, expected)
})
})
}
})
})

Expand Down Expand Up @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
1 change: 1 addition & 0 deletions packages/amazonq/test/web/testRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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$/))
}
Expand Down
9 changes: 4 additions & 5 deletions packages/core/scripts/build/generateServiceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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))
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/scripts/lint/testLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down Expand Up @@ -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)
Expand Down
45 changes: 27 additions & 18 deletions packages/core/src/amazonq/commons/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 }
}
Loading
Loading