diff --git a/messages/common.js b/messages/common.js index be555a676..810df19b0 100644 --- a/messages/common.js +++ b/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.` +}; \ No newline at end of file diff --git a/src/commands/scanner/rule/add.ts b/src/commands/scanner/rule/add.ts index b4e34515b..65ddee5e6 100644 --- a/src/commands/scanner/rule/add.ts +++ b/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 { + async runInternal(): Promise { this.validateFlags(); - this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); const language = this.flags.language as string; const paths = this.resolvePaths(); diff --git a/src/commands/scanner/rule/describe.ts b/src/commands/scanner/rule/describe.ts index c5dd74f6a..9dffe7fa0 100644 --- a/src/commands/scanner/rule/describe.ts +++ b/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 { - this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); + async runInternal(): Promise { 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. diff --git a/src/commands/scanner/rule/list.ts b/src/commands/scanner/rule/list.ts index ba5406632..77ae897a9 100644 --- a/src/commands/scanner/rule/list.ts +++ b/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 { - this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); + async runInternal(): Promise { 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. diff --git a/src/commands/scanner/rule/remove.ts b/src/commands/scanner/rule/remove.ts index f121782de..26bbe40a9 100644 --- a/src/commands/scanner/rule/remove.ts +++ b/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 { - this.ux.styledHeader(commonMessages.getMessage('FEEDBACK_SURVEY_BANNER')); + async runInternal(): Promise { // Step 1: Validate our input. this.validateFlags(); diff --git a/src/lib/ScannerCommand.ts b/src/lib/ScannerCommand.ts index 664373831..1697ca0c0 100644 --- a/src/lib/ScannerCommand.ts +++ b/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 { + this.runCommonSteps(); + return await this.runInternal(); + } + + /** + * Command's should implement this method to add their + * working steps. + */ + abstract runInternal(): Promise; + + /** + * 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. diff --git a/src/lib/ScannerRunCommand.ts b/src/lib/ScannerRunCommand.ts index 6cc092fee..40586d20f 100644 --- a/src/lib/ScannerRunCommand.ts +++ b/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 { + async runInternal(): Promise { // 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; diff --git a/test/TestUtils.ts b/test/TestUtils.ts index 06da598c4..b50b3770d 100644 --- a/test/TestUtils.ts +++ b/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)) { diff --git a/test/commands/scanner/rule/describe.test.ts b/test/commands/scanner/rule/describe.test.ts index fc70f9d91..6e992dc3a 100644 --- a/test/commands/scanner/rule/describe.test.ts +++ b/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'); }); }); diff --git a/test/commands/scanner/rule/list.test.ts b/test/commands/scanner/rule/list.test.ts index 973782dfd..3bfc3ef50 100644 --- a/test/commands/scanner/rule/list.test.ts +++ b/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'); }); diff --git a/test/commands/scanner/run.severity.test.ts b/test/commands/scanner/run.severity.test.ts index 4e411acff..6c9384dc4 100644 --- a/test/commands/scanner/run.severity.test.ts +++ b/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 { - 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 { - const output = stripExtraneousOutput(ctx.stdout); - const outputJson = JSON.parse(output); - - for (let i=0; i { - const output = stripExtraneousOutput(ctx.stdout); - const outputJson = JSON.parse(output); + const output = JSON.parse(ctx.stdout); - for (let i=0; i { - 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',