@W-11494080@ Fix survey request formatting issue

messages/common.js

@@ -1,3 +1,3 @@
 module.exports = {
-	FEEDBACK_SURVEY_BANNER: `We're constantly improving Salesforce Code Analyzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA.`
-};
+	surveyRequestMessage: `We're constantly improving Salesforce Code Analyzer. Tell us what you think! Give feedback at https://research.net/r/SalesforceCA.`
+};

src/commands/scanner/rule/add.ts

@@ -1,10 +1,11 @@
-import {flags, SfdxCommand} from '@salesforce/command';
+import {flags} from '@salesforce/command';
 import {Messages, SfdxError} from '@salesforce/core';
 import {AnyJson} from '@salesforce/ts-types';
 import {Controller} from '../../../Controller';
 import {stringArrayTypeGuard} from '../../../lib/util/Utils';
 import path = require('path');
 import untildify = require('untildify');
+import { ScannerCommand } from '../../../lib/ScannerCommand';
 
 
 // Initialize Messages with the current plugin directory
@@ -13,9 +14,8 @@ Messages.importMessagesDirectory(__dirname);
 // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core,
 // or any library that is using the messages framework can also be loaded this way.
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'add');
-const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
 
-export default class Add extends SfdxCommand {
+export default class Add extends ScannerCommand {
 
 	public static description = messages.getMessage('commandDescription');
 	public static longDescription = messages.getMessage('commandDescriptionLong');
@@ -39,9 +39,8 @@ export default class Add extends SfdxCommand {
 		})
 	};
 
