NEW(pmd): @W-16113607@: Add in progress and additional log events to …

…PMD (#86)

packages/code-analyzer-pmd-engine/pmd-wrapper/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRuleRunner.java

@@ -2,6 +2,10 @@
 
 import net.sourceforge.pmd.PMDConfiguration;
 import net.sourceforge.pmd.PmdAnalysis;
+import net.sourceforge.pmd.lang.document.TextFile;
+import net.sourceforge.pmd.reporting.FileAnalysisListener;
+import net.sourceforge.pmd.reporting.GlobalAnalysisListener;
+import net.sourceforge.pmd.reporting.ListenerInitializer;
 
 import java.nio.file.Paths;
 
@@ -14,7 +18,35 @@ void runRules(String ruleSetInputFile, String filesToScanInputFile, String resul
         config.setReportFile(Paths.get(resultsOutputFile));
 
         try (PmdAnalysis pmd = PmdAnalysis.create(config)) {
+            pmd.addListener(new PmdRunProgressListener());
             pmd.performAnalysis();
         }
     }
 }
+
+class PmdRunProgressListener implements GlobalAnalysisListener {
+    private int totalNumFiles = 0;
+    private int fileCount = 0;
+
+    @Override
+    public ListenerInitializer initializer() {
+        return new ListenerInitializer() {
+            @Override
+            public void setNumberOfFilesToAnalyze(int totalFiles) {
+                totalNumFiles = totalFiles;
+            }
+        };
+    }
+
+    @Override
+    public synchronized FileAnalysisListener startFileAnalysis(TextFile textFile) {
+        // Note that this method must be synchronized so that multiple threads cannot mess
+        // up the order of progress with race conditions.
+        fileCount++;
+        System.out.println("[Progress]" + fileCount + "::" + totalNumFiles);
+        return FileAnalysisListener.noop();
+    }
+
+    @Override
+    public void close() {}
+}

packages/code-analyzer-pmd-engine/pmd-wrapper/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java

@@ -15,7 +15,6 @@
 import java.io.FileNotFoundException;
 import java.io.PrintStream;
 import java.lang.reflect.Type;
-import java.nio.file.FileSystemException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -146,8 +145,12 @@ void whenCallingMainWithRunAndValidArgs_thenItWritesValidJsonToFile(@TempDir Pat
                 containsString("\"rule\": \"VfUnescapeEl\"")
         ));
 
-        // Assert stdOut contains time arguments and time information (which help us with debugging)
-        assertThat(stdOut, allOf(containsString("ARGUMENTS"), containsString("milliseconds")));
+        // Assert stdOut contains arguments, progress information, and duration information (which help us with debugging)
+        assertThat(stdOut, allOf(
+                containsString("ARGUMENTS"),
+                containsString("[Progress]1::2"),
+                containsString("[Progress]2::2"),
+                containsString("milliseconds")));
     }
 
 

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

