From 70ba652045d1fc06a1b35e3b17f046287852ae16 Mon Sep 17 00:00:00 2001 From: Stephen Carter Date: Mon, 9 Dec 2024 18:22:27 -0500 Subject: [PATCH] CHANGE(pmd): @W-17310830@: Update pmd-wrapper to run rules by language forcefully --- .../sfca/cpdwrapper/CpdRunInputData.java | 8 +- .../salesforce/sfca/cpdwrapper/CpdRunner.java | 42 +--- .../sfca/pmdwrapper/PmdRunInputData.java | 13 ++ .../salesforce/sfca/pmdwrapper/PmdRunner.java | 80 ++++++- .../sfca/pmdwrapper/PmdWrapper.java | 24 +- .../sfca/shared/ProgressReporter.java | 37 ++++ .../sfca/cpdwrapper/CpdWrapperTest.java | 2 +- .../sfca/pmdwrapper/PmdWrapperTest.java | 206 +++++++++++++++--- .../src/pmd-engine.ts | 39 ++-- .../src/pmd-wrapper.ts | 27 ++- .../code-analyzer-pmd-engine/src/utils.ts | 4 - .../test/pmd-engine.test.ts | 2 +- 12 files changed, 370 insertions(+), 114 deletions(-) create mode 100644 packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunInputData.java create mode 100644 packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/shared/ProgressReporter.java diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunInputData.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunInputData.java index a9c2fa79..31e39480 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunInputData.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunInputData.java @@ -9,9 +9,9 @@ class CpdRunInputData { public Map runDataPerLanguage; public boolean skipDuplicateFiles; -} -class LanguageSpecificRunData { - public List filesToScan; - public int minimumTokens; + static class LanguageSpecificRunData { + public List filesToScan; + public int minimumTokens; + } } \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java index 4cca507d..9b429b79 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/cpdwrapper/CpdRunner.java @@ -2,13 +2,13 @@ import com.salesforce.sfca.shared.CodeLocation; import com.salesforce.sfca.shared.ProcessingError; +import com.salesforce.sfca.shared.ProgressReporter; 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; import net.sourceforge.pmd.lang.Language; -import net.sourceforge.pmd.lang.document.FileLocation; import net.sourceforge.pmd.reporting.Report; import net.sourceforge.pmd.util.log.PmdReporter; import org.slf4j.event.Level; @@ -37,7 +37,7 @@ public Map run(CpdRunInputData runInputData) thro Map results = new HashMap<>(); for (String language : languagesToProcess) { - LanguageSpecificRunData languageSpecificRunData = runInputData.runDataPerLanguage.get(language); + CpdRunInputData.LanguageSpecificRunData languageSpecificRunData = runInputData.runDataPerLanguage.get(language); List pathsToScan = languageSpecificRunData.filesToScan.stream().map(Paths::get).collect(Collectors.toList()); CpdLanguageRunResults languageRunResults = runLanguage( language, pathsToScan, languageSpecificRunData.minimumTokens, runInputData.skipDuplicateFiles); @@ -116,14 +116,14 @@ private void validateRunInputData(CpdRunInputData runInputData) { throw new RuntimeException("The \"runDataPerLanguage\" field was not set."); } - Set> entries = runInputData.runDataPerLanguage.entrySet(); + Set> entries = runInputData.runDataPerLanguage.entrySet(); if (entries.isEmpty()) { throw new RuntimeException("The \"runDataPerLanguage\" field didn't have any languages listed."); } - for (Map.Entry entry: entries) { + for (Map.Entry entry: entries) { String language = entry.getKey(); - LanguageSpecificRunData languageSpecificRunData = entry.getValue(); + CpdRunInputData.LanguageSpecificRunData languageSpecificRunData = entry.getValue(); if (languageSpecificRunData.filesToScan == null || languageSpecificRunData.filesToScan.isEmpty()) { throw new RuntimeException(("The \"filesToScan\" field was missing or empty for language: " + language)); @@ -168,38 +168,6 @@ public int numErrors() { } } -// This class helps us track the overall progress of all language runs -class ProgressReporter { - private Map progressPerLanguage = new HashMap<>(); - private float lastReportedProgress = 0.0f; - - public void initialize(List 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; diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunInputData.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunInputData.java new file mode 100644 index 00000000..4793ea44 --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunInputData.java @@ -0,0 +1,13 @@ +package com.salesforce.sfca.pmdwrapper; + +import java.util.List; +import java.util.Map; + +public class PmdRunInputData { + public String ruleSetInputFile; + public Map runDataPerLanguage; + + public static class LanguageSpecificRunData { + public List filesToScan; + } +} diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunner.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunner.java index 94d3b107..ddfa4550 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunner.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdRunner.java @@ -2,24 +2,58 @@ import com.salesforce.sfca.shared.CodeLocation; import com.salesforce.sfca.shared.ProcessingError; +import com.salesforce.sfca.shared.ProgressReporter; import net.sourceforge.pmd.PMDConfiguration; import net.sourceforge.pmd.PmdAnalysis; +import net.sourceforge.pmd.lang.Language; +import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.document.TextFile; import net.sourceforge.pmd.reporting.*; +import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; public class PmdRunner { - PmdRunResults runRules(String ruleSetInputFile, String filesToScanInputFile) { - PMDConfiguration config = new PMDConfiguration(); - config.addRuleSet(ruleSetInputFile); - config.setInputFilePath(Paths.get(filesToScanInputFile)); + private final ProgressReporter progressReporter = new ProgressReporter(); + + PmdRunResults run(PmdRunInputData inputData) { + validateRunInputData(inputData); + + List languagesToProcess = new ArrayList<>(inputData.runDataPerLanguage.keySet()); + progressReporter.initialize(languagesToProcess); PmdRunResults runResults = new PmdRunResults(); + for (String language: languagesToProcess) { + runLanguage(language, inputData, runResults); + } + return runResults; + } + + private void runLanguage(String language, PmdRunInputData inputData, PmdRunResults runResults) { + PmdRunInputData.LanguageSpecificRunData languageSpecificRunData = inputData.runDataPerLanguage.get(language); + + PMDConfiguration config = new PMDConfiguration(); + config.addRuleSet(inputData.ruleSetInputFile); + List inputPathList = languageSpecificRunData.filesToScan.stream().map(Paths::get).collect(Collectors.toList()); + config.setInputPathList(inputPathList); + + // Force the language so that pmd doesn't look at file extensions. Note: we already associated the files based + // on their file extensions to the correct languages the typescript side. + Language pmdLanguageId = config.getLanguageRegistry().getLanguageById(language); + if (pmdLanguageId == null) { + throw new RuntimeException("The language \"" + language + "\" is not recognized by PMD."); + } + LanguageVersion forcedLangVer = config.getLanguageVersionDiscoverer() + .getDefaultLanguageVersion(pmdLanguageId); + config.setForceLanguageVersion(forcedLangVer); - // TODO: This is temporary. Soon we'll be looping over the languages. try (PmdAnalysis pmd = PmdAnalysis.create(config)) { - pmd.addListener(new PmdRunProgressListener()); + pmd.addListener(new PmdRunProgressListener(progressReporter, language)); Report report = pmd.performAnalysisAndCollectReport(); for (Report.ProcessingError reportProcessingError : report.getProcessingErrors()) { runResults.processingErrors.add( @@ -33,15 +67,44 @@ PmdRunResults runRules(String ruleSetInputFile, String filesToScanInputFile) { runResults.violations.add(violation); } } + } + + private void validateRunInputData(PmdRunInputData inputData) { + if (inputData.ruleSetInputFile == null) { + throw new RuntimeException(("The \"ruleSetInputFile\" field was missing.")); + } - return runResults; + if (inputData.runDataPerLanguage == null) { + throw new RuntimeException("The \"runDataPerLanguage\" field was not set."); + } + + Set> entries = inputData.runDataPerLanguage.entrySet(); + if (entries.isEmpty()) { + throw new RuntimeException("The \"runDataPerLanguage\" field didn't have any languages listed."); + } + + for (Map.Entry entry: entries) { + String language = entry.getKey(); + PmdRunInputData.LanguageSpecificRunData languageSpecificRunData = entry.getValue(); + if (languageSpecificRunData.filesToScan == null || languageSpecificRunData.filesToScan.isEmpty()) { + throw new RuntimeException(("The \"filesToScan\" field was missing or empty for language: " + language)); + } + } } } class PmdRunProgressListener implements GlobalAnalysisListener { + private final ProgressReporter progressReporter; + private final String language; + private int totalNumFiles = 0; private int fileCount = 0; + public PmdRunProgressListener(ProgressReporter progressReporter, String language) { + this.progressReporter = progressReporter; + this.language = language; + } + @Override public ListenerInitializer initializer() { return new ListenerInitializer() { @@ -57,7 +120,8 @@ 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); + progressReporter.updateProgressForLanguage(language, 100 * ((float) fileCount / totalNumFiles)); + progressReporter.reportOverallProgress(); return FileAnalysisListener.noop(); } diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdWrapper.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdWrapper.java index fc53606c..c00ca280 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdWrapper.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/pmdwrapper/PmdWrapper.java @@ -2,6 +2,7 @@ import com.google.gson.Gson; +import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; import java.nio.file.Files; @@ -22,19 +23,18 @@ * - {languages} is a comma separated list of languages associated with the rules to describe * RUN: * - Runs the rules provided by the input ruleset file on a set of files and writes results to a JSON file - * - Invocation: java -cp {classPath} com.salesforce.sfca.pmdwrapper.PmdWrapper run {ruleSetInputFile} {filesToScanInputFile} {resultsOutputFile} + * - Invocation: java -cp {classPath} com.salesforce.sfca.pmdwrapper.PmdWrapper run {argsInputFile} {resultsOutputFile} * - {classPath} is the list of entries to add to the class path * - {argsInputFile} is a JSON file containing the input arguments for the run command. * Example: * { + * "ruleSetInputFile": "/some/rulesetFileForRulesToRun.xml", * "runDataPerLanguage": { * "apex": { - * "ruleSetInputFile": "/some/rulesetFileForApexRules.xml", * "filesToScan": ["/full/path/to/apex_file1.cls", "/full/path/to/apex_file2.trigger", ...] * }, * ..., * "xml": { - * "ruleSetInputFile": "/some/rulesetFileForXmlRules.xml", * "filesToScan": ["/full/path/to/xml_file1.xml", "/full/path/to/xml_file2.xml", ...] * } * } @@ -123,19 +123,25 @@ private static List getCustomRulesetsFromFile(String file) { } private static void invokeRunCommand(String[] args) { - if (args.length != 3) { - throw new RuntimeException("Invalid number of arguments following the \"run\" command. Expected 3 but received: " + args.length); + if (args.length != 2) { + throw new RuntimeException("Invalid number of arguments following the \"run\" command. Expected 2 but received: " + args.length); } - String ruleSetInputFile = args[0]; - String filesToScanInputFile = args[1]; - String resultsOutputFile = args[2]; + String argsInputFile = args[0]; + String resultsOutputFile = args[1]; Gson gson = new Gson(); + PmdRunInputData inputData; + try (FileReader reader = new FileReader(argsInputFile)) { + inputData = gson.fromJson(reader, PmdRunInputData.class); + } catch (Exception e) { + throw new RuntimeException("Could not read contents from \"" + argsInputFile + "\"", e); + } + PmdRunner pmdRunner = new PmdRunner(); PmdRunResults results; try { - results = pmdRunner.runRules(ruleSetInputFile, filesToScanInputFile); + results = pmdRunner.run(inputData); } catch (Exception e) { throw new RuntimeException("Error while attempting to invoke PmdRunner.run: " + e.getMessage(), e); } diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/shared/ProgressReporter.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/shared/ProgressReporter.java new file mode 100644 index 00000000..9c0cb54b --- /dev/null +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/main/java/com/salesforce/sfca/shared/ProgressReporter.java @@ -0,0 +1,37 @@ +package com.salesforce.sfca.shared; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +// This class helps us track the overall progress of all language runs +public class ProgressReporter { + private Map progressPerLanguage = new HashMap<>(); + private float lastReportedProgress = 0.0f; + + public void initialize(List 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(); + } +} \ No newline at end of file diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/cpdwrapper/CpdWrapperTest.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/cpdwrapper/CpdWrapperTest.java index 3dac87c8..448d169d 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/cpdwrapper/CpdWrapperTest.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/cpdwrapper/CpdWrapperTest.java @@ -73,7 +73,7 @@ void whenCallingMainWithRunAndTooFewArgs_thenError() { } @Test - void whenCallingMainWithDescribeAndTooManyArgs_thenError() { + void whenCallingMainWithRunAndTooManyArgs_thenError() { String[] args = {"run", "too", "many", "args"}; Exception thrown = assertThrows(Exception.class, () -> callCpdWrapper(args)); assertThat(thrown.getMessage(), is("Invalid number of arguments following the \"run\" command. Expected 2 but received: 3")); diff --git a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java index 1c998d04..63736094 100644 --- a/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java +++ b/packages/code-analyzer-pmd-engine/pmd-cpd-wrappers/src/test/java/com/salesforce/sfca/pmdwrapper/PmdWrapperTest.java @@ -9,6 +9,7 @@ import com.google.gson.Gson; import com.google.gson.JsonElement; import com.google.gson.JsonParser; +import com.google.gson.JsonSyntaxException; import com.google.gson.reflect.TypeToken; import com.salesforce.sfca.testtools.StdOutCaptor; import org.junit.jupiter.api.Test; @@ -126,16 +127,143 @@ void whenCallingMainWithDescribeWithCustomRulesetsFile_thenRulesetsAreApplied(@T @Test void whenCallingMainWithRunAndTwoFewArgs_thenError() { - String[] args = {"run", "not", "enough"}; + String[] args = {"run", "notEnough"}; Exception thrown = assertThrows(Exception.class, () -> callPmdWrapper(args)); - assertThat(thrown.getMessage(), is("Invalid number of arguments following the \"run\" command. Expected 3 but received: 2")); + assertThat(thrown.getMessage(), is("Invalid number of arguments following the \"run\" command. Expected 2 but received: 1")); } @Test void whenCallingMainWithRunAndTooManyArgs_thenError() { - String[] args = {"run", "too", "many", "args", "unfortunately"}; + String[] args = {"run", "too", "many", "args"}; Exception thrown = assertThrows(Exception.class, () -> callPmdWrapper(args)); - assertThat(thrown.getMessage(), is("Invalid number of arguments following the \"run\" command. Expected 3 but received: 4")); + assertThat(thrown.getMessage(), is("Invalid number of arguments following the \"run\" command. Expected 2 but received: 3")); + } + + @Test + void whenCallingMainWithRunAndInputFileThatDoesNotExist_thenError() { + String[] args = {"run", "/does/not/exist.json", "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), containsString("Could not read contents from \"/does/not/exist.json\"")); + assertThat(thrown.getCause(), instanceOf(FileNotFoundException.class)); + } + + @Test + void whenCallingMainWithRunAndInputFileThatDoesNotContainValidJson_thenError(@TempDir Path tempDir) throws Exception { + String inputFileContents = "{\"oops"; // Not valid json + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), containsString("Could not read contents from \"" + inputFile + "\"")); + assertThat(thrown.getCause(), instanceOf(JsonSyntaxException.class)); + } + + @Test + void whenCallingRunWithMissingField_runDataPerLanguage_thenError(@TempDir Path tempDir) throws Exception { + String ruleSetInputFile = createSampleRulesetFile(tempDir); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\"\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The \"runDataPerLanguage\" field was not set.")); + } + + @Test + void whenCallingRunWithZeroLanguages_thenError(@TempDir Path tempDir) throws Exception { + String ruleSetInputFile = createSampleRulesetFile(tempDir); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\",\n" + + " \"runDataPerLanguage\": {" + + " }\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The \"runDataPerLanguage\" field didn't have any languages listed.")); + } + + @Test + void whenCallingRunWithMissingField_filesToScan_thenError(@TempDir Path tempDir) throws Exception { + String ruleSetInputFile = createSampleRulesetFile(tempDir); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\",\n" + + " \"runDataPerLanguage\": {" + + " \"apex\": {}\n" + + " }\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The \"filesToScan\" field was missing or empty for language: apex")); + } + + @Test + void whenCallingRunWithEmptyArrayFor_filesToScan_thenError(@TempDir Path tempDir) throws Exception { + String ruleSetInputFile = createSampleRulesetFile(tempDir); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\",\n" + + " \"runDataPerLanguage\": {" + + " \"apex\": {\n" + + " \"filesToScan\": []\n" + + " }\n" + + " }\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The \"filesToScan\" field was missing or empty for language: apex")); + } + + @Test + void whenCallingRunWithInvalidLanguage_thenError(@TempDir Path tempDir) throws Exception { + String ruleSetInputFile = createSampleRulesetFile(tempDir); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\",\n" + + " \"runDataPerLanguage\": {" + + " \"unknownLanguage\": {\n" + + " \"filesToScan\": [\"" + makePathJsonSafe(ruleSetInputFile) + "\"]\n" + + " }\n" + + " }\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The language \"unknownLanguage\" is not recognized by PMD.")); + } + + @Test + void whenCallingRunWithMissingField_ruleSetInputFile_thenError(@TempDir Path tempDir) throws Exception { + String dummyApexFile = createTempFile(tempDir, "dummy.cls", ""); + String inputFileContents = "{" + + " \"runDataPerLanguage\": {" + + " \"apex\": {" + + " \"filesToScan\": [\"" + makePathJsonSafe(dummyApexFile) + "\"]" + + " }" + + " }" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); + + String[] args = {"run", inputFile, "/does/not/matter"}; + RuntimeException thrown = assertThrows(RuntimeException.class, () -> callPmdWrapper(args)); + assertThat(thrown.getMessage(), is( + "Error while attempting to invoke PmdRunner.run: The \"ruleSetInputFile\" field was missing.")); } @Test @@ -144,19 +272,8 @@ void whenCallingMainWithRunAndValidArgs_thenItWritesValidJsonToFile(@TempDir Pat // and then copy them to a temp directory (which is needed because the utility assumes absolute file paths). // But if this list grows, then we'll need to create a utility to make this easier. - Path ruleSetInputFilePath = tempDir.resolve("ruleSetInputFile.xml"); - String ruleSetContents = "\n" + - "\n" + - " Rules to Run for Salesforce Code Analyzer\n" + - " \n" + - " \n" + - ""; - Files.write(ruleSetInputFilePath, ruleSetContents.getBytes()); + String ruleSetInputFile = createSampleRulesetFile(tempDir); - Path sampleApexFile = tempDir.resolve("OperationWithLimitsInLoop.cls"); String apexViolationCode = "public class OperationWithLimitsInLoop {\n" + " public static void main() {\n" + " for (Integer i = 0; i < 10; i++) {\n" + @@ -164,25 +281,35 @@ void whenCallingMainWithRunAndValidArgs_thenItWritesValidJsonToFile(@TempDir Pat " }\n" + " }\n" + "}"; - Files.write(sampleApexFile, apexViolationCode.getBytes()); + String sampleApexFile = createTempFile(tempDir, "OperationWithLimitsInLoop.cls", apexViolationCode); - Path sampleVfFile = tempDir.resolve("VfUnescapeEl.page"); String vfViolationCode = "\n" + " \n" + ""; - Files.write(sampleVfFile, vfViolationCode.getBytes()); - - Path filesToScanInputFilePath = tempDir.resolve("filesToScan.txt"); - String filesToScanContents = sampleApexFile.toAbsolutePath() + "\n" + sampleVfFile.toAbsolutePath(); - Files.write(filesToScanInputFilePath, filesToScanContents.getBytes()); + String sampleVfFile = createTempFile(tempDir, "VfUnescapeEl.page", vfViolationCode); + + String inputFileContents = "{\n" + + " \"ruleSetInputFile\":\"" + makePathJsonSafe(ruleSetInputFile) + "\",\n" + + " \"runDataPerLanguage\": {\n" + + " \"apex\": {\n" + + " \"filesToScan\": [\n" + + " \"" + makePathJsonSafe(sampleApexFile) + "\"" + + " ]\n" + + " },\n" + + " \"visualforce\": {\n" + + " \"filesToScan\": [\n" + + " \"" + makePathJsonSafe(sampleVfFile) + "\"" + + " ]\n" + + " }\n" + + " }\n" + + "}"; + String inputFile = createTempFile(tempDir, "inputFile.json", inputFileContents); - String ruleSetInputFile = ruleSetInputFilePath.toAbsolutePath().toString(); - String filesToScanInputFile = filesToScanInputFilePath.toAbsolutePath().toString(); String resultsOutputFile = tempDir.resolve("results.json").toAbsolutePath().toString(); - String[] args = {"run", ruleSetInputFile, filesToScanInputFile, resultsOutputFile}; + String[] args = {"run", inputFile, resultsOutputFile}; String stdOut = callPmdWrapper(args); // Should not error // Assert results contain valid json and that the expected rule violations (note this is more thoroughly tested on the typescript side) @@ -197,11 +324,29 @@ void whenCallingMainWithRunAndValidArgs_thenItWritesValidJsonToFile(@TempDir Pat // 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("[Progress]50.0"), + containsString("[Progress]100.0"), containsString("milliseconds"))); } + private static String createSampleRulesetFile(Path tempDir) throws Exception { + String ruleSetContents = "\n" + + "\n" + + " Rules to Run for Salesforce Code Analyzer\n" + + " \n" + + " \n" + + ""; + return createTempFile(tempDir, "ruleSetInputFile.xml", ruleSetContents); + } + + private static String createTempFile(Path tempDir, String fileName, String fileContents) throws Exception { + Path inputFilePath = tempDir.resolve(fileName); + Files.write(inputFilePath, fileContents.getBytes()); + return inputFilePath.toAbsolutePath().toString(); + } private static String callPmdWrapper(String[] args) { try (StdOutCaptor stdoutCaptor = new StdOutCaptor()) { @@ -209,5 +354,10 @@ private static String callPmdWrapper(String[] args) { return stdoutCaptor.getCapturedOutput(); } } + + private static String makePathJsonSafe(String file) { + return file.replace("\\", "\\\\") // Escape backslashes + .replace("\"", "\\\""); // Escape double quotes + } } diff --git a/packages/code-analyzer-pmd-engine/src/pmd-engine.ts b/packages/code-analyzer-pmd-engine/src/pmd-engine.ts index fc7f0328..011ff5c6 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-engine.ts +++ b/packages/code-analyzer-pmd-engine/src/pmd-engine.ts @@ -1,5 +1,4 @@ import { - CodeLocation, COMMON_TAGS, DescribeOptions, Engine, @@ -15,7 +14,13 @@ import {indent, JavaCommandExecutor, WorkspaceLiaison} from "./utils"; import path from "node:path"; import * as fs from 'node:fs/promises'; import {extensionToLanguageId, LanguageId, PMD_ENGINE_NAME, SHARED_RULE_NAMES} from "./constants"; -import {PmdResults, PmdRuleInfo, PmdViolation, PmdWrapperInvoker} from "./pmd-wrapper"; +import { + LanguageSpecificPmdRunData, + PmdResults, + PmdRuleInfo, + PmdViolation, + PmdWrapperInvoker +} from "./pmd-wrapper"; import {getMessage} from "./messages"; import {PmdEngineConfig} from "./config"; import {RULE_MAPPINGS} from "./pmd-rule-mappings"; @@ -63,29 +68,35 @@ export class PmdEngine extends Engine { async runRules(ruleNames: string[], runOptions: RunOptions): Promise { const workspaceLiaison: WorkspaceLiaison = this.getWorkspaceLiaison(runOptions.workspace); + const relevantLanguageToFilesMap: Map = await workspaceLiaison.getRelevantLanguageToFilesMap(); this.emitRunRulesProgressEvent(2); - const filesToScan: string[] = await workspaceLiaison.getRelevantFiles(); - if (ruleNames.length === 0 || filesToScan.length === 0) { - this.emitRunRulesProgressEvent(100); - return {violations: []}; - } - this.emitRunRulesProgressEvent(4); - const ruleInfoList: PmdRuleInfo[] = await this.getPmdRuleInfoList(workspaceLiaison, - (innerPerc: number) => this.emitRunRulesProgressEvent(4 + 6*(innerPerc/100))); // 4 to 10% + (innerPerc: number) => this.emitRunRulesProgressEvent(2 + 3*(innerPerc/100))); // 2 to 5% const selectedRuleInfoList: PmdRuleInfo[] = ruleNames .map(ruleName => fetchRuleInfoByRuleName(ruleInfoList, ruleName)) .filter(ruleInfo => ruleInfo !== null); - if (selectedRuleInfoList.length === 0) { + const runDataPerLanguage: Record = {}; + const relevantLanguages: Set = new Set(selectedRuleInfoList.map(ruleInfo => ruleInfo.language)); + for (const language of relevantLanguages) { + const filesToScanForLanguage: string[] = relevantLanguageToFilesMap.get(toLanguageId(language)) || /* istanbul ignore next */ []; + if (filesToScanForLanguage.length > 0) { + runDataPerLanguage[language] = { + filesToScan: filesToScanForLanguage + } + } + } + + if (Object.keys(runDataPerLanguage).length === 0) { this.emitRunRulesProgressEvent(100); - return {violations: []}; + return { violations: [] }; } - const pmdResults: PmdResults = await this.pmdWrapperInvoker.invokeRunCommand(selectedRuleInfoList, filesToScan, - (innerPerc: number) => this.emitRunRulesProgressEvent(10 + 88*(innerPerc/100))); // 10 to 98% + + const pmdResults: PmdResults = await this.pmdWrapperInvoker.invokeRunCommand(selectedRuleInfoList, runDataPerLanguage, + (innerPerc: number) => this.emitRunRulesProgressEvent(5 + 93*(innerPerc/100))); // 5 to 98% const violations: Violation[] = []; for (const pmdViolation of pmdResults.violations) { diff --git a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts index 99555744..f42e6b48 100644 --- a/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts +++ b/packages/code-analyzer-pmd-engine/src/pmd-wrapper.ts @@ -18,6 +18,14 @@ export type PmdRuleInfo = { class: string } +export type PmdRunInputData = { + ruleSetInputFile: string, + runDataPerLanguage: Record +} +export type LanguageSpecificPmdRunData = { + filesToScan: string[] +} + export type PmdResults = { violations: PmdViolation[] processingErrors: PmdProcessingError[] @@ -90,7 +98,7 @@ export class PmdWrapperInvoker { } } - async invokeRunCommand(pmdRuleInfoList: PmdRuleInfo[], filesToScan: string[], emitProgress: (percComplete: number) => void): Promise { + async invokeRunCommand(pmdRuleInfoList: PmdRuleInfo[], runDataPerLanguage: Record, emitProgress: (percComplete: number) => void): Promise { const tempDir: string = await this.getTemporaryWorkingDir(); emitProgress(2); @@ -99,12 +107,17 @@ export class PmdWrapperInvoker { 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'); + const inputData: PmdRunInputData = { + ruleSetInputFile: ruleSetInputFile, + runDataPerLanguage: runDataPerLanguage + } + + const inputFile: string = path.join(tempDir, 'pmdRunInput.json'); + await fs.promises.writeFile(inputFile, JSON.stringify(inputData), 'utf-8'); emitProgress(10); const resultsOutputFile: string = path.join(tempDir, 'resultsFile.json'); - const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'run', ruleSetInputFile, filesToScanInputFile, resultsOutputFile]; + const javaCmdArgs: string[] = [PMD_WRAPPER_JAVA_CLASS, 'run', inputFile, resultsOutputFile]; const javaClassPaths: string[] = [ path.join(PMD_WRAPPER_LIB_FOLDER, '*'), ... this.userProvidedJavaClasspathEntries.map(toJavaClasspathEntry) @@ -112,10 +125,8 @@ export class PmdWrapperInvoker { 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 parts: string[] = stdOutMsg.slice(STDOUT_PROGRESS_MARKER.length).split('::'); - const numFilesProcessed: number = parseInt(parts[0]); - const totalNumFiles: number = parseInt(parts[1]); - emitProgress(10 + (80 * numFilesProcessed / totalNumFiles)); + const pmdWrapperProgress: number = parseFloat(stdOutMsg.slice(STDOUT_PROGRESS_MARKER.length)); + emitProgress(10 + (80 * pmdWrapperProgress / 100)); // 10 to 90% } else { this.emitLogEvent(LogLevel.Fine, `[JAVA StdOut]: ${stdOutMsg}`); } diff --git a/packages/code-analyzer-pmd-engine/src/utils.ts b/packages/code-analyzer-pmd-engine/src/utils.ts index db21b9e0..946eb329 100644 --- a/packages/code-analyzer-pmd-engine/src/utils.ts +++ b/packages/code-analyzer-pmd-engine/src/utils.ts @@ -77,10 +77,6 @@ export class WorkspaceLiaison { return this.workspace; } - async getRelevantFiles(): Promise { - return [... (await this.getRelevantLanguageToFilesMap()).values()].flat(); - } - async getRelevantLanguages(): Promise { return [...(await this.getRelevantLanguageToFilesMap()).keys()].sort(); } 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 01fabcc4..c39d9e6a 100644 --- a/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts +++ b/packages/code-analyzer-pmd-engine/test/pmd-engine.test.ts @@ -448,7 +448,7 @@ describe('Tests for the runRules method of PmdEngine', () => { // 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, 30.53, 42.27, 54, 65.73, 77.47, 89.2, 93.6, 98, 100]); + [2, 2.3, 4.4, 4.7, 5, 6.86, 10.58, 14.3, 23.6, 32.9, 42.2, 51.5, 70.1, 88.7, 93.35, 98, 100]); }); it('When a single rule is selected, then return only violations for that rule', async () => {