-	public async run(): Promise<AnyJson> {
+	async runInternal(): Promise<AnyJson> {
 		this.validateFlags();
-		this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER'));
 
 		const language = this.flags.language as string;
 		const paths = this.resolvePaths();

src/commands/scanner/rule/describe.ts

@@ -12,7 +12,6 @@ Messages.importMessagesDirectory(__dirname);
 // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core,
 // or any library that is using the messages framework can also be loaded this way.
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'describe');
-const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
 
 type DescribeStyledRule = Rule & {
 	enabled: boolean;
@@ -41,8 +40,7 @@ export default class Describe extends ScannerCommand {
 		verbose: flags.builtin()
 	};
 
-	public async run(): Promise<AnyJson> {
-		this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER'));
+	async runInternal(): Promise<AnyJson> {
 		const ruleFilters = this.buildRuleFilters();
 		// It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can
 		// allow to boil over.

src/commands/scanner/rule/list.ts

@@ -13,7 +13,6 @@ Messages.importMessagesDirectory(__dirname);
 // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core,
 // or any library that is using the messages framework can also be loaded this way.
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'list');
-const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
 const columns = [messages.getMessage('columnNames.name'),
 				messages.getMessage('columnNames.languages'),
 				messages.getMessage('columnNames.categories'),
@@ -64,8 +63,7 @@ export default class List extends ScannerCommand {
 		// END: Flags consumed by ScannerCommand#buildRuleFilters
 	};
 
-	public async run(): Promise<Rule[]> {
-		this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER'));
+	async runInternal(): Promise<Rule[]> {
 		const ruleFilters = this.buildRuleFilters();
 		// It's possible for this line to throw an error, but that's fine because the error will be an SfdxError that we can
 		// allow to boil over.

src/commands/scanner/rule/remove.ts

@@ -15,7 +15,6 @@ Messages.importMessagesDirectory(__dirname);
 // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core,
 // or any library that is using the messages framework can also be loaded this way.
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'remove');
-const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
 
 export default class Remove extends ScannerCommand {
 	// These determine what's displayed when the --help/-h flag is supplied.
@@ -44,8 +43,7 @@ export default class Remove extends ScannerCommand {
 		})
 	};
 
-	public async run(): Promise<AnyJson> {
-		this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER'));
+	async runInternal(): Promise<AnyJson> {
 		// Step 1: Validate our input.
 		this.validateFlags();
 

src/lib/ScannerCommand.ts

@@ -2,9 +2,35 @@ import {SfdxCommand} from '@salesforce/command';
 import {CategoryFilter, LanguageFilter, RuleFilter, RulesetFilter, RulenameFilter, EngineFilter} from './RuleFilter';
 import {uxEvents, EVENTS} from './ScannerEvents';
 import {stringArrayTypeGuard} from './util/Utils';
+import {AnyJson} from '@salesforce/ts-types';
+
+import {Messages} from '@salesforce/core';
+
+// Initialize Messages with the current plugin directory
+Messages.importMessagesDirectory(__dirname);
+const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
+
 
 export abstract class ScannerCommand extends SfdxCommand {
 
+	public async run(): Promise<AnyJson> {
+		this.runCommonSteps();
+		return await this.runInternal();
+	}
+
+	/**
+	 * Command's should implement this method to add their
+	 * working steps.
+	 */
+	abstract runInternal(): Promise<AnyJson>;
+
+	/**
+	 * Common steps that should be run before every command
+	 */
+	protected runCommonSteps(): void {
+		this.ux.warn(commonMessages.getMessage('surveyRequestMessage'));
+	}
+
 	protected buildRuleFilters(): RuleFilter[] {
 		const filters: RuleFilter[] = [];
 		// Create a filter for any provided categories.

src/lib/ScannerRunCommand.ts

@@ -14,16 +14,14 @@ Messages.importMessagesDirectory(__dirname);
 // Load the specific messages for this file. Messages from @salesforce/command, @salesforce/core,
 // or any library that is using the messages framework can also be loaded this way.
 const messages = Messages.loadMessages('@salesforce/sfdx-scanner', 'run');
-const commonMessages = Messages.loadMessages('@salesforce/sfdx-scanner', 'common');
 // This code is used for internal errors.
 export const INTERNAL_ERROR_CODE = 1;
 
 export abstract class ScannerRunCommand extends ScannerCommand {
 
-	public async run(): Promise<AnyJson> {
+	async runInternal(): Promise<AnyJson> {
 		// First, do any validations that can't be handled with out-of-the-box stuff.
 		await this.validateFlags();
-		this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER'));
 
 		// If severity-threshold is used, that implicitly normalizes the severity.
 		const normalizeSeverity: boolean = (this.flags['normalize-severity'] || this.flags['severity-threshold']) as boolean;

test/TestUtils.ts

@@ -1,7 +1,6 @@
 import fs = require('fs');
 import path = require('path');
-import { test, expect } from '@salesforce/command/lib/test';
-import * as CommonMessages from '../messages/common.js';
+import { test } from '@salesforce/command/lib/test';
 import * as TestOverrides from './test-related-lib/TestOverrides';
 import Sinon = require('sinon');
 import LocalCatalog from '../src/lib/services/LocalCatalog';
@@ -16,12 +15,6 @@ export function prettyPrint(obj: any): string {
 	return JSON.stringify(obj, null, 2);
 }
 
-export function stripExtraneousOutput(stdout: string): string {
-	const splitOutput = stdout.split('\n');
-	expect(splitOutput[0]).to.equal('=== ' + CommonMessages.FEEDBACK_SURVEY_BANNER);
-	return splitOutput.slice(1).join('\n');
-}
-
 export function stubCatalogFixture(): void {
 	// Make sure all catalogs exist where they're supposed to.
 	if (!fs.existsSync(CATALOG_FIXTURE_PATH)) {

test/commands/scanner/rule/describe.test.ts

@@ -20,8 +20,8 @@ describe('scanner:rule:describe', () => {
 				.it('--json flag yields correct results', ctx => {
 					const ctxJson = JSON.parse(ctx.stdout);
 					expect(ctxJson.result.length).to.equal(0, 'Should be no results');
-					expect(ctxJson.warnings.length).to.equal(1, 'Should be one warning');
-					expect(ctxJson.warnings[0]).to.equal(formattedWarning, 'Warning message should match');
+					expect(ctxJson.warnings.length).to.equal(2, 'Incorrect warning count');
+					expect(ctxJson.warnings[1]).to.equal(formattedWarning, 'Warning message should match');
 				});
 		});
 

test/commands/scanner/rule/list.test.ts

@@ -1,5 +1,5 @@
 import {expect} from '@salesforce/command/lib/test';
-import {setupCommandTest, stripExtraneousOutput} from '../../../TestUtils';
+import {setupCommandTest} from '../../../TestUtils';
 import {Rule} from '../../../../src/types';
 import {CATALOG_FILE, ENGINE} from '../../../../src/Constants';
 import fs = require('fs');
@@ -56,8 +56,7 @@ describe('scanner:rule:list', () => {
 
 					// Split the output table by newline and throw out the first two rows, since they just contain header information. That
 					// should leave us with the actual data.
-					const output = stripExtraneousOutput(ctx.stdout);
-					const rows = output.trim().split('\n');
+					const rows = ctx.stdout.trim().split('\n');
 					rows.shift();
 					rows.shift();
 					expect(rows).to.have.lengthOf(totalRuleCount, 'All rules should have been returned');
@@ -307,8 +306,7 @@ describe('scanner:rule:list', () => {
 				.command(['scanner:rule:list', '--category', 'Beebleborp'])
 				.it('Without --json flag, an empty table is printed', ctx => {
 					// Split the result by newline, and make sure there are two rows.
-					const output = stripExtraneousOutput(ctx.stdout);
-					const rows = output.trim().split('\n');
+					const rows = ctx.stdout.trim().split('\n');
 					expect(rows).to.have.lengthOf(2, 'Only the header rows should have been printed');
 				});
 

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

@@ -1,5 +1,5 @@
 import {expect} from '@salesforce/command/lib/test';
-import {setupCommandTest, stripExtraneousOutput} from '../../TestUtils';
+import {setupCommandTest} from '../../TestUtils';
 import {Messages} from '@salesforce/core';
 import path = require('path');
 
@@ -31,12 +31,12 @@ describe('scanner:run', function () {
 					'--severity-threshold', '1'
 				])
 				.it('When no violations are found equal to or greater than flag value, no error is thrown', ctx => {
-					const output = stripExtraneousOutput(ctx.stdout);
-                    const outputJson = JSON.parse(output);
+
+                    const output = JSON.parse(ctx.stdout);
                     // check that test file still has severities of 3
-                    for (let i=0; i<outputJson.length; i++) {
-                        for (let j=0; j<outputJson[i].violations.length; j++) {
-                            expect(outputJson[i].violations[j].normalizedSeverity).to.equal(3);
+                    for (let i=0; i<output.length; i++) {
+                        for (let j=0; j<output[i].violations.length; j++) {
+                            expect(output[i].violations[j].normalizedSeverity).to.equal(3);
                         }
                     }
 
@@ -51,12 +51,12 @@ describe('scanner:run', function () {
 					'--severity-threshold', '3'
 				])
 				.it('When violations are found equal to or greater than flag value, an error is thrown', ctx => {
-					const output = stripExtraneousOutput(ctx.stdout);
-					const outputJson = JSON.parse(output);
+
+                    const output = JSON.parse(ctx.stdout);
                     // check that test file still has severities of 3
-                    for (let i=0; i<outputJson.length; i++) {
-                        for (let j=0; j<outputJson[i].violations.length; j++) {
-                            expect(outputJson[i].violations[j].normalizedSeverity).to.equal(3);
+                    for (let i=0; i<output.length; i++) {
+                        for (let j=0; j<output[i].violations.length; j++) {
+                            expect(output[i].violations[j].normalizedSeverity).to.equal(3);
                         }
                     }
                     expect(ctx.stderr).to.contain(runMessages.getMessage('output.sevThresholdSummary', ['3']));
@@ -94,28 +94,27 @@ describe('scanner:run', function () {
 					'--normalize-severity'
 				])
 				.it('Ensure normalized severity is correct', ctx => {
-					const output = stripExtraneousOutput(ctx.stdout);
-					const outputJson = JSON.parse(output);
-
-					for (let i=0; i<outputJson.length; i++) {
-                        for (let j=0; j<outputJson[i].violations.length; j++) {
-							if (outputJson[i].engine.includes("pmd")){
-								if (outputJson[i].violations[j].severity == 1) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(1);
-								} else if (outputJson[i].violations[j].severity == 2) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(2);
-								} else if (outputJson[i].violations[j].severity == 3) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(3);
-								} else if (outputJson[i].violations[j].severity == 4) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(3);
-								} else if (outputJson[i].violations[j].severity == 5) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(3);
+					const output = JSON.parse(ctx.stdout);
+
+					for (let i=0; i<output.length; i++) {
+                        for (let j=0; j<output[i].violations.length; j++) {
+							if (output[i].engine.includes("pmd")){
+								if (output[i].violations[j].severity == 1) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(1);
+								} else if (output[i].violations[j].severity == 2) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(2);
+								} else if (output[i].violations[j].severity == 3) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(3);
+								} else if (output[i].violations[j].severity == 4) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(3);
+								} else if (output[i].violations[j].severity == 5) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(3);
 								}
-							} else if (outputJson[i].engine.includes("eslint")) {
-								if (outputJson[i].violations[j].severity == 1) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(2);
-								} else if (outputJson[i].violations[j].severity == 2) {
-									expect(outputJson[i].violations[j].normalizedSeverity).to.equal(1);
+							} else if (output[i].engine.includes("eslint")) {
+								if (output[i].violations[j].severity == 1) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(2);
+								} else if (output[i].violations[j].severity == 2) {
+									expect(output[i].violations[j].normalizedSeverity).to.equal(1);
 								}
 							}
 
@@ -129,12 +128,11 @@ describe('scanner:run', function () {
 					'--format', 'json'
 				])
 				.it('Ensure normalized severity is not outputted when --normalize-severity not provided', ctx => {
-					const output = stripExtraneousOutput(ctx.stdout);
-					const outputJson = JSON.parse(output);
+					const output = JSON.parse(ctx.stdout);
 
-					for (let i=0; i<outputJson.length; i++) {
-                        for (let j=0; j<outputJson[i].violations.length; j++) {
-							expect(outputJson[i].violations[j].normalizedSeverity).to.equal(undefined);
+					for (let i=0; i<output.length; i++) {
+                        for (let j=0; j<output[i].violations.length; j++) {
+							expect(output[i].violations[j].normalizedSeverity).to.equal(undefined);
                         }
                     }
 				});

test/commands/scanner/run.test.ts

@@ -1,5 +1,5 @@
 import {expect} from '@salesforce/command/lib/test';
-import {setupCommandTest, stripExtraneousOutput} from '../../TestUtils';
+import {setupCommandTest} from '../../TestUtils';
 import {Messages} from '@salesforce/core';
 import fs = require('fs');
 import path = require('path');
@@ -214,8 +214,8 @@ describe('scanner:run', function () {
 					'--format', 'csv'
 				])
 				.it('Properly writes CSV to console', ctx => {
-					const output = stripExtraneousOutput(ctx.stdout);
-					validateCsvOutput(output, false);
+					// Split the output by newline characters and throw away the first entry, so we're left with just the rows.
+					validateCsvOutput(ctx.stdout, false);
 				});
 
 			setupCommandTest
@@ -640,7 +640,6 @@ describe('scanner:run', function () {
 				.command(['scanner:run', '--target', '**/*.js,**/*.cls', '--format', 'json'])
 				.finally(() => process.chdir("../../../.."))
 				.it('Polyglot project triggers pmd and eslint rules', ctx => {
-					expect(ctx.stderr, ctx.stdout).to.be.empty;
 					const results = JSON.parse(ctx.stdout.substring(ctx.stdout.indexOf("[{"), ctx.stdout.lastIndexOf("}]") + 2));
 					// Look through all of the results and gather a set of unique engines
 					const uniqueEngines = new Set(results.map(r => { return r.engine }));
@@ -694,6 +693,22 @@ describe('scanner:run', function () {
 			});
 	});
 
+	describe('run with format --json', () => {
+		setupCommandTest
+		.command(['scanner:run',
+		'--target', path.join('test', 'code-fixtures', 'apex', 'AnotherTestClass.cls'),
+		'--format', 'json'
+	])
+	.it('provides only json in stdout', ctx => {
+		try {
+			JSON.parse(ctx.stdout);
+		} catch (error) {
+			expect.fail("Invalid JSON output from --format json: " + ctx.stdout, error);
+		}
+
+		});
+	});
+
 	describe('Validation on custom config flags', () => {
 		setupCommandTest
 			.command(['scanner:run',