@W-7792085@ Filter out invalid catalog files

messages/EventKeyTemplates.js

@@ -8,8 +8,9 @@ module.exports = {
 		"invalidCategorySkipped": "Cataloger skipped invalid PMD Category file '%s'.",
 		"invalidRulesetSkipped": "Cataloger skipped invalid PMD Ruleset file '%s'.",
 		"xmlDropped": "Dropping XML file [%s] since its path does not conform to Rulesets or Category.",
-		"pmdSkippedFile": "PMD failed to evaluate against file '%s'. Message: %s"
-	},
+		"pmdSkippedFile": "PMD failed to evaluate against file '%s'. Message: %s",
+		"customRuleFileNotFound": "Custom rule file path [%s] for language [%s] was not found.",
+},
 	"error": {
 		"internal": {
 			"unexpectedError": "Unexpected error occurred while cataloging rules: %s",

src/lib/pmd/PmdCatalogWrapper.ts

@@ -9,6 +9,7 @@ import {OutputProcessor} from './OutputProcessor';
 import * as PmdLanguageManager from './PmdLanguageManager';
 import {PMD_LIB, PMD_VERSION, PmdSupport} from './PmdSupport';
 import path = require('path');
+import {uxEvents} from '../ScannerEvents';
 
 Messages.importMessagesDirectory(__dirname);
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'EventKeyTemplates');
@@ -97,9 +98,12 @@ export class PmdCatalogWrapper extends PmdSupport {
 	protected async getRulePathEntries(): Promise<Map<string, Set<string>>> {
 		const pathSetMap = new Map<string, Set<string>>();
 
-		const customPathEntries: Map<string, Set<string>> = await this.getCustomRulePathEntries();
+		const customRulePaths: Map<string, Set<string>> = await this.getCustomRulePathEntries();
+		const fileHandler = new FileHandler();
+
+
 		// Iterate through the custom paths.
-		customPathEntries.forEach(async (paths: Set<string>, langKey: string) => {
+		for (const [langKey, paths] of customRulePaths.entries()) {
 			// If the language by which these paths are mapped can be de-aliased into one of PMD's default-supported
 			// languages, we should use the name PMD recognizes. That way, if they have custom paths for 'ecmascript'
 			// and 'js', we'll turn both of those into 'javascript'.
@@ -112,11 +116,16 @@ export class PmdCatalogWrapper extends PmdSupport {
 			const pathSet = pathSetMap.get(lang) || new Set<string>();
 			if (paths) {
 				for (const value of paths.values()) {
-					pathSet.add(value);
+					if (await fileHandler.exists(value)) {
+						pathSet.add(value);
+					} else {
+						// The catalog file may have been deleted or moved. Show the user a warning.
+						uxEvents.emit('warning-always', messages.getMessage('warning.customRuleFileNotFound', [value, lang]));
+					}
 				}
 			}
 			pathSetMap.set(lang, pathSet);
-		});
+		}
 
 		// Now, we'll want to add the default PMD JARs for any activated languages.
 		const supportedLanguages = await PmdLanguageManager.getSupportedLanguages();

test/lib/pmd/PmdCatalogWrapper.test.ts

@@ -5,7 +5,10 @@ import {Config} from '../../../src/lib/util/Config';
 import {expect} from 'chai';
 import Sinon = require('sinon');
 import path = require('path');
-import { ENGINE } from '../../../src/Constants';
+import {ENGINE,LANGUAGE} from '../../../src/Constants';
+import {FileHandler} from '../../../src/lib/util/FileHandler';
+import {uxEvents} from '../../../src/lib/ScannerEvents';
+
 // In order to get access to PmdCatalogWrapper's protected methods, we're going to extend it with a test class here.
 class TestablePmdCatalogWrapper extends PmdCatalogWrapper {
 	public async buildCommandArray(): Promise<[string, string[]]> {
@@ -25,11 +28,12 @@ describe('PmdCatalogWrapper', () => {
 				before(() => {
 					Sinon.createSandbox();
 					// Spoof a config that claims that only Apex's default PMD JAR is enabled.
-					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves(['apex']);
+					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves([LANGUAGE.APEX]);
 					// Spoof a CustomPathManager that claims that a custom JAR exists for Java.
 					const customJars: Map<string, Set<string>> = new Map();
-					customJars.set('java', new Set([irrelevantPath]));
+					customJars.set(LANGUAGE.JAVA, new Set([irrelevantPath]));
 					Sinon.stub(CustomRulePathManager.prototype, 'getRulePathEntries').withArgs(PmdEngine.NAME).resolves(customJars);
+					Sinon.stub(FileHandler.prototype, 'exists').resolves(true);
 				});
 
 				after(() => {
@@ -42,7 +46,7 @@ describe('PmdCatalogWrapper', () => {
 					const params = (await target.buildCommandArray())[1];
 
 					// Expect there to be 6 parameters.
-					expect(params.length).to.equal(6, 'Should have been 6 parameters');
+					expect(params.length).to.equal(6, `Should have been 6 parameters: ${params}`);
 					// Expect the first two parameters to be the catalog and -cp flag.
 					expect(params[0]).to.equal('-DcatalogName=PmdCatalog.json', 'First parameter should be catalog override');
 					expect(params[1]).to.equal('-cp', 'Second parameter should be -cp flag');
@@ -61,11 +65,12 @@ describe('PmdCatalogWrapper', () => {
 				before(() => {
 					Sinon.createSandbox();
 					// Spoof a config that claims that only Apex's default PMD JAR is enabled.
-					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves(['apex']);
+					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves([LANGUAGE.APEX]);
 					// Spoof a CustomPathManager that claims that a custom JAR exists for plsql, using a weird alias for that language.
 					const customJars: Map<string, Set<string>> = new Map();
 					customJars.set('Pl/SqL', new Set([irrelevantPath]));
 					Sinon.stub(CustomRulePathManager.prototype, 'getRulePathEntries').withArgs(PmdEngine.NAME).resolves(customJars);
+					Sinon.stub(FileHandler.prototype, 'exists').resolves(true);
 				});
 
 				after(() => {
@@ -78,7 +83,7 @@ describe('PmdCatalogWrapper', () => {
 					const params = (await target.buildCommandArray())[1];
 
 					// Expect there to be 6 parameters.
-					expect(params.length).to.equal(6, 'Should have been 6 parameters');
+					expect(params.length).to.equal(6, `Should have been 6 parameters: ${params}`);
 					// Expect the first two parameters to be the catalog and -cp flag.
 					expect(params[0]).to.equal('-DcatalogName=PmdCatalog.json', 'First parameter should be catalog override');
 					expect(params[1]).to.equal('-cp', 'Second parameter should be -cp flag');
@@ -97,11 +102,12 @@ describe('PmdCatalogWrapper', () => {
 				before(() => {
 					Sinon.createSandbox();
 					// Spoof a config that claims that only apex is the supported language
-					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves(['apex']);
+					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves([LANGUAGE.APEX]);
 					const customJars: Map<string, Set<string>> = new Map();
 					customJars.set('pl/sql', new Set([irrelevantPath]));
-					customJars.set('java', new Set());
+					customJars.set(LANGUAGE.JAVA, new Set());
 					Sinon.stub(CustomRulePathManager.prototype, 'getRulePathEntries').withArgs(PmdEngine.NAME).resolves(customJars);
+					Sinon.stub(FileHandler.prototype, 'exists').resolves(true);
 				});
 
 				after(() => {
@@ -126,6 +132,49 @@ describe('PmdCatalogWrapper', () => {
 					expect(params[5]).to.match(/^apex=.*jar$/, 'Sixth parameter should be apex-specific');
 				});
 			});
+
+			describe('Missing Rule Files are Handled Gracefully', () => {
+				const validJar = 'jar-that-exists.jar';
+				const missingJar = 'jar-that-is-missing.jar';
+				// This jar is automatically included by the PmdCatalogWrapper
+				const pmdJar = path.resolve(path.join('dist', 'pmd', 'lib', 'pmd-java-6.22.0.jar'));
+				let uxSpy = null;
+
+				before(() => {
+					Sinon.createSandbox();
+					Sinon.stub(Config.prototype, 'getSupportedLanguages').withArgs(ENGINE.PMD).resolves([LANGUAGE.JAVA]);
+					const customJars: Map<string, Set<string>> = new Map();
+					// Simulate CustomPaths.json contains a jar that has been deleted or moved
+					customJars.set(LANGUAGE.JAVA, new Set([validJar, missingJar]));
+					Sinon.stub(CustomRulePathManager.prototype, 'getRulePathEntries').withArgs(PmdEngine.NAME).resolves(customJars);
+					const stub = Sinon.stub(FileHandler.prototype, 'exists');
+					stub.withArgs(validJar).resolves(true);
+					stub.withArgs(missingJar).resolves(false);
+					uxSpy = Sinon.spy(uxEvents, 'emit');
+				});
+
+				after(() => {
+					Sinon.restore();
+				});
+
+				it('Missing file should be filtered out and display warning', async () => {
+					const target = await TestablePmdCatalogWrapper.create({});
+					const entries = await target.getRulePathEntries();
+
+					// The rule path entries should only include the jar that exists
+					expect(entries.size).to.equal(1, `Entries: ${Array.from(entries.keys())}`);
+					const jars = entries.get(LANGUAGE.JAVA);
+					const jarsErrorMessage =  `Jars: ${Array.from(jars)}`;
+					expect(jars.size).to.equal(2, jarsErrorMessage);
+					expect(jars).to.contain(validJar, jarsErrorMessage);
+					expect(jars).to.contain(pmdJar, jarsErrorMessage);
+					expect(jars).to.not.contain(missingJar, jarsErrorMessage);
+
+					// A warning should be displayed
+					Sinon.assert.calledOnce(uxSpy);
+					Sinon.assert.calledWith(uxSpy, 'warning-always', `Custom rule file path [${missingJar}] for language [${LANGUAGE.JAVA}] was not found.`);
+				});
+			});
 		});
 	});
 });