Merge remote-tracking branch 'upstream/master' into all-repomap

Will-ShaoHua · Will-ShaoHua · commit 0d18cd0e3900 · 2024-12-11T12:56:19.000-08:00
diff --git a/buildspec/linuxIntegrationTests.yml b/buildspec/linuxIntegrationTests.yml
@@ -51,6 +51,27 @@ phases:
             # Ensure that "docker" group has permissions to the socket.
             # - chown codebuild-user /var/run/docker.sock
             - chmod 666 /var/run/docker.sock
+            # Pull Docker Images for SAM tests
+
+            # Nodejs
+            - |
+                docker pull public.ecr.aws/sam/build-nodejs18.x:latest
+                docker pull public.ecr.aws/sam/build-nodejs20.x:latest
+                docker pull public.ecr.aws/sam/build-nodejs22.x:latest
+            # Java
+            - |
+                docker pull public.ecr.aws/sam/build-java8.al2:latest
+                docker pull public.ecr.aws/sam/build-java11:latest
+                docker pull public.ecr.aws/sam/build-java17:latest
+            # Python
+            - |
+                docker pull public.ecr.aws/sam/build-python3.10:latest
+                docker pull public.ecr.aws/sam/build-python3.11:latest
+                docker pull public.ecr.aws/sam/build-python3.12:latest
+                docker pull public.ecr.aws/sam/build-python3.13:latest
+            # Dotnet
+            - |
+                docker pull public.ecr.aws/sam/build-dotnet6:latest
 
     pre_build:
         commands:
diff --git a/codecov.yml b/codecov.yml
@@ -81,11 +81,6 @@ coverage:
                 target: 80
                 threshold: 5
                 only_pulls: true
-            tests:
-                # Most code in test/ should always be "covered"!
-                target: 95
-                paths:
-                    - '**/test/**'
         patch:
             default:
                 # Note: `default` measures the entire project.
diff --git a/packages/amazonq/.changes/next-release/Bug Fix-53ead65f-187e-47d9-9432-a48200b604d6.json b/packages/amazonq/.changes/next-release/Bug Fix-53ead65f-187e-47d9-9432-a48200b604d6.json
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Code Review: Cleaned up output logs when running /review"
+}
diff --git a/packages/amazonq/.changes/next-release/Bug Fix-64bec56a-f152-4345-b036-0c5ddb0ce5a2.json b/packages/amazonq/.changes/next-release/Bug Fix-64bec56a-f152-4345-b036-0c5ddb0ce5a2.json
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Code Review: Fixed a bug where applying a fix did not update the positions of other issues in the same file."
+}
diff --git a/packages/amazonq/.changes/next-release/Bug Fix-a243e8d5-b494-46a1-9ec0-c8c6af3cbf35.json b/packages/amazonq/.changes/next-release/Bug Fix-a243e8d5-b494-46a1-9ec0-c8c6af3cbf35.json
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Auth: SSO session was bad, but no reauth prompt given"
+}
diff --git a/packages/amazonq/.changes/next-release/Bug Fix-d5c39800-ce37-4f6c-be23-9cc4035d0cef.json b/packages/amazonq/.changes/next-release/Bug Fix-d5c39800-ce37-4f6c-be23-9cc4035d0cef.json
@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "Code Review: Fixed a bug where projects with repeated path names did not scan properly."
+}
diff --git a/packages/amazonq/package.json b/packages/amazonq/package.json
@@ -210,7 +210,8 @@
                     "type": "tree",
                     "id": "aws.amazonq.SecurityIssuesTree",
                     "name": "%AWS.amazonq.security%",
