Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW (Extension) @W-16442046@ Apex guru fixer logic - to show and insert apex guru suggestions #116

Merged
merged 9 commits into from
Aug 19, 2024
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