Merge pull request #1206 from forcedotcom/dev

RELEASE: @W-14198233@: Merging dev to release for v3.17.0

Author: jfeingold35

package.json

@@ -1,7 +1,7 @@
 {
 	"name": "@salesforce/sfdx-scanner",
 	"description": "Static code scanner that applies quality and security rules to Apex code, and provides feedback.",
-	"version": "3.16.0",
+	"version": "3.17.0",
 	"author": "ISV SWAT",
 	"bugs": "https://github.com/forcedotcom/sfdx-scanner/issues",
 	"dependencies": {

retire-js/RetireJsVulns.json

@@ -407,7 +407,7 @@
       },
       {
         "below": "1.19.3",
-        "severity": "medium",
+        "severity": "high",
         "cwe": [
           "CWE-400"
         ],
@@ -2812,7 +2812,7 @@
     "vulnerabilities": [
       {
         "below": "0.5.0",
-        "severity": "high",
+        "severity": "medium",
         "cwe": [
           "CWE-79"
         ],
@@ -3555,7 +3555,7 @@
       },
       {
         "below": "2.0.3",
-        "severity": "high",
+        "severity": "medium",
         "cwe": [
           "CWE-79"
         ],

sfge/src/main/java/com/salesforce/rules/PerformNullCheckOnSoqlVariables.java

@@ -62,6 +62,11 @@ protected boolean isEnabled() {
         return true;
     }
 
+    @Override
+    protected boolean isPilot() {
+        return false;
+    }
+
     /**
      * Tests a vertex using a symbol provider to check if it violates this rule.
      *

src/lib/formatter/RuleResultRecombinator.ts

@@ -110,12 +110,7 @@ export class RuleResultRecombinator {
 	private static constructXml(results: RuleResult[]): string {
 		let resultXml = ``;
 
-		// If the results were just an empty string, we can return it.
-		if (results.length === 0) {
-			return resultXml;
-		}
-
-		const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined)
+		const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined)
 
 		let problemCount = 0;
 
@@ -188,10 +183,6 @@ export class RuleResultRecombinator {
 	}
 
 	private static constructJunit(results: RuleResult[]): string {
-		// If there are no results, we can just return an empty string.
-		if (!results || results.length === 0) {
-			return '';
-		}
 
 		// Otherwise, we'll need to start constructing our JUnit XML. To do that, we'll need a map from file names to
 		// lists of the <failure> tags generated from violations found in the corresponding file.
@@ -279,10 +270,6 @@ URL: ${url}`;
 	}
 
 	private static constructTable(results: RuleResult[]): RecombinedData {
-		// If the results were just an empty string, we can return it.
-		if (results.length === 0) {
-			return '';
-		}
 		const columns = this.violationsAreDfa(results)
 			? ['Source Location', 'Sink Location', 'Description', 'Category', 'URL']
 			: ['Location', 'Description', 'Category', 'URL'];
@@ -341,10 +328,6 @@ URL: ${url}`;
 	}
 
 	private static constructJson(results: RuleResult[], verboseViolations = false): string {
-		if (results.length === 0) {
-			return '';
-		}
-
 		if (verboseViolations) {
 			const resultsVerbose = JSON.parse(JSON.stringify(results)) as RuleResult[];
 			for (const result of resultsVerbose) {
@@ -363,12 +346,7 @@ URL: ${url}`;
 	}
 
 	private static async constructHtml(results: RuleResult[], verboseViolations = false): Promise<string> {
-		// If the results were just an empty string, we can return it.
-		if (results.length === 0) {
-			return '';
-		}
-
-		const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined);
+		const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined);
 		const isDfa = this.violationsAreDfa(results);
 
 
@@ -434,12 +412,8 @@ URL: ${url}`;
 	}
 
 	private static async constructCsv(results: RuleResult[]): Promise<string> {
-		// If the results were just an empty list, we can return an empty string
-		if (results.length === 0) {
-			return '';
-		}
 		const isDfa: boolean = this.violationsAreDfa(results);
-		const normalizeSeverity: boolean = results[0].violations.length > 0 && !(results[0].violations[0].normalizedSeverity === undefined)
+		const normalizeSeverity: boolean = results[0]?.violations.length > 0 && !(results[0]?.violations[0].normalizedSeverity === undefined)
 
 		const csvRows = [];
 		// There will always be columns for the problem counter and the severity.

src/lib/util/Config.ts

@@ -37,7 +37,7 @@ const DEFAULT_CONFIG: ConfigContent = {
 			name: ENGINE.PMD,
 			targetPatterns: [
 				"**/*.cls","**/*.trigger","**/*.java","**/*.page","**/*.component","**/*.xml",
-				"!**/node_modules/**","!**/*-meta.xml"
+				"!**/node_modules/**"
 			],
 			supportedLanguages: ['apex', 'vf'],
 			disabled: false

src/lib/util/RunOutputProcessor.ts

@@ -28,21 +28,27 @@ export class RunOutputProcessor {
 
 
 	public processRunOutput(rrr: RecombinedRuleResults): AnyJson {
-		const {minSev, summaryMap, results} = rrr;
-		// If the results are an empty string, it means no violations were found.
-		if (results === '') {
-			// Build an appropriate message...
+		const {minSev, results, summaryMap} = rrr;
+
+		const hasViolations = [...summaryMap.values()].some(summary => summary.violationCount !== 0);
+
+		// If there are neither violations nor an outfile, then we want to avoid writing empty
+		// results to the console.
+		// NOTE: If there's an outfile, we skip this part. This is because we still want to generate
+		//       an empty outfile
+		if (!this.opts.outfile && !hasViolations) {
+			// Build a message indicating which engines were run...
 			const msg = messages.getMessage('output.noViolationsDetected', [[...summaryMap.keys()].join(', ')]);
 			// ...log it to the console...
 			this.ux.log(msg);
 			// ...and return it for use with the --json flag.
 			return msg;
 		}
 
-		// If we actually have violations, there's some stuff we need to do with them. We'll build an array of message parts,
-		// and then log them all at the end.
+		// If we have violations (or an outfile but no violations), we'll build an array of
+		// message parts, and then log them all at the end.
 		let msgComponents: string[] = [];
-		// We need a summary of the information we were provided.
+		// We need a summary of the information we were provided (blank/empty if no violations).
 		msgComponents = [...msgComponents, ...this.buildRunSummaryMsgParts(rrr)];
 		// We need to surface the results directly to the user, then add a message describing what we did.
 		msgComponents.push(this.opts.outfile ? this.writeToOutfile(results) : this.writeToConsole(results));

src/lib/util/VersionUpgradeManager.ts

@@ -38,7 +38,7 @@ upgradeScriptsByVersion.set('v2.7.0', (config: ConfigContent): Promise<void> =>
 	return Promise.resolve();
 });
 upgradeScriptsByVersion.set('v3.0.0', (config: ConfigContent): Promise<void> => {
-	// In v3.0.0, we're changing RetireJS from a supplemental engine that must be manually enabled to an enabled-byu-default
+	// In v3.0.0, we're changing RetireJS from a supplemental engine that must be manually enabled to an enabled-by-default
 	// engine. So we need to change its `disabled` config value from true to false.
 	const retireJsConfig: EngineConfigContent = config.engines.find(e => e.name === ENGINE.RETIRE_JS);
 	if (retireJsConfig) {
@@ -63,6 +63,16 @@ upgradeScriptsByVersion.set('v3.6.0', async (config: ConfigContent): Promise<voi
 		}
 	}
 });
+upgradeScriptsByVersion.set('v3.17.0', (config: ConfigContent): Promise<void> => {
+	// In v3.17.0, we're changing PMD's config so that it no longer excludes Salesforce metadata
+	// files by default. This will automatically apply to any newly-generated configs, but we also
+	// want to retroactively remove this exclusion for existing users.
+	const pmdConfig: EngineConfigContent = config.engines.find(e => e.name === ENGINE.PMD);
+	if (pmdConfig) {
+		pmdConfig.targetPatterns = pmdConfig.targetPatterns.filter(s => s !== '!**/*-meta.xml');
+	}
+	return Promise.resolve();
+});
 
 
 // ================ CLASSES =====================

test/commands/scanner/run.filters.test.ts

@@ -19,7 +19,13 @@ describe('scanner:run tests that result in the use of RuleFilters', function ()
 				'--engine', 'eslint-lwc'
 			])
 			.it('LWC Engine Successfully parses LWC code', ctx => {
-				expect(ctx.stdout).to.contain('No rule violations found.');
+				// If there's a summary, then it'll be separated from the CSV by an empty line. Throw it away.
+				const [csv, _] = ctx.stdout.trim().split(/\n\r?\n/);
+
+				// Confirm there are no violations.
+				// Since it's a CSV, the rows themselves are separated by newline characters.
+				// The header should not have any newline characters after it. There should be no violation rows.
+				expect(csv.indexOf('\n')).to.equal(-1, "Should be no violations detected");
 			});
 
 		setupCommandTest