refactor: do validation checks AFTER user picks project #4093

dhasani23 · David Hasani · web-flow · commit cce77a223ce0 · 2023-11-28T12:53:52.000-08:00
Problem:
We run a shell command (`javap`) to determine the Java version of each of the
projects open in the user's workspace. Running this command for each open
project is totally unnecessary.

Solution:
Show options to transform any of the projects open in the workspace, and then
once the user has selected a project, the validation checks are run (`javap`
etc.) to check that it is a valid project. The solution is mostly a refactoring
-- the logic to do the validation checks was pre-existing, it is simply moved to
happen after the user selects a project.

Co-authored-by: David Hasani &lt;davhasan@amazon.com&gt;
diff --git a/src/codewhisperer/commands/startTransformByQ.ts b/src/codewhisperer/commands/startTransformByQ.ts
@@ -20,7 +20,8 @@ import {
     pollTransformationJob,
     convertToTimeString,
     convertDateToTimestamp,
-    getValidModules,
+    getOpenProjects,
+    validateProjectSelection,
 } from '../service/transformByQHandler'
 import { QuickPickItem } from 'vscode'
 import { MultiStepInputFlowController } from '../../shared//multiStepInputFlowController'
@@ -59,51 +60,58 @@ interface UserInputState {
     totalSteps: number
     targetLanguage: QuickPickItem
     targetVersion: QuickPickItem
-    module: QuickPickItem
+    project: QuickPickItem
 }
 
