Skip to content

Commit

Permalink
NEW(cpd): @W-16866831@: Add in progress events to cpd engine (#120)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephen-carter-at-sf authored Nov 12, 2024
1 parent 91792c2 commit b5122c3
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.salesforce.sfca.cpdwrapper;

import net.sourceforge.pmd.cpd.CPDConfiguration;
import net.sourceforge.pmd.cpd.CPDListener;
import net.sourceforge.pmd.cpd.CpdAnalysis;
import net.sourceforge.pmd.cpd.Mark;
import net.sourceforge.pmd.cpd.Match;
Expand All @@ -24,25 +25,27 @@
* Class to help us invoke CPD - once for each language that should be processed
*/
class CpdRunner {
private final ProgressReporter progressReporter = new ProgressReporter();

public Map<String, CpdLanguageRunResults> run(CpdRunInputData runInputData) throws IOException {
validateRunInputData(runInputData);

Map<String, CpdLanguageRunResults> results = new HashMap<>();
List<String> languagesToProcess = runInputData.filesToScanPerLanguage.entrySet().stream()
.filter(entry -> !entry.getValue().isEmpty()) // Keep only non-empty lists
.map(Map.Entry::getKey)
.collect(Collectors.toList());

for (Map.Entry<String, List<String>> entry : runInputData.filesToScanPerLanguage.entrySet()) {
String language = entry.getKey();
List<String> filesToScan = entry.getValue();
if (filesToScan.isEmpty()) {
continue;
}
progressReporter.initialize(languagesToProcess);

Map<String, CpdLanguageRunResults> results = new HashMap<>();
for (String language : languagesToProcess) {
List<String> filesToScan = runInputData.filesToScanPerLanguage.get(language);
List<Path> pathsToScan = filesToScan.stream().map(Paths::get).collect(Collectors.toList());
CpdLanguageRunResults languageRunResults = runLanguage(language, pathsToScan, runInputData.minimumTokens, runInputData.skipDuplicateFiles);

if (!languageRunResults.matches.isEmpty() || !languageRunResults.processingErrors.isEmpty()) {
results.put(language, languageRunResults);
}
}

return results;
}

Expand All @@ -64,8 +67,15 @@ private CpdLanguageRunResults runLanguage(String language, List<Path> pathsToSca
CpdLanguageRunResults languageRunResults = new CpdLanguageRunResults();

try (CpdAnalysis cpd = CpdAnalysis.create(config)) {
cpd.performAnalysis(report -> {

// Note that we could use cpd.files().getCollectedFiles().size() to get the true totalNumFiles but
// unfortunately getCollectedFiles doesn't cache and does a sort operation which is expensive.
// So instead we use pathsToScan.size() since we send in the list of files instead of folders and so
// these numbers should be the same.
int totalNumFiles = pathsToScan.size();
cpd.setCpdListener(new CpdLanguageRunListener(progressReporter, language, totalNumFiles));

cpd.performAnalysis(report -> {
for (Report.ProcessingError reportProcessingError : report.getProcessingErrors()) {
CpdLanguageRunResults.ProcessingError processingErr = new CpdLanguageRunResults.ProcessingError();
processingErr.file = reportProcessingError.getFileId().getAbsolutePath();
Expand Down Expand Up @@ -134,4 +144,77 @@ public boolean isLoggable(Level level) {
public int numErrors() {
return 0;
}
}

// This class helps us track the overall progress of all language runs
class ProgressReporter {
private Map<String, Float> progressPerLanguage = new HashMap<>();
private float lastReportedProgress = 0.0f;

public void initialize(List<String> languages) {
progressPerLanguage = new HashMap<>();
languages.forEach(l -> this.updateProgressForLanguage(l, 0.0f));
}

public void updateProgressForLanguage(String language, float percComplete) {
progressPerLanguage.put(language, percComplete);
}

public void reportOverallProgress() {
float currentProgress = this.calculateOverallPercentage();
// The progress goes very fast, so we make sure to only report progress if there has been a significant enough increase (at least 1%)
if (currentProgress >= lastReportedProgress + 1) {
System.out.println("[Progress]" + currentProgress);
lastReportedProgress = currentProgress;
}
}

private float calculateOverallPercentage() {
float sum = 0.0f;
for (float progress : progressPerLanguage.values()) {
sum += progress;
}
return sum / progressPerLanguage.size();
}
}

// This class is a specific listener for a run of cpd for a single language.
class CpdLanguageRunListener implements CPDListener {
private final ProgressReporter progressReporter;
private final String language;
private final int totalNumFiles;
private int numFilesAdded = 0;
private int currentPhase = CPDListener.INIT;

public CpdLanguageRunListener(ProgressReporter progressReporter, String language, int totalNumFiles) {
this.progressReporter = progressReporter;
this.language = language;
this.totalNumFiles = totalNumFiles;
}

@Override
public void addedFile(int i) {
// All files are added while we still are on phase 0 - INIT, i.e. before the phase is updated to phase 1 - HASH.
this.numFilesAdded += i;
updateAndReportCompletePercentage();
}

@Override
public void phaseUpdate(int i) {
this.currentPhase = i;
updateAndReportCompletePercentage();
}

private void updateAndReportCompletePercentage() {
this.progressReporter.updateProgressForLanguage(this.language, calculateCompletePercentage());
this.progressReporter.reportOverallProgress();
}

private float calculateCompletePercentage() {
if (this.currentPhase == CPDListener.INIT) {
// Using Math.min just in case the totalNumFiles is inaccurate - although it shouldn't be.
return 25*(Math.min((float) this.numFilesAdded / this.totalNumFiles, 1.0f));
}
return 100 * ((float) this.currentPhase / CPDListener.DONE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,19 @@ void whenCallingRunWithValidFilesThatHaveDuplicates_thenJsonOutputShouldContainR
// Also check that stdOut contains runtime information
assertThat(stdOut, containsString("ARGUMENTS:"));
assertThat(stdOut, containsString("milliseconds"));

// Also check that stdOut contains correct progress information
assertThat(stdOut, allOf(
containsString("[Progress]6.25"),
containsString("[Progress]12.5"),
containsString("[Progress]25.0"),
containsString("[Progress]37.5"),
containsString("[Progress]50.0"),
containsString("[Progress]56.25"),
containsString("[Progress]62.5"),
containsString("[Progress]75.0"),
containsString("[Progress]87.5"),
containsString("[Progress]100.0")));
}

@Test
Expand Down Expand Up @@ -364,7 +377,7 @@ void whenCallingRunWithTwoIdenticalFilesButSkipDuplicateFilesIsFalse_thenJsonOut
String outputFile = tempDir.resolve("output.json").toAbsolutePath().toString();
String[] args = {"run", inputFile, outputFile};

callCpdWrapper(args);
String stdOut = callCpdWrapper(args);

String resultsJsonString = new String(Files.readAllBytes(Paths.get(outputFile)));

Expand Down Expand Up @@ -399,6 +412,13 @@ void whenCallingRunWithTwoIdenticalFilesButSkipDuplicateFilesIsFalse_thenJsonOut
expectedOutput = expectedOutput.replaceAll("\\s+", "");

assertThat(resultsJsonString, is(expectedOutput));

assertThat(stdOut, allOf(
containsString("[Progress]12.5"),
containsString("[Progress]25.0"),
containsString("[Progress]50.0"),
containsString("[Progress]75.0"),
containsString("[Progress]100.0")));
}

@Test
Expand Down
12 changes: 10 additions & 2 deletions packages/code-analyzer-pmd-engine/src/cpd-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,17 @@ export class CpdEngine extends Engine {

async describeRules(describeOptions: DescribeOptions): Promise<RuleDescription[]> {
const workspaceLiaison: WorkspaceLiaison = this.getWorkspaceLiaison(describeOptions.workspace);
this.emitDescribeRulesProgressEvent(33);
const relevantLanguages: LanguageId[] = await workspaceLiaison.getRelevantLanguages();
return relevantLanguages.map(createRuleForLanguage);
const ruleDescriptions: RuleDescription[] = relevantLanguages.map(createRuleForLanguage);
this.emitDescribeRulesProgressEvent(100);
return ruleDescriptions;
}

async runRules(ruleNames: string[], runOptions: RunOptions): Promise<EngineRunResults> {
const workspaceLiaison: WorkspaceLiaison = this.getWorkspaceLiaison(runOptions.workspace);
const relevantLanguageToFilesMap: Map<LanguageId, string[]> = await workspaceLiaison.getRelevantLanguageToFilesMap();
this.emitRunRulesProgressEvent(2);

const filesToScanPerLanguage: CpdLanguageToFilesMap = {};
for (const languageId of ruleNames.map(getLanguageFromRuleName)) {
Expand All @@ -72,6 +76,7 @@ export class CpdEngine extends Engine {
}

if (Object.keys(filesToScanPerLanguage).length == 0) {
this.emitRunRulesProgressEvent(100);
return { violations: [] };
}

Expand All @@ -80,8 +85,10 @@ export class CpdEngine extends Engine {
minimumTokens: this.minimumTokens,
skipDuplicateFiles: this.skipDuplicateFiles
}
this.emitRunRulesProgressEvent(5);

const cpdRunResults: CpdRunResults = await this.cpdWrapperInvoker.invokeRunCommand(inputData);
const cpdRunResults: CpdRunResults = await this.cpdWrapperInvoker.invokeRunCommand(inputData,
(innerPerc: number) => this.emitRunRulesProgressEvent(5 + 93*(innerPerc/100))); // 5 to 98%

const violations: Violation[] = [];
for (const cpdLanguage in cpdRunResults) {
Expand All @@ -97,6 +104,7 @@ export class CpdEngine extends Engine {
}
}

this.emitRunRulesProgressEvent(100);
return {
violations: violations
};
Expand Down
17 changes: 15 additions & 2 deletions packages/code-analyzer-pmd-engine/src/cpd-wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {getMessage} from "./messages";
const CPD_WRAPPER_JAVA_CLASS: string = "com.salesforce.sfca.cpdwrapper.CpdWrapper";
const CPD_WRAPPER_LIB_FOLDER: string = path.resolve(__dirname, '..', 'dist', 'pmd-cpd-wrappers', 'lib');

const STDOUT_PROGRESS_MARKER = '[Progress]';

export type CpdRunInputData = {
filesToScanPerLanguage: CpdLanguageToFilesMap,
minimumTokens: number,
Expand Down Expand Up @@ -52,11 +54,13 @@ export class CpdWrapperInvoker {
this.emitLogEvent = emitLogEvent;
}

async invokeRunCommand(inputData: CpdRunInputData): Promise<CpdRunResults> {
async invokeRunCommand(inputData: CpdRunInputData, emitProgress: (percComplete: number) => void): Promise<CpdRunResults> {
const tempDir: string = await this.getTemporaryWorkingDir();
emitProgress(5);

const inputFile: string = path.join(tempDir, 'cpdRunInput.json');
await fs.promises.writeFile(inputFile, JSON.stringify(inputData), 'utf-8');
emitProgress(10);

const outputFile: string = path.join(tempDir, 'cpdRunOutput.json');

Expand All @@ -66,12 +70,21 @@ export class CpdWrapperInvoker {
];
this.emitLogEvent(LogLevel.Fine, `Calling JAVA command with class path containing ${JSON.stringify(javaClassPaths)} and arguments: ${JSON.stringify(javaCmdArgs)}`);
await this.javaCommandExecutor.exec(javaCmdArgs, javaClassPaths, (stdOutMsg: string) => {
if (stdOutMsg.startsWith(STDOUT_PROGRESS_MARKER)) {
const cpdWrapperProgress: number = parseFloat(stdOutMsg.slice(STDOUT_PROGRESS_MARKER.length));
emitProgress(10 + 80*(cpdWrapperProgress/100)); // 10 to 90%
}
this.emitLogEvent(LogLevel.Fine, `[JAVA StdOut]: ${stdOutMsg}`);
});

try {
const resultsFileContents: string = await fs.promises.readFile(outputFile, 'utf-8');
return JSON.parse(resultsFileContents);
emitProgress(95);

const cpdResults:CpdRunResults = JSON.parse(resultsFileContents);
emitProgress(100);
return cpdResults;

} catch (err) /* istanbul ignore next */ {
const errMsg: string = err instanceof Error ? err.message : String(err);
throw new Error(getMessage('ErrorParsingOutputFile', outputFile, errMsg), {cause: err});
Expand Down
26 changes: 25 additions & 1 deletion packages/code-analyzer-pmd-engine/test/cpd-engine.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
import {changeWorkingDirectoryToPackageRoot} from "./test-helpers";
import {EngineRunResults, RuleDescription, Violation, Workspace} from "@salesforce/code-analyzer-engine-api";
import {
DescribeRulesProgressEvent,
EngineRunResults,
EventType,
RuleDescription,
RunRulesProgressEvent,
Violation,
Workspace
} from "@salesforce/code-analyzer-engine-api";
import {CpdEngine} from "../src/cpd-engine";
import fs from "node:fs";
import path from "node:path";
Expand All @@ -20,9 +28,17 @@ describe('Tests for the describeRules method of PmdEngine', () => {
// TODO: BEFORE GA, we need to eventually decide on what the default languages should be. For now we just return all.

const engine: CpdEngine = new CpdEngine();
const progressEvents: DescribeRulesProgressEvent[] = [];
engine.onEvent(EventType.DescribeRulesProgressEvent, (e: DescribeRulesProgressEvent) => progressEvents.push(e));


const ruleDescriptions: RuleDescription[] = await engine.describeRules({});

await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_allLanguages.goldfile.json');

// Also check that we have all the correct progress events
expect(progressEvents.map(e => e.percentComplete)).toEqual([33, 100]);

});

it('When using defaults with workspace that only contains apex code, then only apex rule is returned', async () => {
Expand Down Expand Up @@ -78,8 +94,12 @@ describe('Tests for the runRules method of CpdEngine', () => {

it('When using defaults and workspace contains relevant files containing duplicate blocks, then return violations', async () => {
const engine: CpdEngine = new CpdEngine();
const progressEvents: RunRulesProgressEvent[] = [];
engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));

const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'sampleCpdWorkspace')]);
const ruleNames: string[] = ['DetectCopyPasteForApex', 'DetectCopyPasteForHtml', 'DetectCopyPasteForJavascript'];

const results: EngineRunResults = await engine.runRules(ruleNames, {workspace: workspace});

const expViolation1: Violation = {
Expand Down Expand Up @@ -159,5 +179,9 @@ describe('Tests for the runRules method of CpdEngine', () => {
expect(results.violations).toContainEqual(expViolation1);
expect(results.violations).toContainEqual(expViolation2);
expect(results.violations).toContainEqual(expViolation3);

// Also check that we have all the correct progress events
expect(progressEvents.map(e => e.percentComplete)).toEqual([2, 5, 9.65, 14.3, 17.4, 20.5,
26.7, 32.9, 39.1, 42.2, 45.3, 51.5, 57.7, 63.9, 67, 70.1, 76.3, 82.5, 88.7, 93.35, 98, 100]);
});
});

0 comments on commit b5122c3

Please sign in to comment.