Skip to content

Commit

Permalink
NEW (Extension) @W-16442046@ Apex guru fixer logic - to show and inse…
Browse files Browse the repository at this point in the history
…rt apex guru suggestions (#116)
  • Loading branch information
jag-j authored Aug 19, 2024
1 parent b18dd7b commit 6ac34f3
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 18 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
20 changes: 11 additions & 9 deletions src/apexguru/apex-guru-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export async function activate(context: vscode.ExtensionContext): Promise<vscode
outputChannel: outputChannel
});
});

context.subscriptions.push(runApexGuruOnSelectedFile);
}

Expand Down
19 changes: 18 additions & 1 deletion src/lib/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { PathlessRuleViolation, RuleResult, RuleViolation } from '../types';
import { ApexGuruViolation, PathlessRuleViolation, RuleResult, RuleViolation } from '../types';
import {messages} from './messages';
import * as vscode from 'vscode';

Expand Down Expand Up @@ -89,6 +89,23 @@ export class DiagnosticManager {
target: vscode.Uri.parse(violation.url),
value: violation.ruleName
} : violation.ruleName;
if (engine === 'apexguru') {
const apexGuruViolation = violation as ApexGuruViolation;

if (apexGuruViolation.suggestedCode !== undefined) {
diagnostic.relatedInformation = [
new vscode.DiagnosticRelatedInformation(
new vscode.Location(vscode.Uri.parse('Current Code'), range),
`${apexGuruViolation.currentCode}`
),
new vscode.DiagnosticRelatedInformation(
new vscode.Location(vscode.Uri.parse('ApexGuru Suggestions'), range),
`${apexGuruViolation.suggestedCode}`
)
];
}

}
return diagnostic;
}

Expand Down
42 changes: 40 additions & 2 deletions src/lib/fixer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class Fixer implements vscode.CodeActionProvider {
// Throw out diagnostics that aren't ours, or are for the wrong line.
.filter(diagnostic => 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], []);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -73,7 +75,7 @@ abstract class FixGenerator {
* Abstract template method for generating fixes.
* @abstract
*/
public abstract generateFixes(processedLines: Set<number>): vscode.CodeAction[];
public abstract generateFixes(processedLines: Set<number>, document?: vscode.TextDocument, diagnostic?: vscode.Diagnostic): vscode.CodeAction[];
}

/**
Expand All @@ -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<number>, 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.
Expand Down
4 changes: 2 additions & 2 deletions src/lib/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
},
Expand All @@ -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}`,
Expand Down
1 change: 1 addition & 0 deletions src/test/suite/apexguru/apex-guru-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ suite('Apex Guru Test Suite', () => {
category: 'BestPractices',
line: 10,
column: 1,
currentCode: undefined,
suggestedCode: 'System.out.println("Hello World");'
}]
});
Expand Down
126 changes: 123 additions & 3 deletions src/test/suite/fixer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<number>();
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');
});
});
});
});
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export type DfaRuleViolation = BaseViolation & {
export type ApexGuruViolation = BaseViolation & {
line: number;
column: number;
currentCode: string;
suggestedCode: string;
}

Expand Down

0 comments on commit 6ac34f3

Please sign in to comment.