diff --git a/packages/code-analyzer-pmd-engine/pmd-wrapper/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRuleRunner.java b/packages/code-analyzer-pmd-engine/pmd-wrapper/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRuleRunner.java index 41cdd661..3b9a1a40 100644 --- a/packages/code-analyzer-pmd-engine/pmd-wrapper/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRuleRunner.java +++ b/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() {} +} \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/pmd-wrapper/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java b/packages/code-analyzer-pmd-engine/pmd-wrapper/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java index 7f5be1c1..410f0c56 100644 --- a/packages/code-analyzer-pmd-engine/pmd-wrapper/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java +++ b/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"))); } diff --git a/packages/code-analyzer-pmd-engine/src/pmd-engine.ts b/packages/code-analyzer-pmd-engine/src/pmd-engine.ts index c8b50321..88b515f4 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-engine.ts +++ b/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,22 +41,35 @@ export class PmdEngine extends Engine { async describeRules(describeOptions: DescribeOptions): Promise { 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 { 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 = 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) { @@ -64,22 +77,22 @@ export class PmdEngine extends Engine { 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 { + private async getPmdRuleInfoList(workspaceLiaison: PmdWorkspaceLiaison, emitProgress: (percComplete: number) => void): Promise { 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)!; diff --git a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts index 839be2d5..8d2edd2d 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts +++ b/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 { + async invokeDescribeCommand(languages: PmdLanguage[], emitProgress: (percComplete: number) => void): Promise { 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 { + async invokeRunCommand(pmdRuleInfoList: PmdRuleInfo[], filesToScan: string[], emitProgress: (percComplete: number) => void): Promise { 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}); diff --git a/packages/code-analyzer-pmd-engine/src/utils.ts b/packages/code-analyzer-pmd-engine/src/utils.ts index ed5d8847..7066614e 100644 --- a/packages/code-analyzer-pmd-engine/src/utils.ts +++ b/packages/code-analyzer-pmd-engine/src/utils.ts @@ -15,17 +15,17 @@ export async function createTempDir(parentTempDir?: string) : Promise { 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 { + async exec(javaCmdArgs: string[], javaClassPaths: string[] = [], processStdOut: ProcessStdOutFcn = NO_OP): Promise { return new Promise((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) => { diff --git a/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts b/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts index 85016183..250a8436 100644 --- a/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts +++ b/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 () => {