NEW(pmd): @W-16606013@: Add ability to add files to java classpath an…

…d specify custom rulesets (#106)

packages/code-analyzer-engine-api/src/config.ts

@@ -27,7 +27,7 @@ export class ConfigValueExtractor {
     private readonly fieldPathRoot: string;
     private readonly configRoot: string;
 
-    constructor(configObject: ConfigObject, fieldPathRoot: string = '', configRoot: string = process.cwd(), ) {
+    constructor(configObject: ConfigObject, fieldPathRoot: string = '', configRoot: string = process.cwd()) {
         this.configObj = configObject;
         this.fieldPathRoot = fieldPathRoot;
         this.configRoot = configRoot;
@@ -114,15 +114,15 @@ export class ConfigValueExtractor {
     }
 
     extractRequiredFile(fieldName: string): string {
-        return ValueValidator.validateFile(this.configObj[fieldName], this.getFieldPath(fieldName), this.getConfigRoot());
+        return ValueValidator.validateFile(this.configObj[fieldName], this.getFieldPath(fieldName), [this.getConfigRoot()]);
     }
 
     extractFile(fieldName: string, defaultValue?: string): string | undefined {
         return !this.hasValueDefinedFor(fieldName) ? defaultValue : this.extractRequiredFile(fieldName);
     }
 
     extractRequiredFolder(fieldName: string): string {
-        return ValueValidator.validateFolder(this.configObj[fieldName], this.getFieldPath(fieldName), this.getConfigRoot());
+        return ValueValidator.validateFolder(this.configObj[fieldName], this.getFieldPath(fieldName), [this.getConfigRoot()]);
     }
 
     extractFolder(fieldName: string, defaultValue?: string): string | undefined {
@@ -190,27 +190,26 @@ export class ValueValidator {
         return value as T[];
     }
 
-    static validateFile(value: unknown, fieldPath: string, possibleFileRoot?: string): string {
-        const fileValue: string = ValueValidator.validatePath(value, fieldPath, possibleFileRoot);
+    static validateFile(value: unknown, fieldPath: string, possiblePathRoots: string[] = []): string {
+        const fileValue: string = ValueValidator.validatePath(value, fieldPath, possiblePathRoots);
         if (fs.statSync(fileValue).isDirectory()) {
             throw new Error(getMessage('ConfigFileValueMustNotBeFolder', fieldPath, fileValue));
         }
         return fileValue;
     }
 
-    static validateFolder(value: unknown, fieldPath: string, possibleFolderRoot?: string): string {
-        const folderValue: string = ValueValidator.validatePath(value, fieldPath, possibleFolderRoot);
+    static validateFolder(value: unknown, fieldPath: string, possiblePathRoots: string[] = []): string {
+        const folderValue: string = ValueValidator.validatePath(value, fieldPath, possiblePathRoots);
         if (!fs.statSync(folderValue).isDirectory()) {
             throw new Error(getMessage('ConfigFolderValueMustNotBeFile', fieldPath, folderValue));
         }
         return folderValue;
     }
 
-    static validatePath(value: unknown, fieldPath: string, possiblePathRoot?: string): string {
+    static validatePath(value: unknown, fieldPath: string, possiblePathRoots: string[] = []): string {
         const pathValue: string = ValueValidator.validateString(value, fieldPath);
         const pathsToTry: string[] = [];
-        if (possiblePathRoot) {
-            // If a possible root is supplied, then we first assume the pathValue is relative to it
+        for (const possiblePathRoot of possiblePathRoots) {
             pathsToTry.push(toAbsolutePath(pathValue, possiblePathRoot));
         }
         // Otherwise we try to resolve it without a possible root

packages/code-analyzer-engine-api/src/engines.ts

@@ -48,14 +48,18 @@ export abstract class Engine {
     protected emitDescribeRulesProgressEvent(percentComplete: number): void {
         this.emitEvent({
             type: EventType.DescribeRulesProgressEvent,
-            percentComplete: percentComplete
+            percentComplete: roundToHundredths(percentComplete)
         });
     }
 
     protected emitRunRulesProgressEvent(percentComplete: number): void {
         this.emitEvent({
             type: EventType.RunRulesProgressEvent,
-            percentComplete: percentComplete
+            percentComplete: roundToHundredths(percentComplete)
         });
     }
+}
+
+export function roundToHundredths(num: number): number {
+    return Math.round(num * 100) / 100;
 }

packages/code-analyzer-engine-api/test/config.test.ts

@@ -88,6 +88,39 @@ describe("Tests for ValueValidator", () => {
             ['resolved', 'resolved']);
     });
 
+    it("When an absolute file is given to validatePath, then it is returned", () => {
+        const inputFile: string = path.resolve(__dirname, 'config.test.ts');
+        expect(ValueValidator.validatePath(inputFile, 'someFieldName')).toEqual(inputFile);
+    });
+
+    it("When an relative file is given to validatePath with a correct root, then it is returned", () => {
+        const inputFile: string = 'test-data/sampleWorkspace';
+        const inputRoot: string = __dirname;
+        expect(ValueValidator.validatePath(inputFile, 'someFieldName', [
+            path.resolve(__dirname, '..'), // Not correct (so it should try the next one)
+            inputRoot // Correct
+        ])).toEqual(
+            path.resolve(__dirname, 'test-data', 'sampleWorkspace'));
+    });
+
+    it("When an non-string value is given to validatePath, then error", () => {
+        expect(() => ValueValidator.validatePath(3, 'someFieldName')).toThrow(
+            getMessage('ConfigValueMustBeOfType', 'someFieldName', 'string', 'number'));
+    });
+
+    it("When an absolute file that does not exist is given to validatePath, then error", () => {
+        const inputFile: string = path.resolve(__dirname, 'does.not.exist');
+        expect(() => ValueValidator.validatePath(inputFile, 'someFieldName')).toThrow(
+            getMessage('ConfigPathValueDoesNotExist', 'someFieldName', inputFile));
+    });
+
+    it("When an relative file that does not exist is given to validatePath, then error", () => {
+        const inputFile: string = 'test-data/does.not.exist';
+        const inputRoot: string = __dirname;
+        expect(() => ValueValidator.validatePath(inputFile, 'someFieldName', [inputRoot])).toThrow(
+            getMessage('ConfigPathValueDoesNotExist', 'someFieldName', path.resolve(__dirname, 'test-data', 'does.not.exist')));
+    });
+
     it("When an absolute file is given to validateFile, then it is returned", () => {
         const inputFile: string = path.resolve(__dirname, 'config.test.ts');
         expect(ValueValidator.validateFile(inputFile, 'someFieldName')).toEqual(inputFile);
@@ -96,7 +129,7 @@ describe("Tests for ValueValidator", () => {
     it("When an relative file is given to validateFile with a correct root, then it is returned", () => {
         const inputFile: string = 'test-data/sampleWorkspace/someFile.txt';
         const inputRoot: string = __dirname;
-        expect(ValueValidator.validateFile(inputFile, 'someFieldName', inputRoot)).toEqual(
+        expect(ValueValidator.validateFile(inputFile, 'someFieldName', [inputRoot])).toEqual(
             path.resolve(__dirname, 'test-data', 'sampleWorkspace', 'someFile.txt'));
     });
 
@@ -114,7 +147,7 @@ describe("Tests for ValueValidator", () => {
     it("When an relative file that does not exist is given to validateFile, then error", () => {
         const inputFile: string = 'test-data/does.not.exist';
         const inputRoot: string = __dirname;
-        expect(() => ValueValidator.validateFile(inputFile, 'someFieldName', inputRoot)).toThrow(
+        expect(() => ValueValidator.validateFile(inputFile, 'someFieldName', [inputRoot])).toThrow(
             getMessage('ConfigPathValueDoesNotExist', 'someFieldName', path.resolve(__dirname, 'test-data', 'does.not.exist')));
     });
 
@@ -130,7 +163,7 @@ describe("Tests for ValueValidator", () => {
     it("When an relative folder is given to validateFolder with a correct root, then it is returned", () => {
         const inputFile: string = 'test-data/sampleWorkspace';
         const inputRoot: string = __dirname;
-        expect(ValueValidator.validateFolder(inputFile, 'someFieldName', inputRoot)).toEqual(
+        expect(ValueValidator.validateFolder(inputFile, 'someFieldName', [inputRoot])).toEqual(
             path.resolve(__dirname, 'test-data', 'sampleWorkspace'));
     });
 
@@ -148,7 +181,7 @@ describe("Tests for ValueValidator", () => {
     it("When an relative folder that does not exist is given to validateFolder, then error", () => {
         const inputFile: string = 'doesNotExist';
         const inputRoot: string = __dirname;
-        expect(() => ValueValidator.validateFile(inputFile, 'someFieldName', inputRoot)).toThrow(
+        expect(() => ValueValidator.validateFile(inputFile, 'someFieldName', [inputRoot])).toThrow(
             getMessage('ConfigPathValueDoesNotExist', 'someFieldName', path.resolve(__dirname, 'doesNotExist')));
     });
 

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

@@ -4,11 +4,18 @@
 import net.sourceforge.pmd.PmdAnalysis;
 import net.sourceforge.pmd.lang.rule.Rule;
 import net.sourceforge.pmd.lang.rule.RuleSet;
+import net.sourceforge.pmd.lang.rule.RuleSetLoadException;
+import net.sourceforge.pmd.util.log.PmdReporter;
+import org.slf4j.event.Level;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Pattern;
+import java.util.regex.Matcher;
 
 class PmdRuleDescriber {
     /**
@@ -65,21 +72,51 @@ class PmdRuleDescriber {
             )
     );
 
-    List<PmdRuleInfo> describeRulesFor(Set<String> languages) {
+    List<PmdRuleInfo> describeRulesFor(List<String> customRulesets, Set<String> languages) {
         List<PmdRuleInfo> ruleInfoList = new ArrayList<>();
         PMDConfiguration config = new PMDConfiguration();
+
+        // Set Reported which will throw error during PmdAnalysis.create if a custom ruleset cannot be found
+        config.setReporter(new PmdErrorListener());
+
+        // Add in user specified custom rulesets, which must be added before standard rules so that they take preference
+        // in case of duplication
+        for (String customRuleset : customRulesets) {
+            config.addRuleSet(customRuleset);
+        }
+
+        // Add in standard rulesets based on the languages specified
         for (String lang : languages) {
             for (String ruleSetFile : LANGUAGE_TO_STANDARD_RULESETS.get(lang)) {
                 config.addRuleSet(ruleSetFile);
             }
         }
 
+        // Keep track of "<language>::<ruleName>" so that we don't have duplicates
+        Set<String> alreadySeen = new HashSet<>();
+
         try (PmdAnalysis pmd = PmdAnalysis.create(config)) {
             for (RuleSet ruleSet : pmd.getRulesets()) {
                 for (Rule rule : ruleSet.getRules()) {
+
+                    // Filter out custom rules that don't match languages specified
+                    String language = rule.getLanguage().toString();
+                    if (!languages.contains(language)) {
+                        continue;
+                    }
+
+                    // Filter out any rules that we have already seen (duplicates) which can happen if user specifies a
+                    // ruleset that references an existing rule
+                    String langPlusName = language + "::" + rule.getName();
+                    if(alreadySeen.contains(langPlusName)) {
+                        continue;
+                    }
+                    alreadySeen.add(langPlusName);
+
+                    // Add rule info
                     PmdRuleInfo pmdRuleInfo = new PmdRuleInfo();
                     pmdRuleInfo.name = rule.getName();
-                    pmdRuleInfo.language = rule.getLanguage().toString();
+                    pmdRuleInfo.language = language;
                     pmdRuleInfo.description = getLimitedDescription(rule);
                     pmdRuleInfo.externalInfoUrl = rule.getExternalInfoUrl();
                     pmdRuleInfo.ruleSet = rule.getRuleSetName();
@@ -89,6 +126,7 @@ List<PmdRuleInfo> describeRulesFor(Set<String> languages) {
                 }
             }
         }
+
         return ruleInfoList;
     }
 
@@ -115,3 +153,39 @@ private static String getLimitedDescription(Rule rule) {
         return ruleDescription;
     }
 }
+
+// This class simply helps us process any errors that may be thrown by PMD. By default PMD suppresses errors so that
+// they are not thrown. So here, we look out for the errors that we care about and process it to throw a better
+// error messages.
+class PmdErrorListener implements PmdReporter {
+    @Override
+    public void logEx(Level level, @Nullable String s, Object[] objects, @Nullable Throwable throwable) {
+        if (throwable != null) {
+            String message = throwable.getMessage();
+            if (throwable instanceof RuleSetLoadException && message.contains("Cannot load ruleset ")) {
+                Pattern pattern = Pattern.compile("Cannot load ruleset (.+?):");
+                Matcher matcher = pattern.matcher(message);
+                if (matcher.find()) {
+                    String ruleset = matcher.group(1).trim();
+                    String errorMessage = "PMD errored when attempting to load a custom ruleset \"" + ruleset + "\". " +
+                            "Make sure the resource is a valid file on disk or on the Java classpath.";
+
+                    // The typescript side can more easily handle error messages that come from stdout with "[Error] " marker
+                    System.out.println("[Error] " + errorMessage);
+                    throw new RuntimeException(errorMessage, throwable);
+                }
+            }
+            throw new RuntimeException("PMD threw an unexpected exception:\n" + message, throwable);
+        }
+    }
+
+    // These methods aren't needed or used, but they are required to be implemented (since the interface does not give them default implementations)
+    @Override
+    public boolean isLoggable(Level level) {
+        return false;
+    }
+    @Override
+    public int numErrors() {
+        return 0;
+    }
+}

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

@@ -4,6 +4,9 @@
 
 import java.io.FileWriter;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
@@ -47,14 +50,16 @@ public static void main(String[] args) {
     }
 
     private static void invokeDescribeCommand(String[] args) {
-        if (args.length != 2) {
-            throw new RuntimeException("Invalid number of arguments following the \"describe\" command. Expected 2 but received: " + args.length);
+        if (args.length != 3) {
+            throw new RuntimeException("Invalid number of arguments following the \"describe\" command. Expected 3 but received: " + args.length);
         }
+
         String outFile = args[0];
-        Set<String> languages = Arrays.stream(args[1].toLowerCase().split(",")).collect(Collectors.toSet());
+        List<String> customRulesets = getCustomRulesetsFromFile(args[1]);
+        Set<String> languages = Arrays.stream(args[2].toLowerCase().split(",")).collect(Collectors.toSet());
 
         PmdRuleDescriber ruleDescriber = new PmdRuleDescriber();
-        List<PmdRuleInfo> pmdRuleInfoList = ruleDescriber.describeRulesFor(languages);
+        List<PmdRuleInfo> pmdRuleInfoList = ruleDescriber.describeRulesFor(customRulesets, languages);
 
         Gson gson = new Gson();
         try (FileWriter fileWriter = new FileWriter(outFile)) {
@@ -64,6 +69,18 @@ private static void invokeDescribeCommand(String[] args) {
         }
     }
 
+    private static List<String> getCustomRulesetsFromFile(String file) {
+        Path path = Paths.get(file);
+        try {
+            return Files.readAllLines(path).stream()
+                    .map(String::trim)
+                    .filter(line -> !line.isEmpty())
+                    .collect(Collectors.toList());
+        } catch (IOException e) {
+            throw new RuntimeException("Could not read contents of " + file, e);
+        }
+    }
+
     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);