-async function collectInputs(validModules: vscode.QuickPickItem[] | undefined) {
+async function collectInputs(validProjects: vscode.QuickPickItem[] | undefined) {
     // const targetLanguages: QuickPickItem[] = CodeWhispererConstants.targetLanguages.map(label => ({ label }))
     const state = {} as Partial<UserInputState>
     // only supporting target language of Java and target version of JDK17 for now, so skip to pickModule prompt
     transformByQState.setTargetJDKVersionToJDK17()
-    await MultiStepInputFlowController.run(input => pickModule(input, state, validModules))
+    await MultiStepInputFlowController.run(input => pickProject(input, state, validProjects))
     // await MultiStepInputFlowController.run(input => pickTargetLanguage(input, state, targetLanguages, validModules))
     return state as UserInputState
 }
 
-async function pickModule(
+async function pickProject(
     input: MultiStepInputFlowController,
     state: Partial<UserInputState>,
-    validModules: vscode.QuickPickItem[] | undefined
+    validProjects: vscode.QuickPickItem[] | undefined
 ) {
     const pick = await input.showQuickPick({
         title: CodeWhispererConstants.transformByQWindowTitle,
         step: DropdownStep.STEP_1,
         totalSteps: DropdownStep.STEP_1,
         placeholder: CodeWhispererConstants.selectModulePrompt,
-        items: validModules!,
+        items: validProjects!,
         shouldResume: () => Promise.resolve(true),
         ignoreFocusOut: false,
     })
-    state.module = pick
-    transformByQState.setModuleName(he.encode(state.module.label)) // encode to avoid HTML injection risk
+    state.project = pick
+    transformByQState.setProjectName(he.encode(state.project.label)) // encode to avoid HTML injection risk
 }
 
 export async function startTransformByQ() {
     await telemetry.amazonq_codeTransformInvoke.run(async span => {
         span.record({ codeTransform_SessionId: codeTransformTelemetryState.getSessionId() })
-        let validModules: vscode.QuickPickItem[] | undefined
+
+        let openProjects: vscode.QuickPickItem[] = []
         try {
-            validModules = await getValidModules()
+            openProjects = await getOpenProjects()
         } catch (err) {
-            getLogger().error('Failed to get valid modules: ', err)
+            getLogger().error('Failed to get open projects: ', err)
             throw err
         }
+        const state = await collectInputs(openProjects)
 
-        span.record({ codeTransform_SourceJavaVersion: transformByQState.getSourceJDKVersion() })
+        try {
+            await validateProjectSelection(state.project)
+        } catch (err) {
+            getLogger().error('Selected project is not Java 8, not Java 11, or does not use Maven', err)
+            throw err
+        }
 
-        const state = await collectInputs(validModules)
+        span.record({ codeTransform_SourceJavaVersion: transformByQState.getSourceJDKVersion() })
 
         const selection = await vscode.window.showWarningMessage(
             CodeWhispererConstants.dependencyDisclaimer,
@@ -116,7 +124,7 @@ export async function startTransformByQ() {
         }
 
         transformByQState.setToRunning()
-        transformByQState.setModulePath(state.module.description!)
+        transformByQState.setProjectPath(state.project.description!)
         sessionPlanProgress['uploadCode'] = StepProgress.Pending
         sessionPlanProgress['buildCode'] = StepProgress.Pending
         sessionPlanProgress['transformCode'] = StepProgress.Pending
@@ -146,7 +154,7 @@ export async function startTransformByQ() {
             throwIfCancelled()
             try {
                 // TODO: we want to track zip failures separately from uploadPayload failures
-                const payloadFileName = await zipCode(state.module.description!)
+                const payloadFileName = await zipCode(state.project.description!)
                 await vscode.commands.executeCommand('aws.amazonq.refresh') // so that button updates
                 uploadId = await uploadPayload(payloadFileName)
             } catch (error) {
@@ -277,11 +285,11 @@ export async function startTransformByQ() {
             vscode.commands.executeCommand('setContext', 'gumby.isTransformAvailable', true)
             const durationInMs = new Date().getTime() - startTime.getTime()
 
-            if (state.module) {
+            if (state.project) {
                 sessionJobHistory = processHistory(
                     sessionJobHistory,
                     convertDateToTimestamp(startTime),
-                    transformByQState.getModuleName(),
+                    transformByQState.getProjectName(),
                     transformByQState.getStatus(),
                     convertToTimeString(durationInMs),
                     transformByQState.getJobId()
diff --git a/src/codewhisperer/models/model.ts b/src/codewhisperer/models/model.ts
@@ -256,8 +256,8 @@ export class ZipManifest {
 export class TransformByQState {
     private transformByQState: TransformByQStatus = TransformByQStatus.NotStarted
 
-    private moduleName: string = ''
-    private modulePath: string = ''
+    private projectName: string = ''
+    private projectPath: string = ''
 
     private jobId: string = ''
 
@@ -296,12 +296,12 @@ export class TransformByQState {
         return this.transformByQState === TransformByQStatus.PartiallySucceeded
     }
 
-    public getModuleName() {
-        return this.moduleName
+    public getProjectName() {
+        return this.projectName
     }
 
-    public getModulePath() {
-        return this.modulePath
+    public getProjectPath() {
+        return this.projectPath
     }
 
     public getJobId() {
@@ -360,12 +360,12 @@ export class TransformByQState {
         this.transformByQState = TransformByQStatus.PartiallySucceeded
     }
 
-    public setModuleName(name: string) {
-        this.moduleName = name
+    public setProjectName(name: string) {
+        this.projectName = name
     }
 
-    public setModulePath(path: string) {
-        this.modulePath = path
+    public setProjectPath(path: string) {
+        this.projectPath = path
     }
 
     public setJobId(id: string) {
diff --git a/src/codewhisperer/service/transformByQHandler.ts b/src/codewhisperer/service/transformByQHandler.ts
@@ -57,75 +57,70 @@ export function throwIfCancelled() {
     }
 }
 
-/*
- * This function searches for a .class file in each opened module. Then it runs javap on the found .class file to get the JDK version
- * for the file, and sets the version in the state variable. Only JDK8 and JDK11 are supported.
- */
-export async function getValidModules() {
+export async function getOpenProjects() {
     const folders = vscode.workspace.workspaceFolders
-    const validModules: vscode.QuickPickItem[] = []
     if (folders === undefined) {
         vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
-        throw Error('No Java projects found since no projects are open')
+        throw new ToolkitError('No Java projects found since no projects are open', { code: 'NoOpenProjects' })
     }
-    let containsSupportedJava = false // workspace must contain Java 8 or Java 11 code for this to be true
-    let containsPomXml = false // workspace must contain a 'pom.xml' file for this to be true
-    let failureReason = 'NoJavaProjectsAvailable'
+    const openProjects: vscode.QuickPickItem[] = []
     for (const folder of folders) {
-        const compiledJavaFiles = await vscode.workspace.findFiles(
-            new vscode.RelativePattern(folder, '**/*.class'),
-            '**/node_modules/**',
-            1
-        )
-        if (compiledJavaFiles.length < 1) {
-            continue
-        }
-        const classFilePath = compiledJavaFiles[0].fsPath
-        const baseCommand = 'javap'
-        const args = ['-v', classFilePath]
-        const spawnResult = spawnSync(baseCommand, args, { shell: false, encoding: 'utf-8' })
-
-        if (spawnResult.error || spawnResult.status !== 0) {
-            failureReason = 'CouldNotRunJavaCommand'
-            continue // if cannot get Java version, move on to other projects in workspace
-        }
-        const majorVersionIndex = spawnResult.stdout.indexOf('major version: ')
-        const javaVersion = spawnResult.stdout.slice(majorVersionIndex + 15, majorVersionIndex + 17).trim()
-        if (javaVersion === CodeWhispererConstants.JDK8VersionNumber) {
-            transformByQState.setSourceJDKVersionToJDK8()
-            containsSupportedJava = true
-        } else if (javaVersion === CodeWhispererConstants.JDK11VersionNumber) {
-            transformByQState.setSourceJDKVersionToJDK11()
-            containsSupportedJava = true
-        } else {
-            continue
-        }
-        const buildFile = await vscode.workspace.findFiles(
-            new vscode.RelativePattern(folder, '**/pom.xml'), // only supporting projects with a pom.xml for now
-            '**/node_modules/**',
-            1
-        )
-        if (buildFile.length < 1) {
-            checkIfGradle(folder)
-            continue
-        } else {
-            containsPomXml = true
-        }
-        validModules.push({ label: folder.name, description: folder.uri.fsPath })
+        openProjects.push({
+            label: folder.name,
+            description: folder.uri.fsPath,
+        })
+    }
+    return openProjects
+}
+
+/*
+ * This function searches for a .class file in the selected project. Then it runs javap on the found .class file to get the JDK version
+ * for the project, and sets the version in the state variable. Only JDK8 and JDK11 are supported. It also ensure a pom.xml file is found,
+ * since only the Maven build system is supported for now.
+ */
+export async function validateProjectSelection(project: vscode.QuickPickItem) {
+    const projectPath = project.description
+    const compiledJavaFiles = await vscode.workspace.findFiles(
+        new vscode.RelativePattern(projectPath!, '**/*.class'),
+        '**/node_modules/**',
+        1
+    )
+    if (compiledJavaFiles.length < 1) {
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        throw new ToolkitError('No Java projects found', { code: 'NoJavaProjectsAvailable' })
     }
-    if (!containsSupportedJava) {
+    const classFilePath = compiledJavaFiles[0].fsPath
+    const baseCommand = 'javap'
+    const args = ['-v', classFilePath]
+    const spawnResult = spawnSync(baseCommand, args, { shell: false, encoding: 'utf-8' })
+
+    if (spawnResult.error || spawnResult.status !== 0) {
         vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
-        throw new ToolkitError('No Java projects found', { code: failureReason })
+        throw new ToolkitError('Unable to determine Java version', { code: 'CannotDetermineJavaVersion' })
     }
-    if (!containsPomXml) {
-        vscode.window.showErrorMessage(CodeWhispererConstants.noPomXmlFoundMessage, { modal: true })
-        throw new ToolkitError('No build file found', { code: 'CouldNotFindPomXml' })
+    const majorVersionIndex = spawnResult.stdout.indexOf('major version: ')
+    const javaVersion = spawnResult.stdout.slice(majorVersionIndex + 15, majorVersionIndex + 17).trim()
+    if (javaVersion === CodeWhispererConstants.JDK8VersionNumber) {
+        transformByQState.setSourceJDKVersionToJDK8()
+    } else if (javaVersion === CodeWhispererConstants.JDK11VersionNumber) {
+        transformByQState.setSourceJDKVersionToJDK11()
     } else {
-        telemetry.amazonq_codeTransformInvoke.record({
-            codeTransform_ProjectType: 'maven',
-        })
+        vscode.window.showErrorMessage(CodeWhispererConstants.noSupportedJavaProjectsFoundMessage, { modal: true })
+        throw new ToolkitError('Project selected is not Java 8 or Java 11', { code: 'UnsupportedJavaVersion' })
     }
-    return validModules
+    const buildFile = await vscode.workspace.findFiles(
+        new vscode.RelativePattern(projectPath!, '**/pom.xml'),
+        '**/node_modules/**',
+        1
+    )
+    if (buildFile.length < 1) {
+        await checkIfGradle(projectPath!)
+        vscode.window.showErrorMessage(CodeWhispererConstants.noPomXmlFoundMessage, { modal: true })
+        throw new ToolkitError('No valid Maven build file found', { code: 'CouldNotFindPomXml' })
+    }
+    telemetry.amazonq_codeTransformInvoke.record({
+        codeTransform_ProjectType: 'maven',
+    })
 }
 
 export function getSha256(fileName: string) {
@@ -384,14 +379,14 @@ export async function pollTransformationJob(jobId: string, validStates: string[]
     return status
 }
 
-async function checkIfGradle(folder: vscode.WorkspaceFolder) {
-    const gradleBuildFiles = await vscode.workspace.findFiles(
-        new vscode.RelativePattern(folder, '**/build.gradle'),
+async function checkIfGradle(projectPath: string) {
+    const gradleBuildFile = await vscode.workspace.findFiles(
+        new vscode.RelativePattern(projectPath, '**/build.gradle'),
         '**/node_modules/**',
         1
     )
 
-    if (gradleBuildFiles.length > 1) {
+    if (gradleBuildFile.length > 0) {
         telemetry.amazonq_codeTransformInvoke.record({
             codeTransform_ProjectType: 'gradle',
         })
diff --git a/src/codewhisperer/service/transformationResultsViewProvider.ts b/src/codewhisperer/service/transformationResultsViewProvider.ts
@@ -303,7 +303,7 @@ export class ProposedTransformationExplorer {
                 diffModel.parseDiff(
                     path.join(pathContainingArchive, ExportResultArchiveStructure.PathToDiffPatch),
                     path.join(pathContainingArchive, ExportResultArchiveStructure.PathToSourceDir),
-                    transformByQState.getModulePath()
+                    transformByQState.getProjectPath()
                 )
 
                 vscode.commands.executeCommand('setContext', 'gumby.reviewState', TransformByQReviewStatus.InReview)
diff --git a/src/test/codewhisperer/commands/transformByQ.test.ts b/src/test/codewhisperer/commands/transformByQ.test.ts
@@ -18,9 +18,9 @@ import {
     convertDateToTimestamp,
     convertToTimeString,
     throwIfCancelled,
-    getValidModules,
     stopJob,
     pollTransformationJob,
+    validateProjectSelection,
 } from '../../../codewhisperer/service/transformByQHandler'
 
 describe('transformByQ', function () {
@@ -68,11 +68,7 @@ describe('transformByQ', function () {
         assert.strictEqual(model.transformByQState.getStatus(), 'Cancelled')
     })
 
-    it('WHEN get valid modules called on valid project THEN correctly extracts JDK8 version', async function () {
-        sinon
-            .stub(vscode.workspace, 'workspaceFolders')
-            .get(() => [{ uri: vscode.Uri.file('/user/sample/project/'), name: 'SampleProject', index: 0 }])
-
+    it('WHEN validateProjectSelection called on valid project THEN correctly extracts JDK8 version', async function () {
         const findFilesStub = sinon.stub(vscode.workspace, 'findFiles')
         findFilesStub.onFirstCall().resolves([vscode.Uri.file('/user/sample/project/ClassFile.class')])
         findFilesStub.onSecondCall().resolves([vscode.Uri.file('/user/sample/project/pom.xml')])
@@ -88,26 +84,31 @@ describe('transformByQ', function () {
         }
         const spawnSyncStub = sinon.stub().returns(spawnSyncResult)
 
-        const { getValidModules } = proxyquire('../../../codewhisperer/service/transformByQHandler', {
+        const { validateProjectSelection } = proxyquire('../../../codewhisperer/service/transformByQHandler', {
             child_process: { spawnSync: spawnSyncStub },
         })
 
-        const validModules = await getValidModules()
-        assert.strictEqual(validModules![0].label, 'SampleProject')
+        const dummyQuickPickItem: vscode.QuickPickItem = {
+            label: 'SampleProject',
+            description: '/dummy/path/here',
+        }
+        await assert.doesNotReject(async () => {
+            await validateProjectSelection(dummyQuickPickItem)
+        })
         assert.strictEqual(model.transformByQState.getSourceJDKVersion(), '8')
     })
 
-    it('WHEN get valid modules called on project with no class files THEN throws error', async function () {
-        sinon
-            .stub(vscode.workspace, 'workspaceFolders')
-            .get(() => [{ uri: vscode.Uri.file('/user/sample/project/'), name: 'SampleProject', index: 0 }])
-
+    it('WHEN validateProjectSelection called on project with no class files THEN throws error', async function () {
         const findFilesStub = sinon.stub(vscode.workspace, 'findFiles')
         findFilesStub.onFirstCall().resolves([])
+        const dummyQuickPickItem: vscode.QuickPickItem = {
+            label: 'SampleProject',
+            description: '/dummy/path/here',
+        }
 
         await assert.rejects(
             async () => {
-                await getValidModules()
+                await validateProjectSelection(dummyQuickPickItem)
             },
             {
                 name: 'Error',

Original file line number	Diff line number	Diff line change
`@@ -256,8 +256,8 @@ export class ZipManifest {`
`256`	`256`	`export class TransformByQState {`
`257`	`257`	`private transformByQState: TransformByQStatus = TransformByQStatus.NotStarted`
`258`	`258`
`259`		`- private moduleName: string = ''`
`260`		`- private modulePath: string = ''`
	`259`	`+ private projectName: string = ''`
	`260`	`+ private projectPath: string = ''`
`261`	`261`
`262`	`262`	`private jobId: string = ''`
`263`	`263`
`@@ -296,12 +296,12 @@ export class TransformByQState {`
`296`	`296`	`return this.transformByQState === TransformByQStatus.PartiallySucceeded`
`297`	`297`	`}`
`298`	`298`
`299`		`- public getModuleName() {`
`300`		`- return this.moduleName`
	`299`	`+ public getProjectName() {`
	`300`	`+ return this.projectName`
`301`	`301`	`}`
`302`	`302`
`303`		`- public getModulePath() {`
`304`		`- return this.modulePath`
	`303`	`+ public getProjectPath() {`
	`304`	`+ return this.projectPath`
`305`	`305`	`}`
`306`	`306`
`307`	`307`	`public getJobId() {`
`@@ -360,12 +360,12 @@ export class TransformByQState {`
`360`	`360`	`this.transformByQState = TransformByQStatus.PartiallySucceeded`
`361`	`361`	`}`
`362`	`362`
`363`		`- public setModuleName(name: string) {`
`364`		`- this.moduleName = name`
	`363`	`+ public setProjectName(name: string) {`
	`364`	`+ this.projectName = name`
`365`	`365`	`}`
`366`	`366`
`367`		`- public setModulePath(path: string) {`
`368`		`- this.modulePath = path`
	`367`	`+ public setProjectPath(path: string) {`
	`368`	`+ this.projectPath = path`
`369`	`369`	`}`
`370`	`370`
`371`	`371`	`public setJobId(id: string) {`
Original file line number	Diff line number	Diff line change
`@@ -303,7 +303,7 @@ export class ProposedTransformationExplorer {`
`303`	`303`	`diffModel.parseDiff(`
`304`	`304`	`path.join(pathContainingArchive, ExportResultArchiveStructure.PathToDiffPatch),`
`305`	`305`	`path.join(pathContainingArchive, ExportResultArchiveStructure.PathToSourceDir),`
`306`		`- transformByQState.getModulePath()`
	`306`	`+ transformByQState.getProjectPath()`
`307`	`307`	`)`
`308`	`308`
`309`	`309`	`vscode.commands.executeCommand('setContext', 'gumby.reviewState', TransformByQReviewStatus.InReview)`