diff --git a/package.json b/package.json index bf82dca..0b80067 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ }, { "command": "sfca.runApexGuruAnalysisOnSelectedFile", - "title": "***SFDX: Scan for Performance Issues with ApexGuru***" + "title": "SFDX: Scan for Performance Issues with ApexGuru on Selected File" } ], "configuration": { diff --git a/src/apexguru/apex-guru-service.ts b/src/apexguru/apex-guru-service.ts index ed37aad..e4fb791 100644 --- a/src/apexguru/apex-guru-service.ts +++ b/src/apexguru/apex-guru-service.ts @@ -27,7 +27,7 @@ export async function isApexGuruEnabledInOrg(outputChannel: vscode.LogOutputChan // This could throw an error for a variety of reasons. The API endpoint has not been deployed to the instance, org has no perms, timeouts etc,. // In all of these scenarios, we return false. const errMsg = e instanceof Error ? e.message : e as string; - outputChannel.error('***ApexGuru perm check failed with error:***' + errMsg); + outputChannel.error('Apex Guru perm check failed with error:' + errMsg); outputChannel.show(); return false; } @@ -47,12 +47,11 @@ export async function runApexGuruOnFile(selection: vscode.Uri, runInfo: RunInfo) progress.report(messages.apexGuru.progress); const connection = await CoreExtensionService.getConnection(); const requestId = await initiateApexGuruRequest(selection, outputChannel, connection); - outputChannel.appendLine('***Apex Guru request Id:***' + requestId); + outputChannel.appendLine('Code Analyzer with ApexGuru request Id:' + requestId); const queryResponse: ApexGuruQueryResponse = await pollAndGetApexGuruResponse(connection, requestId, Constants.APEX_GURU_MAX_TIMEOUT_SECONDS, Constants.APEX_GURU_RETRY_INTERVAL_MILLIS); const decodedReport = Buffer.from(queryResponse.report, 'base64').toString('utf8'); - outputChannel.appendLine('***Retrieved analysis report from ApexGuru***:' + decodedReport); const ruleResult = transformStringToRuleResult(selection.fsPath, decodedReport); new DiagnosticManager().displayDiagnostics([selection.fsPath], [ruleResult], diagnosticCollection); @@ -64,7 +63,7 @@ export async function runApexGuruOnFile(selection: vscode.Uri, runInfo: RunInfo) }); } catch (e) { const errMsg = e instanceof Error ? e.message : e as string; - outputChannel.appendLine('***Apex Guru initiate request failed***'); + outputChannel.error('Initial Code Analyzer with ApexGuru request failed.'); outputChannel.appendLine(errMsg); } } @@ -107,8 +106,8 @@ export async function initiateApexGuruRequest(selection: vscode.Uri, outputChann }); if (response.status != 'new' && response.status != 'success') { - outputChannel.warn('***Apex Guru returned unexpected response:***' + response.status); - throw Error('***Apex Guru returned unexpected response:***' + response.status); + outputChannel.warn('Code Analyzer with Apex Guru returned unexpected response:' + response.status); + throw Error('Code Analyzer with Apex Guru returned unexpected response:' + response.status); } const requestId = response.requestId; @@ -129,16 +128,19 @@ export function transformStringToRuleResult(fileName: string, jsonString: string }; reports.forEach(parsed => { - const encodedClassAfter = parsed.properties.find((prop: ApexGuruProperty) => prop.name === 'code_after')?.value; + const encodedCodeBefore = parsed.properties.find((prop: ApexGuruProperty) => prop.name === 'code_before')?.value; + const encodedCodeAfter = parsed.properties.find((prop: ApexGuruProperty) => prop.name === 'code_after')?.value; + const lineNumber = parseInt(parsed.properties.find((prop: ApexGuruProperty) => prop.name === 'line_number')?.value); const violation: ApexGuruViolation = { ruleName: parsed.type, message: parsed.value, severity: 1, category: parsed.type, // Replace with actual category if available - line: parseInt(parsed.properties.find((prop: ApexGuruProperty) => prop.name === 'line_number')?.value), + line: lineNumber, column: 1, - suggestedCode: Buffer.from(encodedClassAfter, 'base64').toString('utf8') + currentCode: encodedCodeBefore ? Buffer.from(encodedCodeBefore, 'base64').toString('utf8') : encodedCodeBefore, + suggestedCode: encodedCodeAfter ? Buffer.from(encodedCodeAfter, 'base64').toString('utf8') : encodedCodeAfter, }; ruleResult.violations.push(violation); diff --git a/src/extension.ts b/src/extension.ts index 0c89c9d..9fd6174 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -149,6 +149,7 @@ export async function activate(context: vscode.ExtensionContext): Promise messages.diagnostics.source && messages.diagnostics.source.isSource(diagnostic.source) && diagnostic.range.isEqual(range)) // Get and use the appropriate fix generator. - .map(diagnostic => this.getFixGenerator(document, diagnostic).generateFixes(processedLines)) + .map(diagnostic => this.getFixGenerator(document, diagnostic).generateFixes(processedLines, document, diagnostic)) // Combine all the fixes into one array. .reduce((acc, next) => [...acc, ...next], []); } @@ -45,6 +45,8 @@ export class Fixer implements vscode.CodeActionProvider { case 'pmd': case 'pmd-custom': return new _PmdFixGenerator(document, diagnostic); + case 'apexguru': + return new _ApexGuruFixGenerator(document, diagnostic); default: return new _NoOpFixGenerator(document, diagnostic); } @@ -73,7 +75,7 @@ abstract class FixGenerator { * Abstract template method for generating fixes. * @abstract */ - public abstract generateFixes(processedLines: Set): vscode.CodeAction[]; + public abstract generateFixes(processedLines: Set, document?: vscode.TextDocument, diagnostic?: vscode.Diagnostic): vscode.CodeAction[]; } /** @@ -87,6 +89,42 @@ export class _NoOpFixGenerator extends FixGenerator { } } +export class _ApexGuruFixGenerator extends FixGenerator { + /** + * Generate an array of fixes, if possible. + * @returns + */ + // eslint-disable-next-line @typescript-eslint/no-unused-vars + public generateFixes(processedLines: Set, document: vscode.TextDocument, diagnostic: vscode.Diagnostic): vscode.CodeAction[] { + console.log(diagnostic); + const fixes: vscode.CodeAction[] = []; + const lineNumber = this.diagnostic.range.start.line; + if (!processedLines.has(lineNumber)) { + fixes.push(this.generateApexGuruSuppresssion(document)) + processedLines.add(lineNumber); + } + return fixes; + } + + public generateApexGuruSuppresssion(document: vscode.TextDocument): vscode.CodeAction { + const suggestedCode = this.diagnostic.relatedInformation[1].message; + + const action = new vscode.CodeAction(messages.fixer.fixWithApexGuruSuggestions, vscode.CodeActionKind.QuickFix); + action.diagnostics = [this.diagnostic]; + + const edit = new vscode.WorkspaceEdit(); + const range = this.diagnostic.range; // Assuming the range is the location of the existing code in the document + const oneLineAbove = new vscode.Position(range.start.line > 1 ? range.start.line - 1 : 0, range.start.character); + edit.insert(document.uri, oneLineAbove, suggestedCode + '\n'); + + // Assign the edit to the action + action.edit = edit; + + return action; + } + +} + /** * FixGenerator to be used for PMD and Custom PMD. * @private Must be exported for testing purposes, but shouldn't be used publicly, hence the leading underscore. diff --git a/src/lib/messages.ts b/src/lib/messages.ts index 9dd22ba..ac016ce 100644 --- a/src/lib/messages.ts +++ b/src/lib/messages.ts @@ -21,7 +21,7 @@ export const messages = { }, apexGuru: { progress: { - message: "Code Analyzer running ApexGuru analysis." + message: "Code Analyzer is running ApexGuru analysis." }, finishedScan: (violationCount: number) => `Scan complete. ${violationCount} violations found.` }, @@ -40,7 +40,7 @@ export const messages = { fixer: { supressOnLine: "Suppress violations on this line.", supressOnClass: "Suppress violations on this class.", - fixWithApexGuruSuggestions: "***Fix violations with suggestions from Apex Guru***" + fixWithApexGuruSuggestions: "Insert ApexGuru suggestions." }, diagnostics: { messageGenerator: (severity: number, message: string) => `Sev${severity}: ${message}`, diff --git a/src/test/suite/apexguru/apex-guru-service.test.ts b/src/test/suite/apexguru/apex-guru-service.test.ts index 446131d..c83471b 100644 --- a/src/test/suite/apexguru/apex-guru-service.test.ts +++ b/src/test/suite/apexguru/apex-guru-service.test.ts @@ -176,6 +176,7 @@ suite('Apex Guru Test Suite', () => { category: 'BestPractices', line: 10, column: 1, + currentCode: undefined, suggestedCode: 'System.out.println("Hello World");' }] }); diff --git a/src/test/suite/fixer.test.ts b/src/test/suite/fixer.test.ts index 0e22e42..7f72999 100644 --- a/src/test/suite/fixer.test.ts +++ b/src/test/suite/fixer.test.ts @@ -7,9 +7,7 @@ import * as vscode from 'vscode'; import {expect} from 'chai'; import path = require('path'); -import Sinon = require('sinon'); -import {messages} from '../../lib/messages'; -import {_NoOpFixGenerator, _PmdFixGenerator} from '../../lib/fixer'; +import {_NoOpFixGenerator, _PmdFixGenerator, _ApexGuruFixGenerator} from '../../lib/fixer'; suite('fixer.ts', () => { // Note: __dirname is used here because it's consistent across file systems. @@ -489,4 +487,126 @@ suite('fixer.ts', () => { }); }); }); + suite('_ApexGuruFixGenerator', () => { + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass1.cls')); + suite('#generateFixes()', () => { + const processedLines = new Set(); + const fileUri = vscode.Uri.file(path.join(codeFixturesPath, 'MyClass1.cls')); + + let doc: vscode.TextDocument; + // Load the document and store its starting contents. + setup(async () => { + doc = await vscode.workspace.openTextDocument(fileUri); + await vscode.window.showTextDocument(doc); + }); + + test('Should generate a suppression fix if line is not processed', async () => { + // Create a fake diagnostic. + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(7, 4), + new vscode.Position(7, 10) + ), + 'some message', + vscode.DiagnosticSeverity.Warning + ); + diag.relatedInformation = [ + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'current code' + ), + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'apex guru suggested code' + ) + ]; + + // Instantiate the fixer. + const fixGenerator: _ApexGuruFixGenerator = new _ApexGuruFixGenerator(doc, diag); + + // Generate fixes. + const fixes: vscode.CodeAction[] = fixGenerator.generateFixes(processedLines, doc, diag); + + // Validate results. + expect(fixes).to.have.lengthOf(1, 'One fix should be offered'); + const fix = fixes[0].edit.get(fileUri)[0]; + expect(fix.newText).to.equal('apex guru suggested code\n', 'The suppression code should match the suggestion'); + expect(fix.range.start.line).to.equal(6, 'The suppression should be added one line above the diagnostic line'); + }); + + test('Should not generate a suppression fix if line is already processed', async () => { + processedLines.add(7); + + // Create a fake diagnostic. + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(7, 4), + new vscode.Position(7, 10) + ), + 'This message is unimportant', + vscode.DiagnosticSeverity.Warning + ); + diag.relatedInformation = [ + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'current code' + ), + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'apex guru suggested code' + ) + ]; + + // Instantiate the fixer. + const fixGenerator: _ApexGuruFixGenerator = new _ApexGuruFixGenerator(doc, diag); + + // Generate fixes. + const fixes: vscode.CodeAction[] = fixGenerator.generateFixes(processedLines, doc, diag); + + // Validate results. + expect(fixes).to.have.lengthOf(0, 'No fix should be offered if the line is already processed'); + }); + }); + + suite('#generateApexGuruSuppresssion()', () => { + let doc: vscode.TextDocument; + // Load the document and store its starting contents. + setup(async () => { + doc = await vscode.workspace.openTextDocument(fileUri); + await vscode.window.showTextDocument(doc); + }); + + test('Should generate the correct ApexGuru suppression code action', async () => { + // Create a fake diagnostic. + const diag = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(7, 4), + new vscode.Position(7, 10) + ), + 'Some message', + vscode.DiagnosticSeverity.Warning + ); + diag.relatedInformation = [ + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'Some other information' + ), + new vscode.DiagnosticRelatedInformation( + new vscode.Location(fileUri, new vscode.Position(0, 0)), + 'apex guru suggested code' + ) + ]; + + // Instantiate the fixer. + const fixGenerator: _ApexGuruFixGenerator = new _ApexGuruFixGenerator(doc, diag); + + // Generate the suppression code action. + const fix = fixGenerator.generateApexGuruSuppresssion(doc); + + // Validate results. + expect(fix.edit.get(fileUri)[0].newText).to.equal('apex guru suggested code\n', 'The suppression code should match the suggestion'); + expect(fix.edit.get(fileUri)[0].range.start.line).to.equal(6, 'The suppression should be added one line above the diagnostic line'); + }); + }); + }); }); diff --git a/src/types.ts b/src/types.ts index 589fb23..50ab2c0 100644 --- a/src/types.ts +++ b/src/types.ts @@ -35,6 +35,7 @@ export type DfaRuleViolation = BaseViolation & { export type ApexGuruViolation = BaseViolation & { line: number; column: number; + currentCode: string; suggestedCode: string; }