Skip to content

Commit 7b9f8d4

Browse files
NEW(cpd): @W-16866831@: Add in progress events to cpd engine
1 parent 44f2ffd commit 7b9f8d4

File tree

5 files changed

+164
-16
lines changed

5 files changed

+164
-16
lines changed

packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java

Lines changed: 93 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.salesforce.sfca.cpdwrapper;
22

33
import net.sourceforge.pmd.cpd.CPDConfiguration;
4+
import net.sourceforge.pmd.cpd.CPDListener;
45
import net.sourceforge.pmd.cpd.CpdAnalysis;
56
import net.sourceforge.pmd.cpd.Mark;
67
import net.sourceforge.pmd.cpd.Match;
@@ -24,25 +25,27 @@
2425
* Class to help us invoke CPD - once for each language that should be processed
2526
*/
2627
class CpdRunner {
28+
private final ProgressReporter progressReporter = new ProgressReporter();
29+
2730
public Map<String, CpdLanguageRunResults> run(CpdRunInputData runInputData) throws IOException {
2831
validateRunInputData(runInputData);
2932

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

32-
for (Map.Entry<String, List<String>> entry : runInputData.filesToScanPerLanguage.entrySet()) {
33-
String language = entry.getKey();
34-
List<String> filesToScan = entry.getValue();
35-
if (filesToScan.isEmpty()) {
36-
continue;
37-
}
38+
progressReporter.initialize(languagesToProcess);
39+
40+
Map<String, CpdLanguageRunResults> results = new HashMap<>();
41+
for (String language : languagesToProcess) {
42+
List<String> filesToScan = runInputData.filesToScanPerLanguage.get(language);
3843
List<Path> pathsToScan = filesToScan.stream().map(Paths::get).collect(Collectors.toList());
3944
CpdLanguageRunResults languageRunResults = runLanguage(language, pathsToScan, runInputData.minimumTokens, runInputData.skipDuplicateFiles);
40-
4145
if (!languageRunResults.matches.isEmpty() || !languageRunResults.processingErrors.isEmpty()) {
4246
results.put(language, languageRunResults);
4347
}
4448
}
45-
4649
return results;
4750
}
4851

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

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

71+
// Note that we could use cpd.files().getCollectedFiles().size() to get the true totalNumFiles but
72+
// unfortunately getCollectedFiles doesn't cache and does a sort operation which is expensive.
73+
// So instead we use pathsToScan.size() since we send in the list of files instead of folders and so
74+
// these numbers should be the same.
75+
int totalNumFiles = pathsToScan.size();
76+
cpd.setCpdListener(new CpdLanguageRunListener(progressReporter, language, totalNumFiles));
77+
78+
cpd.performAnalysis(report -> {
6979
for (Report.ProcessingError reportProcessingError : report.getProcessingErrors()) {
7080
CpdLanguageRunResults.ProcessingError processingErr = new CpdLanguageRunResults.ProcessingError();
7181
processingErr.file = reportProcessingError.getFileId().getAbsolutePath();
@@ -134,4 +144,77 @@ public boolean isLoggable(Level level) {
134144
public int numErrors() {
135145
return 0;
136146
}
147+
}
148+
149+
// This class helps us track the overall progress of all language runs
150+
class ProgressReporter {
151+
private Map<String, Float> progressPerLanguage = new HashMap<>();
152+
private float lastReportedProgress = 0.0f;
153+
154+
public void initialize(List<String> languages) {
155+
progressPerLanguage = new HashMap<>();
156+
languages.forEach(l -> this.updateProgressForLanguage(l, 0.0f));
157+
}
158+
159+
public void updateProgressForLanguage(String language, float percComplete) {
160+
progressPerLanguage.put(language, percComplete);
161+
}
162+
163+
public void reportOverallProgress() {
164+
float currentProgress = this.calculateOverallPercentage();
165+
// The progress goes very fast, so we make sure to only report progress if there has been a significant enough increase (at least 1%)
166+
if (currentProgress >= lastReportedProgress + 1) {
167+
System.out.println("[Progress]" + currentProgress);
168+
lastReportedProgress = currentProgress;
169+
}
170+
}
171+
172+
private float calculateOverallPercentage() {
173+
float sum = 0.0f;
174+
for (float progress : progressPerLanguage.values()) {
175+
sum += progress;
176+
}
177+
return sum / progressPerLanguage.size();
178+
}
179+
}
180+
181+
// This class is a specific listener for a run of cpd for a single language.
182+
class CpdLanguageRunListener implements CPDListener {
183+
private final ProgressReporter progressReporter;
184+
private final String language;
185+
private final int totalNumFiles;
186+
private int numFilesAdded = 0;
187+
private int currentPhase = CPDListener.INIT;
188+
189+
public CpdLanguageRunListener(ProgressReporter progressReporter, String language, int totalNumFiles) {
190+
this.progressReporter = progressReporter;
191+
this.language = language;
192+
this.totalNumFiles = totalNumFiles;
193+
}
194+
195+
@Override
196+
public void addedFile(int i) {
197+
// All files are added while we still are on phase 0 - INIT, i.e. before the phase is updated to phase 1 - HASH.
198+
this.numFilesAdded += i;
199+
updateAndReportCompletePercentage();
200+
}
201+
202+
@Override
203+
public void phaseUpdate(int i) {
204+
this.currentPhase = i;
205+
updateAndReportCompletePercentage();
206+
}
207+
208+
private void updateAndReportCompletePercentage() {
209+
this.progressReporter.updateProgressForLanguage(this.language, calculateCompletePercentage());
210+
this.progressReporter.reportOverallProgress();
211+
}
212+
213+
private float calculateCompletePercentage() {
214+
if (this.currentPhase == CPDListener.INIT) {
215+
// Using Math.min just in case the totalNumFiles is inaccurate - although it shouldn't be.
216+
return 25*(Math.min((float) this.numFilesAdded / this.totalNumFiles, 1.0f));
217+
}
218+
return 100 * ((float) this.currentPhase / CPDListener.DONE);
219+
}
137220
}

packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/cpdwrapper/CpdWrapperTest.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,19 @@ void whenCallingRunWithValidFilesThatHaveDuplicates_thenJsonOutputShouldContainR
318318
// Also check that stdOut contains runtime information
319319
assertThat(stdOut, containsString("ARGUMENTS:"));
320320
assertThat(stdOut, containsString("milliseconds"));
321+
322+
// Also check that stdOut contains correct progress information
323+
assertThat(stdOut, allOf(
324+
containsString("[Progress]6.25"),
325+
containsString("[Progress]12.5"),
326+
containsString("[Progress]25.0"),
327+
containsString("[Progress]37.5"),
328+
containsString("[Progress]50.0"),
329+
containsString("[Progress]56.25"),
330+
containsString("[Progress]62.5"),
331+
containsString("[Progress]75.0"),
332+
containsString("[Progress]87.5"),
333+
containsString("[Progress]100.0")));
321334
}
322335

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

367-
callCpdWrapper(args);
380+
String stdOut = callCpdWrapper(args);
368381

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

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

401414
assertThat(resultsJsonString, is(expectedOutput));
415+
416+
assertThat(stdOut, allOf(
417+
containsString("[Progress]12.5"),
418+
containsString("[Progress]25.0"),
419+
containsString("[Progress]50.0"),
420+
containsString("[Progress]75.0"),
421+
containsString("[Progress]100.0")));
402422
}
403423

404424
@Test

packages/code-analyzer-pmd-engine/src/cpd-engine.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,17 @@ export class CpdEngine extends Engine {
5555

5656
async describeRules(describeOptions: DescribeOptions): Promise<RuleDescription[]> {
5757
const workspaceLiaison: WorkspaceLiaison = this.getWorkspaceLiaison(describeOptions.workspace);
58+
this.emitDescribeRulesProgressEvent(33);
5859
const relevantLanguages: LanguageId[] = await workspaceLiaison.getRelevantLanguages();
59-
return relevantLanguages.map(createRuleForLanguage);
60+
const ruleDescriptions: RuleDescription[] = relevantLanguages.map(createRuleForLanguage);
61+
this.emitDescribeRulesProgressEvent(100);
62+
return ruleDescriptions;
6063
}
6164

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

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

7478
if (Object.keys(filesToScanPerLanguage).length == 0) {
79+
this.emitRunRulesProgressEvent(100);
7580
return { violations: [] };
7681
}
7782

@@ -80,8 +85,10 @@ export class CpdEngine extends Engine {
8085
minimumTokens: this.minimumTokens,
8186
skipDuplicateFiles: this.skipDuplicateFiles
8287
}
88+
this.emitRunRulesProgressEvent(5);
8389

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

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

107+
this.emitRunRulesProgressEvent(100);
100108
return {
101109
violations: violations
102110
};

packages/code-analyzer-pmd-engine/src/cpd-wrapper.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import {getMessage} from "./messages";
77
const CPD_WRAPPER_JAVA_CLASS: string = "com.salesforce.sfca.cpdwrapper.CpdWrapper";
88
const CPD_WRAPPER_LIB_FOLDER: string = path.resolve(__dirname, '..', 'dist', 'pmd-cpd-wrappers', 'lib');
99

10+
const STDOUT_PROGRESS_MARKER = '[Progress]';
11+
1012
export type CpdRunInputData = {
1113
filesToScanPerLanguage: CpdLanguageToFilesMap,
1214
minimumTokens: number,
@@ -52,11 +54,13 @@ export class CpdWrapperInvoker {
5254
this.emitLogEvent = emitLogEvent;
5355
}
5456

55-
async invokeRunCommand(inputData: CpdRunInputData): Promise<CpdRunResults> {
57+
async invokeRunCommand(inputData: CpdRunInputData, emitProgress: (percComplete: number) => void): Promise<CpdRunResults> {
5658
const tempDir: string = await this.getTemporaryWorkingDir();
59+
emitProgress(5);
5760

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

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

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

7280
try {
7381
const resultsFileContents: string = await fs.promises.readFile(outputFile, 'utf-8');
74-
return JSON.parse(resultsFileContents);
82+
emitProgress(95);
83+
84+
const cpdResults:CpdRunResults = JSON.parse(resultsFileContents);
85+
emitProgress(100);
86+
return cpdResults;
87+
7588
} catch (err) /* istanbul ignore next */ {
7689
const errMsg: string = err instanceof Error ? err.message : String(err);
7790
throw new Error(getMessage('ErrorParsingOutputFile', outputFile, errMsg), {cause: err});

packages/code-analyzer-pmd-engine/test/cpd-engine.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import {changeWorkingDirectoryToPackageRoot} from "./test-helpers";
2-
import {EngineRunResults, RuleDescription, Violation, Workspace} from "@salesforce/code-analyzer-engine-api";
2+
import {
3+
DescribeRulesProgressEvent,
4+
EngineRunResults,
5+
EventType,
6+
RuleDescription,
7+
RunRulesProgressEvent,
8+
Violation,
9+
Workspace
10+
} from "@salesforce/code-analyzer-engine-api";
311
import {CpdEngine} from "../src/cpd-engine";
412
import fs from "node:fs";
513
import path from "node:path";
@@ -20,9 +28,17 @@ describe('Tests for the describeRules method of PmdEngine', () => {
2028
// TODO: BEFORE GA, we need to eventually decide on what the default languages should be. For now we just return all.
2129

2230
const engine: CpdEngine = new CpdEngine();
31+
const progressEvents: DescribeRulesProgressEvent[] = [];
32+
engine.onEvent(EventType.DescribeRulesProgressEvent, (e: DescribeRulesProgressEvent) => progressEvents.push(e));
33+
34+
2335
const ruleDescriptions: RuleDescription[] = await engine.describeRules({});
2436

2537
await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_allLanguages.goldfile.json');
38+
39+
// Also check that we have all the correct progress events
40+
expect(progressEvents.map(e => e.percentComplete)).toEqual([33, 100]);
41+
2642
});
2743

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

7995
it('When using defaults and workspace contains relevant files containing duplicate blocks, then return violations', async () => {
8096
const engine: CpdEngine = new CpdEngine();
97+
const progressEvents: RunRulesProgressEvent[] = [];
98+
engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));
99+
81100
const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'sampleCpdWorkspace')]);
82101
const ruleNames: string[] = ['DetectCopyPasteForApex', 'DetectCopyPasteForHtml', 'DetectCopyPasteForJavascript'];
102+
83103
const results: EngineRunResults = await engine.runRules(ruleNames, {workspace: workspace});
84104

85105
const expViolation1: Violation = {
@@ -159,5 +179,9 @@ describe('Tests for the runRules method of CpdEngine', () => {
159179
expect(results.violations).toContainEqual(expViolation1);
160180
expect(results.violations).toContainEqual(expViolation2);
161181
expect(results.violations).toContainEqual(expViolation3);
182+
183+
// Also check that we have all the correct progress events
184+
expect(progressEvents.map(e => e.percentComplete)).toEqual([2, 5, 9.65, 14.3, 17.4, 20.5,
185+
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]);
162186
});
163187
});

0 commit comments

Comments
 (0)