-                    "when": "!aws.isSageMaker && !aws.isWebExtHost && !aws.amazonq.showLoginView"
+                    "when": "!aws.isSageMaker && !aws.isWebExtHost && !aws.amazonq.showLoginView",
+                    "visibility": "collapsed"
                 },
                 {
                     "type": "webview",
diff --git a/packages/amazonq/test/unit/codewhisperer/util/zipUtil.test.ts b/packages/amazonq/test/unit/codewhisperer/util/zipUtil.test.ts
@@ -14,11 +14,13 @@ import { ToolkitError } from 'aws-core-vscode/shared'
 import { LspClient } from 'aws-core-vscode/amazonq'
 import { fs } from 'aws-core-vscode/shared'
 import path from 'path'
+import JSZip from 'jszip'
 
 describe('zipUtil', function () {
     const workspaceFolder = getTestWorkspaceFolder()
     const appRoot = join(workspaceFolder, 'java11-plain-maven-sam-app')
     const appCodePath = join(appRoot, 'HelloWorldFunction', 'src', 'main', 'java', 'helloworld', 'App.java')
+    const appCodePathWithRepeatedProjectName = join(workspaceFolder, 'workspaceFolder', 'App.java')
 
     describe('getProjectPaths', function () {
         it('Should return the correct project paths', function () {
@@ -112,6 +114,30 @@ describe('zipUtil', function () {
             )
             assert.equal(zipMetadata2.lines, zipMetadata.lines + 1)
         })
+
+        it('should handle path with repeated project name for file scan', async function () {
+            const zipMetadata = await zipUtil.generateZip(
+                vscode.Uri.file(appCodePathWithRepeatedProjectName),
+                CodeAnalysisScope.FILE_ON_DEMAND
+            )
+
+            const zipFileData = await fs.readFileBytes(zipMetadata.zipFilePath)
+            const zip = await JSZip.loadAsync(zipFileData)
+            const files = Object.keys(zip.files)
+            assert.ok(files.includes(join('workspaceFolder', 'workspaceFolder', 'App.java')))
+        })
+
+        it('should handle path with repeated project name for project scan', async function () {
+            const zipMetadata = await zipUtil.generateZip(
+                vscode.Uri.file(appCodePathWithRepeatedProjectName),
+                CodeAnalysisScope.PROJECT
+            )
+
+            const zipFileData = await fs.readFileBytes(zipMetadata.zipFilePath)
+            const zip = await JSZip.loadAsync(zipFileData)
+            const files = Object.keys(zip.files)
+            assert.ok(files.includes(join('workspaceFolder', 'workspaceFolder', 'App.java')))
+        })
     })
 
     describe('generateZipTestGen', function () {
diff --git a/packages/core/src/auth/sso/cache.ts b/packages/core/src/auth/sso/cache.ts
@@ -10,11 +10,12 @@ import { getLogger } from '../../shared/logger/logger'
 import fs from '../../shared/fs/fs'
 import { createDiskCache, KeyedCache, mapCache } from '../../shared/utilities/cacheUtils'
 import { stripUndefined } from '../../shared/utilities/collectionUtils'
-import { hasProps, selectFrom } from '../../shared/utilities/tsUtils'
+import { getMissingProps, hasProps, selectFrom } from '../../shared/utilities/tsUtils'
 import { SsoToken, ClientRegistration } from './model'
 import { DevSettings } from '../../shared/settings'
 import { onceChanged } from '../../shared/utilities/functionUtils'
 import globals from '../../shared/extensionGlobals'
+import { ToolkitError } from '../../shared/errors'
 
 interface RegistrationKey {
     readonly startUrl: string
@@ -92,6 +93,12 @@ export function getTokenCache(directory = getCacheDir()): KeyedCache<SsoAccess>
 
         stripUndefined(token)
 
+        // Validate data is not missing.
+        const missingProps = getMissingProps(token, 'accessToken', 'refreshToken')
+        if (missingProps.length > 0) {
+            throw new ToolkitError(`SSO cache data unexpectedly missing props: ${JSON.stringify(missingProps)}`)
+        }
+
         return {
             token,
             registration,
diff --git a/packages/core/src/codewhisperer/commands/basicCommands.ts b/packages/core/src/codewhisperer/commands/basicCommands.ts
@@ -464,7 +464,6 @@ export const applySecurityFix = Commands.declare(
                 new vscode.Range(document.lineAt(0).range.start, document.lineAt(document.lineCount - 1).range.end),
                 updatedContent
             )
-            SecurityIssueProvider.instance.disableEventHandler()
             const isApplied = await vscode.workspace.applyEdit(edit)
             if (isApplied) {
                 void document.save().then((didSave) => {
diff --git a/packages/core/src/codewhisperer/service/securityIssueProvider.ts b/packages/core/src/codewhisperer/service/securityIssueProvider.ts
@@ -12,7 +12,6 @@ export class SecurityIssueProvider {
     }
 
     private _issues: AggregatedCodeScanIssue[] = []
-    private _disableEventHandler: boolean = false
     public get issues() {
         return this._issues
     }
@@ -21,19 +20,11 @@ export class SecurityIssueProvider {
         this._issues = issues
     }
 
-    public disableEventHandler() {
-        this._disableEventHandler = true
-    }
-
     public handleDocumentChange(event: vscode.TextDocumentChangeEvent) {
         // handleDocumentChange function may be triggered while testing by our own code generation.
         if (!event.contentChanges || event.contentChanges.length === 0) {
             return
         }
-        if (this._disableEventHandler) {
-            this._disableEventHandler = false
-            return
-        }
         const { changedRange, lineOffset } = event.contentChanges.reduce(
             (acc, change) => ({
                 changedRange: acc.changedRange.union(change.range),
diff --git a/packages/core/src/codewhisperer/util/gitUtil.ts b/packages/core/src/codewhisperer/util/gitUtil.ts
@@ -3,8 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { showOutputMessage } from '../../shared/utilities/messages'
-import { getLogger, globals, removeAnsi } from '../../shared'
+import { getLogger, removeAnsi } from '../../shared'
 import { ChildProcess, ChildProcessOptions } from '../../shared/utilities/processUtils'
 import { Uri } from 'vscode'
 
@@ -17,10 +16,10 @@ export async function isGitRepo(folder: Uri): Promise<boolean> {
         rejectOnErrorCode: true,
         onStdout: (text) => {
             output += text
-            showOutputMessage(removeAnsi(text), globals.outputChannel)
+            getLogger().verbose(removeAnsi(text))
         },
         onStderr: (text) => {
-            showOutputMessage(removeAnsi(text), globals.outputChannel)
+            getLogger().error(removeAnsi(text))
         },
         spawnOptions: {
             cwd: folder.fsPath,
diff --git a/packages/core/src/codewhisperer/util/zipUtil.ts b/packages/core/src/codewhisperer/util/zipUtil.ts
@@ -22,8 +22,7 @@ import {
 } from '../models/errors'
 import { ZipUseCase } from '../models/constants'
 import { ChildProcess, ChildProcessOptions } from '../../shared/utilities/processUtils'
-import { showOutputMessage } from '../../shared/utilities/messages'
-import { globals, removeAnsi } from '../../shared'
+import { removeAnsi } from '../../shared'
 
 export interface ZipMetadata {
     rootDir: string
@@ -141,14 +140,8 @@ export class ZipUtil {
         return zipFilePath
     }
 
-    protected getZipEntryPath(projectName: string, relativePath: string, useCase?: ZipUseCase) {
-        // Workspaces with multiple folders have the folder names as the root folder,
-        // but workspaces with only a single folder don't. So prepend the workspace folder name
-        // if it is not present.
-        if (useCase === ZipUseCase.TEST_GENERATION) {
-            return path.join(projectName, relativePath)
-        }
-        return relativePath.split('/').shift() === projectName ? relativePath : path.join(projectName, relativePath)
+    protected getZipEntryPath(projectName: string, relativePath: string) {
+        return path.join(projectName, relativePath)
     }
 
     /**
@@ -303,10 +296,10 @@ export class ZipUtil {
             rejectOnErrorCode: false,
             onStdout: (text) => {
                 diffContent += text
-                showOutputMessage(removeAnsi(text), globals.outputChannel)
+                getLogger().verbose(removeAnsi(text))
             },
             onStderr: (text) => {
-                showOutputMessage(removeAnsi(text), globals.outputChannel)
+                getLogger().error(removeAnsi(text))
             },
             spawnOptions: {
                 cwd: projectPath,
@@ -340,10 +333,10 @@ export class ZipUtil {
             rejectOnErrorCode: true,
             onStdout: (text) => {
                 diffContent += text
-                showOutputMessage(removeAnsi(text), globals.outputChannel)
+                getLogger().verbose(removeAnsi(text))
             },
             onStderr: (text) => {
-                showOutputMessage(removeAnsi(text), globals.outputChannel)
+                getLogger().error(removeAnsi(text))
             },
             spawnOptions: {
                 cwd: projectPath,
@@ -423,7 +416,8 @@ export class ZipUtil {
             this.getProjectScanPayloadSizeLimitInBytes()
         )
         for (const file of sourceFiles) {
-            const zipEntryPath = this.getZipEntryPath(file.workspaceFolder.name, file.relativeFilePath, useCase)
+            const projectName = path.basename(file.workspaceFolder.uri.fsPath)
+            const zipEntryPath = this.getZipEntryPath(projectName, file.relativeFilePath)
 
             if (ZipConstants.knownBinaryFileExts.includes(path.extname(file.fileUri.fsPath))) {
                 if (useCase === ZipUseCase.TEST_GENERATION) {
diff --git a/packages/core/src/shared/regions/regionProvider.ts b/packages/core/src/shared/regions/regionProvider.ts
@@ -15,7 +15,7 @@ import { AwsContext } from '../awsContext'
 import { getIdeProperties, isAmazonQ, isCloud9 } from '../extensionUtilities'
 import { ResourceFetcher } from '../resourcefetcher/resourcefetcher'
 import { isSsoConnection } from '../../auth/connection'
-import { Auth } from '../../auth'
+import { Auth } from '../../auth/auth'
 
 export const defaultRegion = 'us-east-1'
 export const defaultPartition = 'aws'
diff --git a/packages/core/src/shared/utilities/cacheUtils.ts b/packages/core/src/shared/utilities/cacheUtils.ts
@@ -116,10 +116,23 @@ export function createDiskCache<V, K>(
                 log('loaded', key)
                 return result
             } catch (error) {
+                // Non-recoverable errors mean there is no usable data.
+                // Recoverable errors mean we can possibly use the data for something like
+                // an SSO token refresh, or to just retry.
+                // Returning undefined implies non-recoverable.
+
+                // -- Non-recoverable Errors --
                 if (isFileNotFoundError(error)) {
                     log('read failed (file not found)', key)
                     return
                 }
+                if (error instanceof SyntaxError) {
+                    // file content was malformed or empty
+                    log(`read failed (invalid JSON)`, key)
+                    return
+                }
+
+                // -- Recoverable Errors --
                 log(`read failed ${error}`, key)
                 throw createDiskCacheError(error, 'LOAD', target, key)
             }
diff --git a/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts b/packages/core/src/test/codewhisperer/commands/transformByQ.test.ts
diff --git a/packages/core/src/test/credentials/sso/cache.test.ts b/packages/core/src/test/credentials/sso/cache.test.ts
diff --git a/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts b/packages/core/src/test/credentials/sso/ssoAccessTokenProvider.test.ts
diff --git a/packages/core/src/testFixtures/workspaceFolder/workspaceFolder/App.java b/packages/core/src/testFixtures/workspaceFolder/workspaceFolder/App.java
diff --git a/packages/core/src/testInteg/sam.test.ts b/packages/core/src/testInteg/sam.test.ts
diff --git a/packages/toolkit/.changes/next-release/Bug Fix-868c7cda-80c1-4947-a498-3792f88622ad.json b/packages/toolkit/.changes/next-release/Bug Fix-868c7cda-80c1-4947-a498-3792f88622ad.json

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +	"type": "Bug Fix",
 +	"description": "Code Review: Cleaned up output logs when running /review"
 +}
Original file line number	Diff line number	Diff line change
`@@ -464,7 +464,6 @@ export const applySecurityFix = Commands.declare(`
`464`	`464`	`new vscode.Range(document.lineAt(0).range.start, document.lineAt(document.lineCount - 1).range.end),`
`465`	`465`	`updatedContent`
`466`	`466`	`)`
`467`		`- SecurityIssueProvider.instance.disableEventHandler()`
`468`	`467`	`const isApplied = await vscode.workspace.applyEdit(edit)`
`469`	`468`	`if (isApplied) {`
`470`	`469`	`void document.save().then((didSave) => {`