Merge pull request #1176 from forcedotcom/d/W-14096244

FIX (CodeAnalyzer): @W-14096244@: Restored behavior for violationless output on console.

messages/RunOutputProcessor.js

@@ -1,6 +1,7 @@
 module.exports = {
 	"output": {
 		"engineSummaryTemplate": "Executed %s, found %s violation(s) across %s file(s).",
+		"noViolationsDetected": "Executed engines: %s. No rule violations found.",
 		"sevThresholdSummary": "Rule violations of severity %s or more severe were detected.",
 		"writtenToConsole": "Rule violations were logged to the console.",
 		"writtenToOutFile": "Rule violations were written to %s."

src/lib/util/RunOutputProcessor.ts

@@ -28,9 +28,25 @@ export class RunOutputProcessor {
 
 
 	public processRunOutput(rrr: RecombinedRuleResults): AnyJson {
-		const {minSev, results} = rrr;
+		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;
+		}
 
-		// With the list of violations, 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 (blank/empty if no violations).
 		msgComponents = [...msgComponents, ...this.buildRunSummaryMsgParts(rrr)];

test/commands/scanner/run.severity.test.ts

@@ -20,9 +20,7 @@ describe('scanner:run', function () {
 					'--severity-threshold', '3'
 				])
 				.it('When no violations are found, no error is thrown', ctx => {
-					const output = JSON.parse(ctx.stdout);
-					// There should be no violations (nor any exceptions).
-					expect(output.length).to.equal(0, 'Should be no violations from one engine');
+					expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 					expect(ctx.stderr).to.not.contain(processorMessages.getMessage('output.sevThresholdSummary', ['3']), 'Error should not be present');
 				});
 

test/commands/scanner/run.test.ts

@@ -63,8 +63,8 @@ describe('scanner:run', function () {
 						'--ruleset', 'ApexUnit',
 						'--format', 'xml'
 					])
-					.it('When the file contains no violations, XML with no violations is logged to the console', ctx => {
-						validateNoViolationsXmlOutput(ctx.stdout);
+					.it('When the file contains no violations, a message is logged to the console', ctx => {
+						expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 					});
 			});
 
@@ -180,7 +180,7 @@ describe('scanner:run', function () {
 							fs.unlinkSync('testout.xml');
 						}
 					})
-					.it('The violations are written to the file as an XML', ctx => {
+					.it('Violations are written to file as XML', ctx => {
 						// Verify that the file we wanted was actually created.
 						expect(fs.existsSync('testout.xml')).to.equal(true, 'The command should have created the expected output file');
 						const fileContents = fs.readFileSync('testout.xml').toString();
@@ -199,7 +199,7 @@ describe('scanner:run', function () {
 							fs.unlinkSync('testout.xml');
 						}
 					})
-					.it('An XML file with no violations is created', ctx => {
+					.it('Absence of violations yields empty XML file', ctx => {
 						// Verify that an empty XML file was actually created.
 						expect(fs.existsSync('testout.xml')).to.equal(true, 'The command should have created an empty output file');
 						const fileContents = fs.readFileSync('testout.xml').toString();
@@ -289,8 +289,8 @@ describe('scanner:run', function () {
 					'--ruleset', 'ApexUnit',
 					'--format', 'csv'
 				])
-				.it('When no violations are detected, empty CSV is printed to the console', ctx => {
-					validateNoViolationsCsvOutput(ctx.stdout, false);
+				.it('When no violations are detected, a message is logged to the console', ctx => {
+					expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 				});
 
 			setupCommandTest
@@ -379,8 +379,8 @@ describe('scanner:run', function () {
 					'--ruleset', 'ApexUnit',
 					'--format', 'html'
 				])
-				.it('When no violations are detected, HTML with no violations is logged to the console', ctx => {
-					validateNoViolationsHtmlOutput(ctx.stdout);
+				.it('When no violations are detected, a message is logged to the console', ctx => {
+					expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 				});
 
 			setupCommandTest
@@ -462,8 +462,8 @@ describe('scanner:run', function () {
 					'--ruleset', 'ApexUnit',
 					'--format', 'json'
 				])
-				.it('When no violations are detected, JSON with no violations is logged to the console', ctx => {
-					validateNoViolationsJsonOutput(ctx.stdout);
+				.it('When no violations are detected, a message is logged to the console', ctx => {
+					expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 				});
 
 			setupCommandTest
@@ -604,7 +604,7 @@ describe('scanner:run', function () {
 				.it('--json flag wraps message about no violations occuring', ctx => {
 					const output = JSON.parse(ctx.stdout);
 					expect(output.status).to.equal(0, 'Should have finished properly');
-					expect(output.result.length).to.equal(0, 'When no violations are present, JSON result should be empty array.')
+					expect(output.result).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js']));
 				});
 		});
 
@@ -730,13 +730,8 @@ describe('scanner:run', function () {
 				'--format', 'csv'
 			])
 			.it('The baseConfig enables the usage of default Javascript Types', ctx => {
-				// 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");
+				// There should be no violations.
+				expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint, retire-js']));
 			});
 
 		// TODO: THIS TEST WAS IMPLEMENTED FOR W-7791882. THE FIX FOR THAT BUG WAS SUB-OPTIMAL, AND WE NEED TO CHANGE IT IN 3.0.
@@ -763,13 +758,7 @@ describe('scanner:run', function () {
 				'--env', '{"qunit": true}'
 			])
 			.it('Providing qunit in the --env override should resolve errors about that framework', ctx => {
-				// 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");
+				expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint, retire-js']));
 			});
 	});
 

test/lib/util/RunOutputProcessor.test.ts

@@ -62,16 +62,6 @@ const FAKE_TABLE_OUTPUT = {
 	]
 };
 
-const EMPTY_TABLE_OUTPUT = {
-	"columns": [
-		"Location",
-		"Description",
-		"Category",
-		"URL"
-	],
-	"rows": []
-};
-
 const FAKE_CSV_OUTPUT = `"Problem","File","Severity","Line","Column","Rule","Description","URL","Category","Engine"
 "1","/Users/jfeingold/ts-sample-project/src/file-with-problems.ts","2","3","7","no-unused-vars","'UNUSED' is assigned a value but never used.","https://eslint.org/docs/rules/no-unused-vars","Variables","eslint-typescript"
 "2","/Users/jfeingold/ts-sample-project/src/file-with-problems.ts","2","3","7","@typescript-eslint/no-unused-vars","'UNUSED' is assigned a value but never used.","https://github.com/typescript-eslint/typescript-eslint/blob/v2.33.0/packages/eslint-plugin/docs/rules/no-unused-vars.md","Variables","eslint-typescript"
@@ -146,18 +136,19 @@ describe('RunOutputProcessor', () => {
 				};
 				const rop = new RunOutputProcessor(opts, testUx);
 				const summaryMap: Map<string, EngineExecutionSummary> = new Map();
-				const fakeTableResults: RecombinedRuleResults = {minSev: 1, results: EMPTY_TABLE_OUTPUT, summaryMap: summaryMap}
+				summaryMap.set('pmd', {fileCount: 0, violationCount: 0});
+				summaryMap.set('eslint', {fileCount: 0, violationCount: 0});
+				const fakeRes: RecombinedRuleResults = {minSev: 0, summaryMap, results: ''};
 
 				// THIS IS THE PART BEING TESTED.
-				const output: AnyJson = rop.processRunOutput(fakeTableResults);
+				const output: AnyJson = rop.processRunOutput(fakeRes);
 
-				const expectedTableSummary = `${processorMessages.getMessage('output.writtenToConsole')}`;
-				// We expect that an empty table should be logged to the console and that the output should be an empty array
+				// We expect that the message logged to the console and the message returned should both be the default
+				const expectedMsg = processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint']);
 				Sinon.assert.callCount(logSpy, 1);
-				Sinon.assert.callCount(tableSpy, 1);
-				Sinon.assert.calledWith(logSpy, expectedTableSummary);
-				// TODO is there a better way to check for empty array? output is an empty array, but typed as AnyJson.
-				expect(output.toString()).to.equal('', 'Should have returned empty results');
+				Sinon.assert.callCount(tableSpy, 0);
+				Sinon.assert.calledWith(logSpy, expectedMsg);
+				expect(output).to.equal(expectedMsg, 'Should have returned expected message');
 			});
 
 			describe('Test Case: Table', () => {