diff --git a/src/codewhisperer/commands/basicCommands.ts b/src/codewhisperer/commands/basicCommands.ts index 3229ab6407b..7b2b8d8cd00 100644 --- a/src/codewhisperer/commands/basicCommands.ts +++ b/src/codewhisperer/commands/basicCommands.ts @@ -4,14 +4,14 @@ */ import * as vscode from 'vscode' -import { telemetry } from '../../shared/telemetry/telemetry' +import { ApplyFixSource, CodewhispererCodeScanIssueApplyFix, telemetry } from '../../shared/telemetry/telemetry' import { ExtContext } from '../../shared/extensions' import { Commands } from '../../shared/vscode/commands2' import * as CodeWhispererConstants from '../models/constants' import { DefaultCodeWhispererClient } from '../client/codewhisperer' import { startSecurityScanWithProgress, confirmStopSecurityScan } from './startSecurityScan' import { SecurityPanelViewProvider } from '../views/securityPanelViewProvider' -import { CodeScanIssueCommandArgs, codeScanState } from '../models/model' +import { CodeScanIssue, codeScanState } from '../models/model' import { connectToEnterpriseSso, getStartUrl } from '../util/getStartUrl' import { showConnectionPrompt } from '../util/showSsoPrompt' import { ReferenceLogViewProvider } from '../service/referenceLogViewProvider' @@ -33,6 +33,7 @@ import { localize } from '../../shared/utilities/vsCodeUtils' import { applyPatch } from 'diff' import { closeSecurityIssueWebview, showSecurityIssueWebview } from '../views/securityIssue/securityIssueWebview' import { FileSystemCommon } from '../../srcShared/fs' +import { Mutable } from '../../shared/utilities/tsUtils' export const toggleCodeSuggestions = Commands.declare( 'aws.codeWhisperer.toggleCodeSuggestion', @@ -203,8 +204,8 @@ export const refreshStatusBar = Commands.declare( export const openSecurityIssuePanel = Commands.declare( 'aws.codeWhisperer.openSecurityIssuePanel', - (context: ExtContext) => (issue: CodeScanIssueCommandArgs) => { - showSecurityIssueWebview(context.extensionContext, issue) + (context: ExtContext) => (issue: CodeScanIssue, filePath: string) => { + showSecurityIssueWebview(context.extensionContext, issue, filePath) } ) @@ -217,48 +218,54 @@ export const notifyNewCustomizationsCmd = Commands.declare( export const applySecurityFix = Commands.declare( 'aws.codeWhisperer.applySecurityFix', - () => - async ({ filePath, ...issue }: CodeScanIssueCommandArgs) => { - const [suggestedFix] = issue?.suggestedFixes ?? [] + () => async (issue: CodeScanIssue, filePath: string, source: ApplyFixSource) => { + const [suggestedFix] = issue.suggestedFixes + if (!suggestedFix || !filePath) { + return + } - if (!suggestedFix || !filePath) { - return - } + const applyFixTelemetryEntry: Mutable = { + detectorId: issue.detectorId, + findingId: issue.findingId, + applyFixSource: source, + result: 'Succeeded', + } + try { const patch = suggestedFix.code const document = await vscode.workspace.openTextDocument(filePath) const fileContent = document.getText() const updatedContent = applyPatch(fileContent, patch) + if (!updatedContent) { + vscode.window.showErrorMessage(CodeWhispererConstants.codeFixAppliedFailedMessage) + throw Error('Failed to get updated content from applying diff patch') + } - if (updatedContent) { - // saving the document text if not save - document.save().then(isSaved => { - if (isSaved) { - // writing the patch applied version of document into the file - FileSystemCommon.instance - .writeFile(filePath, updatedContent) - .then(() => { - vscode.window - .showInformationMessage(CodeWhispererConstants.codeFixAppliedSuccessMessage, { - title: CodeWhispererConstants.runSecurityScanButtonTitle, - }) - .then(res => { - if (res?.title === CodeWhispererConstants.runSecurityScanButtonTitle) { - vscode.commands.executeCommand('aws.codeWhisperer.security.scan') - } - }) - closeSecurityIssueWebview(issue.findingId) - }) - .catch(err => { - getLogger().error(`Unable to write updated text content into the file: ${err}`) - }) - } else { - getLogger().error('Failed to save editor text changes into the file.') + // saving the document text if not save + const isSaved = await document.save() + if (!isSaved) { + throw Error('Failed to save editor text changes into the file.') + } + + // writing the patch applied version of document into the file + await FileSystemCommon.instance.writeFile(filePath, updatedContent) + vscode.window + .showInformationMessage(CodeWhispererConstants.codeFixAppliedSuccessMessage, { + title: CodeWhispererConstants.runSecurityScanButtonTitle, + }) + .then(res => { + if (res?.title === CodeWhispererConstants.runSecurityScanButtonTitle) { + vscode.commands.executeCommand('aws.codeWhisperer.security.scan') } }) - } else { - vscode.window.showErrorMessage(CodeWhispererConstants.codeFixAppliedFailedMessage) - } + closeSecurityIssueWebview(issue.findingId) + } catch (err) { + getLogger().error(`Apply fix command failed. ${err}`) + applyFixTelemetryEntry.result = 'Failed' + applyFixTelemetryEntry.reason = err as string + } finally { + telemetry.codewhisperer_codeScanIssueApplyFix.emit(applyFixTelemetryEntry) } + } ) diff --git a/src/codewhisperer/models/model.ts b/src/codewhisperer/models/model.ts index 066acf2211c..49ea8814b9e 100644 --- a/src/codewhisperer/models/model.ts +++ b/src/codewhisperer/models/model.ts @@ -218,10 +218,6 @@ export interface CodeScanIssue { suggestedFixes: SuggestedFix[] } -export interface CodeScanIssueCommandArgs extends CodeScanIssue { - filePath: string -} - export interface AggregatedCodeScanIssue { filePath: string issues: CodeScanIssue[] diff --git a/src/codewhisperer/service/securityIssueCodeActionProvider.ts b/src/codewhisperer/service/securityIssueCodeActionProvider.ts index c30d153d90e..3115118a374 100644 --- a/src/codewhisperer/service/securityIssueCodeActionProvider.ts +++ b/src/codewhisperer/service/securityIssueCodeActionProvider.ts @@ -38,7 +38,7 @@ export class SecurityIssueCodeActionProvider extends SecurityIssueProvider imple fixIssue.command = { title: 'Apply suggested fix', command: 'aws.codeWhisperer.applySecurityFix', - arguments: [{ ...issue, filePath: group.filePath }], + arguments: [issue, group.filePath, 'quickfix'], } codeActions.push(fixIssue) } @@ -46,7 +46,7 @@ export class SecurityIssueCodeActionProvider extends SecurityIssueProvider imple openIssue.command = { title: 'Open "CodeWhisperer Security Issue"', command: 'aws.codeWhisperer.openSecurityIssuePanel', - arguments: [{ ...issue, filePath: group.filePath }], + arguments: [issue, group.filePath], } codeActions.push(openIssue) } diff --git a/src/codewhisperer/service/securityIssueHoverProvider.ts b/src/codewhisperer/service/securityIssueHoverProvider.ts index f593ba44e2a..0c25cf16e14 100644 --- a/src/codewhisperer/service/securityIssueHoverProvider.ts +++ b/src/codewhisperer/service/securityIssueHoverProvider.ts @@ -6,7 +6,7 @@ import * as vscode from 'vscode' import { CodeScanIssue } from '../models/model' import globals from '../../shared/extensionGlobals' import { SecurityIssueProvider } from './securityIssueProvider' -import { telemetry } from '../../shared/telemetry/telemetry' +import { ApplyFixSource, telemetry } from '../../shared/telemetry/telemetry' import path from 'path' export class SecurityIssueHoverProvider extends SecurityIssueProvider implements vscode.HoverProvider { @@ -64,10 +64,9 @@ export class SecurityIssueHoverProvider extends SecurityIssueProvider implements markdownString.appendMarkdown(`${issue.description.markdown}\n\n`) + const args = [issue, filePath] const viewDetailsCommand = vscode.Uri.parse( - `command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent( - JSON.stringify({ ...issue, filePath }) - )}` + `command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent(JSON.stringify(args))}` ) markdownString.appendMarkdown( @@ -75,10 +74,9 @@ export class SecurityIssueHoverProvider extends SecurityIssueProvider implements ) if (suggestedFix) { + const args: [CodeScanIssue, string, ApplyFixSource] = [issue, filePath, 'hover'] const applyFixCommand = vscode.Uri.parse( - `command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent( - JSON.stringify({ ...issue, filePath }) - )}` + `command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent(JSON.stringify(args))}` ) markdownString.appendMarkdown(` | [$(wrench) Apply Fix](${applyFixCommand} "Apply suggested fix")\n`) markdownString.appendMarkdown( diff --git a/src/codewhisperer/views/securityIssue/securityIssueWebview.ts b/src/codewhisperer/views/securityIssue/securityIssueWebview.ts index 9dac563fbc4..df2e1e1c265 100644 --- a/src/codewhisperer/views/securityIssue/securityIssueWebview.ts +++ b/src/codewhisperer/views/securityIssue/securityIssueWebview.ts @@ -5,12 +5,13 @@ import * as vscode from 'vscode' import { VueWebview } from '../../../webviews/main' -import { CodeScanIssueCommandArgs } from '../../models/model' +import { CodeScanIssue } from '../../models/model' export class SecurityIssueWebview extends VueWebview { public readonly id = 'aws.codeWhisperer.securityIssue' public readonly source = 'src/codewhisperer/views/securityIssue/vue/index.js' - private issue: CodeScanIssueCommandArgs | undefined + private issue: CodeScanIssue | undefined + private filePath: string | undefined public constructor() { super() @@ -20,12 +21,16 @@ export class SecurityIssueWebview extends VueWebview { return this.issue } - public setIssue(issue: CodeScanIssueCommandArgs) { + public setIssue(issue: CodeScanIssue) { this.issue = issue } + public setFilePath(filePath: string) { + this.filePath = filePath + } + public applyFix() { - vscode.commands.executeCommand('aws.codeWhisperer.applySecurityFix', this.issue) + vscode.commands.executeCommand('aws.codeWhisperer.applySecurityFix', this.issue, this.filePath, 'webview') } public closeWebview(findingId: string) { @@ -38,9 +43,10 @@ export class SecurityIssueWebview extends VueWebview { const Panel = VueWebview.compilePanel(SecurityIssueWebview) let activePanel: InstanceType | undefined -export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, issue: CodeScanIssueCommandArgs) { +export async function showSecurityIssueWebview(ctx: vscode.ExtensionContext, issue: CodeScanIssue, filePath: string) { activePanel ??= new Panel(ctx) activePanel.server.setIssue(issue) + activePanel.server.setFilePath(filePath) const webviewPanel = await activePanel.show({ title: 'CodeWhisperer Security Issue', diff --git a/src/shared/telemetry/vscodeTelemetry.json b/src/shared/telemetry/vscodeTelemetry.json index 0ba7d07b04e..eda0889a375 100644 --- a/src/shared/telemetry/vscodeTelemetry.json +++ b/src/shared/telemetry/vscodeTelemetry.json @@ -47,6 +47,12 @@ "name": "detectorId", "type": "string", "description": "The id of the detector which produced the code scan issue" + }, + { + "name": "applyFixSource", + "type": "string", + "allowedValues": ["hover", "webview", "quickfix"], + "description": "The location which triggered the apply fix command for a code scan issue" } ], "metrics": [ @@ -266,6 +272,11 @@ "name": "codewhisperer_codeScanIssueHover", "description": "Called when a code scan issue is hovered over", "metadata": [{ "type": "findingId" }, { "type": "detectorId" }] + }, + { + "name": "codewhisperer_codeScanIssueApplyFix", + "description": "Called when a code scan issue suggested fix is applied", + "metadata": [{ "type": "findingId" }, { "type": "detectorId" }, { "type": "applyFixSource" }] } ] } diff --git a/src/test/codewhisperer/commands/basicCommands.test.ts b/src/test/codewhisperer/commands/basicCommands.test.ts index 23048646ddb..21c4e1913e5 100644 --- a/src/test/codewhisperer/commands/basicCommands.test.ts +++ b/src/test/codewhisperer/commands/basicCommands.test.ts @@ -8,7 +8,7 @@ import assert from 'assert' import * as sinon from 'sinon' import * as CodeWhispererConstants from '../../../codewhisperer/models/constants' import { createCodeScanIssue, createMockDocument, resetCodeWhispererGlobalVariables } from '../testUtil' -import { assertTelemetryCurried } from '../../testUtil' +import { assertTelemetry, assertTelemetryCurried } from '../../testUtil' import { toggleCodeSuggestions, showSecurityScan, @@ -138,34 +138,46 @@ describe('CodeWhisperer-basicCommands', function () { await targetCommand.execute() assert.strictEqual(getTestWindow().shownMessages[0].message, 'Open a valid file to scan.') }) + }) + + describe('applySecurityFix', function () { + let sandbox: sinon.SinonSandbox + let saveStub: sinon.SinonStub + let openTextDocumentMock: sinon.SinonStub + let writeFileMock: sinon.SinonStub + + beforeEach(function () { + sandbox = sinon.createSandbox() + saveStub = sinon.stub() + openTextDocumentMock = sinon.stub() + writeFileMock = sinon.stub() + }) + + afterEach(function () { + sandbox.restore() + }) it('should call applySecurityFix command successfully', async function () { const fileName = 'sample.py' - const saveStub = sinon.stub() saveStub.resolves(true) const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub) - const openTextDocumentMock = sinon.stub() openTextDocumentMock.resolves(textDocumentMock) - const sandbox = sinon.createSandbox() sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) - const writeFileMock = sinon.stub() writeFileMock.resolves(true) sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock) targetCommand = testCommand(applySecurityFix) - await targetCommand.execute({ - filePath: fileName, - ...createCodeScanIssue({ - suggestedFixes: [ - { - description: 'fix', - code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line', - }, - ], - }), + const codeScanIssue = createCodeScanIssue({ + suggestedFixes: [ + { + description: 'fix', + code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line', + }, + ], }) + await targetCommand.execute(codeScanIssue, fileName, 'hover') assert.ok(saveStub.calledOnce) assert.ok(writeFileMock.calledOnceWith(fileName, 'first line\n third line\n fourth line')) @@ -173,104 +185,113 @@ describe('CodeWhisperer-basicCommands', function () { getTestWindow().shownMessages[0].message, 'Code fix was applied. Run a security scan to validate the fix.' ) - - sandbox.restore() + assertTelemetry('codewhisperer_codeScanIssueApplyFix', { + detectorId: codeScanIssue.detectorId, + findingId: codeScanIssue.findingId, + applyFixSource: 'hover', + result: 'Succeeded', + }) }) + it('handles patch failure', async function () { const textDocumentMock = createMockDocument() - const openTextDocumentMock = sinon.stub() openTextDocumentMock.resolves(textDocumentMock) - const sandbox = sinon.createSandbox() sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) targetCommand = testCommand(applySecurityFix) - await targetCommand.execute({ - filePath: 'test.py', - ...createCodeScanIssue({ - suggestedFixes: [ - { - code: '@@ -1,1 -1,1 @@\n-mock\n+line5', - description: 'dummy', - }, - ], - }), + const codeScanIssue = createCodeScanIssue({ + suggestedFixes: [ + { + code: '@@ -1,1 -1,1 @@\n-mock\n+line5', + description: 'dummy', + }, + ], }) + await targetCommand.execute(codeScanIssue, 'test.py', 'webview') assert.strictEqual(getTestWindow().shownMessages[0].message, 'Failed to apply suggested code fix.') - - sandbox.restore() + assertTelemetry('codewhisperer_codeScanIssueApplyFix', { + detectorId: codeScanIssue.detectorId, + findingId: codeScanIssue.findingId, + applyFixSource: 'webview', + result: 'Failed', + reason: 'Error: Failed to get updated content from applying diff patch', + }) }) - }) - it('handles document save failure', async function () { - const fileName = 'sample.py' - const saveStub = sinon.stub() - saveStub.resolves(false) - const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub) + it('handles document save failure', async function () { + const fileName = 'sample.py' + saveStub.resolves(false) + const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub) - const openTextDocumentMock = sinon.stub() - openTextDocumentMock.resolves(textDocumentMock) + openTextDocumentMock.resolves(textDocumentMock) - const sandbox = sinon.createSandbox() - sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) - const loggerStub = sinon.stub(getLogger(), 'error') + sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) + const loggerStub = sinon.stub(getLogger(), 'error') - targetCommand = testCommand(applySecurityFix) - await targetCommand.execute({ - filePath: fileName, - ...createCodeScanIssue({ + targetCommand = testCommand(applySecurityFix) + const codeScanIssue = createCodeScanIssue({ suggestedFixes: [ { description: 'fix', code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line', }, ], - }), + }) + await targetCommand.execute(codeScanIssue, fileName, 'quickfix') + + assert.ok(saveStub.calledOnce) + assert.ok(loggerStub.calledOnce) + const actual = loggerStub.getCall(0).args[0] + assert.strictEqual( + actual, + 'Apply fix command failed. Error: Failed to save editor text changes into the file.' + ) + assertTelemetry('codewhisperer_codeScanIssueApplyFix', { + detectorId: codeScanIssue.detectorId, + findingId: codeScanIssue.findingId, + applyFixSource: 'quickfix', + result: 'Failed', + reason: 'Error: Failed to save editor text changes into the file.', + }) }) - assert.ok(saveStub.calledOnce) - assert.ok(loggerStub.calledOnce) - const actual = loggerStub.getCall(0).args[0] - assert.strictEqual(actual, 'Failed to save editor text changes into the file.') + it('handles document write failure', async function () { + const fileName = 'sample.py' + saveStub.resolves(true) + const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub) - sandbox.restore() - }) - it('handles document write failure', async function () { - const fileName = 'sample.py' - const saveStub = sinon.stub() - saveStub.resolves(true) - const textDocumentMock = new MockDocument('first line\n second line\n fourth line', fileName, saveStub) - - const openTextDocumentMock = sinon.stub() - openTextDocumentMock.resolves(textDocumentMock) - const writeFileMock = sinon.stub() - writeFileMock.rejects('writing to file failed.') - - const sandbox = sinon.createSandbox() - sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) - sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock) - const loggerStub = sinon.stub(getLogger(), 'error') - - targetCommand = testCommand(applySecurityFix) - await targetCommand.execute({ - filePath: fileName, - ...createCodeScanIssue({ + openTextDocumentMock.resolves(textDocumentMock) + writeFileMock.rejects('Error: Writing to file failed.') + + sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock) + sinon.stub(FileSystemCommon.prototype, 'writeFile').value(writeFileMock) + const loggerStub = sinon.stub(getLogger(), 'error') + + targetCommand = testCommand(applySecurityFix) + const codeScanIssue = createCodeScanIssue({ suggestedFixes: [ { description: 'fix', code: '@@ -1,3 +1,3 @@\n first line\n- second line\n+ third line\n fourth line', }, ], - }), - }) - - assert.ok(saveStub.calledOnce) - assert.ok(loggerStub.calledOnce) - const actual = loggerStub.getCall(0).args[0] - assert.strictEqual(actual, 'Unable to write updated text content into the file: writing to file failed.') + }) + await targetCommand.execute(codeScanIssue, fileName, 'hover') - sandbox.restore() + assert.ok(saveStub.calledOnce) + assert.ok(loggerStub.calledOnce) + const actual = loggerStub.getCall(0).args[0] + assert.strictEqual(actual, 'Apply fix command failed. Error: Writing to file failed.') + assertTelemetry('codewhisperer_codeScanIssueApplyFix', { + detectorId: codeScanIssue.detectorId, + findingId: codeScanIssue.findingId, + applyFixSource: 'hover', + result: 'Failed', + reason: 'Error: Writing to file failed.', + }) + }) }) }) diff --git a/src/test/codewhisperer/service/securityIssueHoverProvider.test.ts b/src/test/codewhisperer/service/securityIssueHoverProvider.test.ts index cc5d5b12d75..541a13bc723 100644 --- a/src/test/codewhisperer/service/securityIssueHoverProvider.test.ts +++ b/src/test/codewhisperer/service/securityIssueHoverProvider.test.ts @@ -45,10 +45,10 @@ describe('securityIssueHoverProvider', () => { '## Suggested Fix for title ![High](severity-high.svg)\n' + 'description\n\n' + `[$(eye) View Details](command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent( - JSON.stringify({ ...issues[0], filePath: mockDocument.fileName }) + JSON.stringify([issues[0], mockDocument.fileName]) )} 'Open "CodeWhisperer Security Issue"')\n` + ` | [$(wrench) Apply Fix](command:aws.codeWhisperer.applySecurityFix?${encodeURIComponent( - JSON.stringify({ ...issues[0], filePath: mockDocument.fileName }) + JSON.stringify([issues[0], mockDocument.fileName, 'hover']) )} "Apply suggested fix")\n\n` + '\n\n' + '```language\n' + @@ -79,7 +79,7 @@ describe('securityIssueHoverProvider', () => { '## title ![High](severity-high.svg)\n' + 'description\n\n' + `[$(eye) View Details](command:aws.codeWhisperer.openSecurityIssuePanel?${encodeURIComponent( - JSON.stringify({ ...issues[1], filePath: mockDocument.fileName }) + JSON.stringify([issues[1], mockDocument.fileName]) )} 'Open "CodeWhisperer Security Issue"')\n` ) assertTelemetry('codewhisperer_codeScanIssueHover', [