@@ -29,9 +29,9 @@ export class PmdEngine extends Engine {
     constructor() {
         super();
         const javaCmd: string = 'java'; // TODO: Will be configurable soon
-        const javaCommandExecutor: JavaCommandExecutor = new JavaCommandExecutor(javaCmd, stdOutMsg =>
-            this.emitLogEvent(LogLevel.Fine, `[JAVA StdOut]: ${stdOutMsg}`));
-        this.pmdWrapperInvoker = new PmdWrapperInvoker(javaCommandExecutor);
+        const javaCommandExecutor: JavaCommandExecutor = new JavaCommandExecutor(javaCmd);
+        this.pmdWrapperInvoker = new PmdWrapperInvoker(javaCommandExecutor,
+            (logLevel: LogLevel, message: string) => this.emitLogEvent(logLevel, message));
         this.availableLanguages = [PmdLanguage.APEX, PmdLanguage.VISUALFORCE]; // TODO: Will be configurable soon
     }
 
@@ -41,45 +41,58 @@ export class PmdEngine extends Engine {
 
     async describeRules(describeOptions: DescribeOptions): Promise<RuleDescription[]> {
         const workspaceLiaison: PmdWorkspaceLiaison = this.getPmdWorkspaceLiaison(describeOptions.workspace);
-        const ruleInfoList: PmdRuleInfo[] = await this.getPmdRuleInfoList(workspaceLiaison);
+        this.emitDescribeRulesProgressEvent(5);
+
+        const ruleInfoList: PmdRuleInfo[] = await this.getPmdRuleInfoList(workspaceLiaison,
+            (innerPerc: number) => this.emitDescribeRulesProgressEvent(5 + 90*(innerPerc/100))); // 5 to 95%
+
         const ruleDescriptions: RuleDescription[] = ruleInfoList.map(toRuleDescription);
-        return ruleDescriptions.sort((rd1, rd2) => rd1.name.localeCompare(rd2.name));
+        ruleDescriptions.sort((rd1, rd2) => rd1.name.localeCompare(rd2.name));
+        this.emitDescribeRulesProgressEvent(100);
+        return ruleDescriptions;
     }
 
     async runRules(ruleNames: string[], runOptions: RunOptions): Promise<EngineRunResults> {
         const workspaceLiaison: PmdWorkspaceLiaison = this.getPmdWorkspaceLiaison(runOptions.workspace);
+        this.emitRunRulesProgressEvent(2);
+
         const filesToScan: string[] = await workspaceLiaison.getRelevantFiles();
         if (ruleNames.length === 0 || filesToScan.length === 0) {
+            this.emitRunRulesProgressEvent(100);
             return {violations: []};
         }
-        const ruleInfoList: PmdRuleInfo[] = await this.getPmdRuleInfoList(workspaceLiaison);
+        this.emitRunRulesProgressEvent(4);
+
+        const ruleInfoList: PmdRuleInfo[] = await this.getPmdRuleInfoList(workspaceLiaison,
+            (innerPerc: number) => this.emitRunRulesProgressEvent(4 + 6*(innerPerc/100))); // 4 to 10%
+
         const selectedRuleNames: Set<string> = new Set(ruleNames);
         const selectedRuleInfoList: PmdRuleInfo[] = ruleInfoList.filter(ruleInfo => selectedRuleNames.has(ruleInfo.name));
-
-        const pmdResults: PmdResults = await this.pmdWrapperInvoker.invokeRunCommand(selectedRuleInfoList, filesToScan);
+        const pmdResults: PmdResults = await this.pmdWrapperInvoker.invokeRunCommand(selectedRuleInfoList, filesToScan,
+            (innerPerc: number) => this.emitRunRulesProgressEvent(10 + 88*(innerPerc/100))); // 10 to 98%
 
         const violations: Violation[] = [];
         for (const pmdFileResult of pmdResults.files) {
             for (const pmdViolation of pmdFileResult.violations) {
                 violations.push(toViolation(pmdViolation, pmdFileResult.filename));
             }
         }
-
         for (const pmdProcessingError of pmdResults.processingErrors) {
             this.emitLogEvent(LogLevel.Error, getMessage('PmdProcessingErrorForFile', pmdProcessingError.filename,
                 indent(pmdProcessingError.message)));
         }
 
+        this.emitRunRulesProgressEvent(100);
         return {
             violations: violations
         };
     }
 
-    private async getPmdRuleInfoList(workspaceLiaison: PmdWorkspaceLiaison): Promise<PmdRuleInfo[]> {
+    private async getPmdRuleInfoList(workspaceLiaison: PmdWorkspaceLiaison, emitProgress: (percComplete: number) => void): Promise<PmdRuleInfo[]> {
         const cacheKey: string = getCacheKey(workspaceLiaison.getWorkspace());
         if (!this.pmdRuleInfoListCache.has(cacheKey)) {
             const relevantLanguages: PmdLanguage[] = await workspaceLiaison.getRelevantLanguages();
-            const ruleInfoList: PmdRuleInfo[] = await this.pmdWrapperInvoker.invokeDescribeCommand(relevantLanguages);
+            const ruleInfoList: PmdRuleInfo[] = await this.pmdWrapperInvoker.invokeDescribeCommand(relevantLanguages, emitProgress);
             this.pmdRuleInfoListCache.set(cacheKey, ruleInfoList);
         }
         return this.pmdRuleInfoListCache.get(cacheKey)!;

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

@@ -3,6 +3,7 @@ import {PmdLanguage} from "./constants";
 import path from "node:path";
 import fs from "node:fs";
 import {getMessage} from "./messages";
+import {LogLevel} from "@salesforce/code-analyzer-engine-api";
 
 const PMD_WRAPPER_JAVA_CLASS: string = "com.salesforce.sfca.pmdwrapper.PmdWrapper";
 const PMD_WRAPPER_LIB_FOLDER: string = path.resolve(__dirname, '..', 'dist', 'pmd-wrapper', 'lib');
@@ -43,51 +44,76 @@ export type PmdProcessingError = {
 export class PmdWrapperInvoker {
     private readonly javaCommandExecutor: JavaCommandExecutor;
     private temporaryWorkingDir?: string;
+    private emitLogEvent: (logLevel: LogLevel, message: string) => void;
 
-    constructor(javaCommandExecutor: JavaCommandExecutor) {
-        // const javaCommandExecutor: JavaCommandExecutor = new JavaCommandExecutor(); // TODO: Once java_command is configurable, then pass it in
+    constructor(javaCommandExecutor: JavaCommandExecutor, emitLogEvent: (logLevel: LogLevel, message: string) => void) {
         this.javaCommandExecutor = javaCommandExecutor;
+        this.emitLogEvent = emitLogEvent;
     }
 
-    async invokeDescribeCommand(languages: PmdLanguage[]): Promise<PmdRuleInfo[]> {
+    async invokeDescribeCommand(languages: PmdLanguage[], emitProgress: (percComplete: number) => void): Promise<PmdRuleInfo[]> {
         const pmdRulesOutputFile: string = path.join(await this.getTemporaryWorkingDir(), 'ruleInfo.json');
+        emitProgress(10);
 
         const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'describe', pmdRulesOutputFile, languages.join(',')];
         const javaClassPaths: string[] = [
             path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
         ];
-        await this.javaCommandExecutor.exec(javaCmdArgs, javaClassPaths);
+        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) => this.emitLogEvent(LogLevel.Fine, `[JAVA StdOut]: ${stdOutMsg}`));
+        emitProgress(80);
 
         try {
             const pmdRulesFileContents: string = await fs.promises.readFile(pmdRulesOutputFile, 'utf-8');
-            return JSON.parse(pmdRulesFileContents);
+            emitProgress(90);
+            const pmdRuleInfoList: PmdRuleInfo[] = JSON.parse(pmdRulesFileContents);
+            emitProgress(100);
+            return pmdRuleInfoList;
         } catch (err) /* istanbul ignore next */ {
             const errMsg: string = err instanceof Error ? err.message : String(err);
             throw new Error(getMessage('ErrorParsingPmdWrapperOutputFile', pmdRulesOutputFile, errMsg), {cause: err});
         }
     }
 
-    async invokeRunCommand(pmdRuleInfoList: PmdRuleInfo[], filesToScan: string[]): Promise<PmdResults> {
+    async invokeRunCommand(pmdRuleInfoList: PmdRuleInfo[], filesToScan: string[], emitProgress: (percComplete: number) => void): Promise<PmdResults> {
         const tempDir: string = await this.getTemporaryWorkingDir();
+        emitProgress(2);
 
         const ruleSetFileContents: string = createRuleSetFileContentsFor(pmdRuleInfoList);
         const ruleSetInputFile: string = path.join(tempDir, 'ruleSetInputFile.xml');
         await fs.promises.writeFile(ruleSetInputFile, ruleSetFileContents, 'utf-8');
+        emitProgress(6);
 
         const filesToScanInputFile: string = path.join(tempDir, 'filesToScanInputFile.txt');
         await fs.promises.writeFile(filesToScanInputFile, filesToScan.join('\n'), 'utf-8');
+        emitProgress(10);
 
         const resultsOutputFile: string = path.join(tempDir, 'resultsFile.json');
-
         const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'run', ruleSetInputFile, filesToScanInputFile, resultsOutputFile];
         const javaClassPaths: string[] = [
             path.join(PMD_WRAPPER_LIB_FOLDER, '*'),
         ];
-        await this.javaCommandExecutor.exec(javaCmdArgs, javaClassPaths);
+        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('[Progress]')) {
+                const parts: string[] = stdOutMsg.slice(10).split('::');
+                const numFilesProcessed: number = parseInt(parts[0]);
+                const totalNumFiles: number = parseInt(parts[1]);
+                emitProgress(10 + (80 * numFilesProcessed / totalNumFiles));
+            } else {
+                this.emitLogEvent(LogLevel.Fine, `[JAVA StdOut]: ${stdOutMsg}`);
+            }
+        });
 
         try {
             const resultsFileContents: string = await fs.promises.readFile(resultsOutputFile, 'utf-8');
-            return JSON.parse(resultsFileContents);
+            emitProgress(95);
+
+            const pmdResults:PmdResults = JSON.parse(resultsFileContents);
+            emitProgress(100);
+            return pmdResults;
+
         } catch (err) /* istanbul ignore next */ {
             const errMsg: string = err instanceof Error ? err.message : String(err);
             throw new Error(getMessage('ErrorParsingPmdWrapperOutputFile', resultsOutputFile, errMsg), {cause: err});

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

@@ -15,17 +15,17 @@ export async function createTempDir(parentTempDir?: string) : Promise<string> {
     return tmpDirAsync({dir: parentTempDir, keep: false, unsafeCleanup: true});
 }
 
+type ProcessStdOutFcn = (stdOutMsg: string) => void;
+const NO_OP = () => {};
 
 export class JavaCommandExecutor {
     private readonly javaCommand: string;
-    private readonly stdoutCallback: (stdOutMsg: string) => void;
 
-    constructor(javaCommand: string = 'java', stdoutCallback: (stdOutMsg: string) => void = (_m: string) => {}) {
+    constructor(javaCommand: string = 'java') {
         this.javaCommand = javaCommand;
-        this.stdoutCallback = stdoutCallback;
     }
 
-    async exec(javaCmdArgs: string[], javaClassPaths: string[] = []): Promise<void> {
+    async exec(javaCmdArgs: string[], javaClassPaths: string[] = [], processStdOut: ProcessStdOutFcn = NO_OP): Promise<void> {
         return new Promise<void>((resolve, reject) => {
             const stderrMessages: string[] = [];
             const allJavaArgs: string[] = javaClassPaths.length == 0 ? javaCmdArgs :
@@ -35,7 +35,7 @@ export class JavaCommandExecutor {
             javaProcess.stdout.on('data', (data: Buffer) => {
                 const msg: string = data.toString().trim();
                 if(msg.length > 0) { // Not sure why stdout spits out empty lines, but we ignore them nonetheless
-                    this.stdoutCallback(msg);
+                    msg.split("\n").map(line => processStdOut(line));
                 }
             });
             javaProcess.stderr.on('data', (data: Buffer) => {

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

@@ -1,10 +1,12 @@
 import {changeWorkingDirectoryToPackageRoot} from "./test-helpers";
 import {
+    DescribeRulesProgressEvent,
     EngineRunResults,
     EventType,
     LogEvent,
     LogLevel,
     RuleDescription,
+    RunRulesProgressEvent,
     Violation,
     Workspace
 } from "@salesforce/code-analyzer-engine-api";
@@ -30,15 +32,21 @@ describe('Tests for the describeRules method of PmdEngine', () => {
         const engine: PmdEngine = new PmdEngine();
         const logEvents: LogEvent[] = [];
         engine.onEvent(EventType.LogEvent, (e: LogEvent) => logEvents.push(e));
+        const progressEvents: DescribeRulesProgressEvent[] = [];
+        engine.onEvent(EventType.DescribeRulesProgressEvent, (e: DescribeRulesProgressEvent) => progressEvents.push(e));
 
         const ruleDescriptions: RuleDescription[] = await engine.describeRules({});
         await expectRulesToMatchGoldFile(ruleDescriptions, 'rules_apexAndVisualforce.goldfile.json');
 
-        // Also sanity check that we have fine logs with the argument list and the duration in milliseconds
+        // Also check that we have fine logs with the argument list and the duration in milliseconds
         const fineLogEvents: LogEvent[] = logEvents.filter(e => e.logLevel === LogLevel.Fine);
-        expect(fineLogEvents.length).toBeGreaterThanOrEqual(2);
-        expect(fineLogEvents[0].message).toContain('ARGUMENTS');
-        expect(fineLogEvents[1].message).toContain('milliseconds');
+        expect(fineLogEvents.length).toBeGreaterThanOrEqual(3);
+        expect(fineLogEvents[0].message).toContain('Calling JAVA command with');
+        expect(fineLogEvents[1].message).toContain('ARGUMENTS');
+        expect(fineLogEvents[2].message).toContain('milliseconds');
+
+        // Also check that we have all the correct progress events
+        expect(progressEvents.map(e => e.percentComplete)).toEqual([5, 14, 77, 86, 95, 100]);
 
         // Also sanity check that calling describeRules a second time gives same results (from cache):
         expect(await engine.describeRules({})).toEqual(ruleDescriptions);
@@ -136,19 +144,30 @@ describe('Tests for the runRules method of PmdEngine', () => {
 
     it('When workspace contains zero relevant files, then return zero violations', async () => {
         const engine: PmdEngine = new PmdEngine();
+        const progressEvents: RunRulesProgressEvent[] = [];
+        engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));
+
         const workspace: Workspace = new Workspace([path.join(testDataFolder, 'sampleWorkspace', 'dummy.xml')]);
         const ruleNames: string[] = ['OperationWithLimitsInLoop', 'VfUnescapeEl'];
         const results: EngineRunResults = await engine.runRules(ruleNames, {workspace: workspace});
+
         expect(results.violations).toHaveLength(0);
+
+        // Also check that we have all the correct progress events
+        expect(progressEvents.map(e => e.percentComplete)).toEqual([2, 100]);
     });
 
     it('When workspace contains relevant files containing violation, then return violations', async () => {
         const engine: PmdEngine = new PmdEngine();
         const logEvents: LogEvent[] = [];
         engine.onEvent(EventType.LogEvent, (event: LogEvent) => logEvents.push(event));
+        const progressEvents: RunRulesProgressEvent[] = [];
+        engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));
+
         const workspace: Workspace = new Workspace([path.join(testDataFolder, 'sampleWorkspace')]);
         const ruleNames: string[] = ['OperationWithLimitsInLoop', 'VfUnescapeEl'];
         const results: EngineRunResults = await engine.runRules(ruleNames, {workspace: workspace});
+
         expect(results.violations).toHaveLength(3);
         expect(results.violations).toContainEqual(expectedOperationWithLimitsInLoopViolation);
         expect(results.violations).toContainEqual(expectedVfUnescapeElViolation1);
@@ -157,7 +176,11 @@ describe('Tests for the runRules method of PmdEngine', () => {
         // Also check that we throw error event when PMD can't parse a file
         const errorLogEvents: LogEvent[] = logEvents.filter(event => event.logLevel === LogLevel.Error);
         expect(errorLogEvents.length).toBeGreaterThan(0);
-        expect(errorLogEvents[0].message).toMatch(/PMD issued a processing error for file.*dummy/)
+        expect(errorLogEvents[0].message).toMatch(/PMD issued a processing error for file.*dummy/);
+
+        // Also check that we have all the correct progress events
+        expect(progressEvents.map(e => e.percentComplete)).toEqual(
+            [2, 4, 4.6, 8.8, 9.4, 10, 11.76, 15.28, 18.8, 32.88, 46.96, 61.04, 75.12, 89.2, 93.6, 98, 100]);
     });
 
     it('When a single rule is selected, then return only violations for that rule', async () => {