diff --git a/package.json b/package.json index 668eb0943..59e7c0966 100644 --- a/package.json +++ b/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": { diff --git a/retire-js/RetireJsVulns.json b/retire-js/RetireJsVulns.json index 5b5e1f694..498bcd85b 100644 --- a/retire-js/RetireJsVulns.json +++ b/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" ], diff --git a/sfge/src/main/java/com/salesforce/rules/PerformNullCheckOnSoqlVariables.java b/sfge/src/main/java/com/salesforce/rules/PerformNullCheckOnSoqlVariables.java index 2944f090f..dca0eaf3c 100644 --- a/sfge/src/main/java/com/salesforce/rules/PerformNullCheckOnSoqlVariables.java +++ b/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. * diff --git a/src/lib/formatter/RuleResultRecombinator.ts b/src/lib/formatter/RuleResultRecombinator.ts index cc02eaa20..4975a71e2 100644 --- a/src/lib/formatter/RuleResultRecombinator.ts +++ b/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 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 { - // 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 { - // 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. diff --git a/src/lib/util/Config.ts b/src/lib/util/Config.ts index 9facac3f6..df78fbf5b 100644 --- a/src/lib/util/Config.ts +++ b/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 diff --git a/src/lib/util/RunOutputProcessor.ts b/src/lib/util/RunOutputProcessor.ts index 13f76214c..c7b76099a 100644 --- a/src/lib/util/RunOutputProcessor.ts +++ b/src/lib/util/RunOutputProcessor.ts @@ -28,10 +28,16 @@ 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); @@ -39,10 +45,10 @@ export class RunOutputProcessor { 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)); diff --git a/src/lib/util/VersionUpgradeManager.ts b/src/lib/util/VersionUpgradeManager.ts index b7ad764b0..3305c510a 100644 --- a/src/lib/util/VersionUpgradeManager.ts +++ b/src/lib/util/VersionUpgradeManager.ts @@ -38,7 +38,7 @@ upgradeScriptsByVersion.set('v2.7.0', (config: ConfigContent): Promise => return Promise.resolve(); }); upgradeScriptsByVersion.set('v3.0.0', (config: ConfigContent): Promise => { - // 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 => { + // 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 ===================== diff --git a/test/commands/scanner/run.filters.test.ts b/test/commands/scanner/run.filters.test.ts index 8dc6945c1..c11424c74 100644 --- a/test/commands/scanner/run.filters.test.ts +++ b/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 diff --git a/test/commands/scanner/run.test.ts b/test/commands/scanner/run.test.ts index f27a51e2d..0239ba4b3 100644 --- a/test/commands/scanner/run.test.ts +++ b/test/commands/scanner/run.test.ts @@ -30,6 +30,12 @@ describe('scanner:run', function () { expect(violations[1]).to.match(/line="19".+rule="ApexUnitTestClassShouldHaveAsserts"/); } + function validateNoViolationsXmlOutput(xml: string): void { + // We'll split the output by the tag, so we can get individual violations. + // we expect no violations + expect(xml.indexOf(' { setupCommandTest .command(['scanner:run', @@ -174,12 +180,31 @@ 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(); validateXmlOutput(fileContents); }); + + setupCommandTest + .command(['scanner:run', + '--target', path.join('test', 'code-fixtures', 'apex', 'YetAnotherTestClass.cls'), + '--ruleset', 'ApexUnit', + '--outfile', 'testout.xml' + ]) + .finally(ctx => { + // Regardless of what happens in the test itself, we need to delete the file we created. + if (fs.existsSync('testout.xml')) { + fs.unlinkSync('testout.xml'); + } + }) + .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(); + validateNoViolationsXmlOutput(fileContents); + }); }); }); @@ -192,7 +217,7 @@ describe('scanner:run', function () { expect(summary).to.not.equal(null, 'Expected summary to be not null'); expect(summary).to.contain(processorMessages.getMessage('output.engineSummaryTemplate', ['pmd', 2, 1]), 'Summary should be correct'); } - // Since it's a CSV, the rows themselves are separated by newline chaacters, and there's a header row we + // Since it's a CSV, the rows themselves are separated by newline characters, and there's a header row we // need to discard. const rows = csv.trim().split('\n'); rows.shift(); @@ -209,6 +234,21 @@ describe('scanner:run', function () { expect(data[1][5]).to.equal('"ApexUnitTestClassShouldHaveAsserts"', 'Violation #2 should be of the expected type'); } + function validateNoViolationsCsvOutput(contents: string, expectSummary=true): void { + // If there's a summary, then it'll be separated from the CSV by an empty line. + const [csv, summary] = contents.trim().split(/\n\r?\n/); + if (expectSummary) { + expect(summary).to.not.equal(undefined, 'Expected summary to be not undefined'); + expect(summary).to.not.equal(null, 'Expected summary to be not null'); + expect(summary).to.contain(processorMessages.getMessage('output.engineSummaryTemplate', ['pmd', 2, 1]), 'Summary should be correct'); + } + + // Since it's a CSV, the rows themselves are separated by newline characters. + // Test to check there are no violations. + // There should be a header and no other lines, meaning no newline characters. + expect(csv.indexOf('\n')).to.equal(-1, "Should be no violations detected"); + } + setupCommandTest .command(['scanner:run', '--target', path.join('test', 'code-fixtures', 'apex', 'SomeTestClass.cls'), @@ -236,7 +276,6 @@ describe('scanner:run', function () { // Verify that the correct message is displayed to user expect(ctx.stdout).to.contain(processorMessages.getMessage('output.engineSummaryTemplate', ['pmd', 2, 1]), 'Expected summary to be correct'); expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.csv'])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.noViolationsDetected', [])); // Verify that the file we wanted was actually created. expect(fs.existsSync('testout.csv')).to.equal(true, 'The command should have created the expected output file'); @@ -266,10 +305,12 @@ describe('scanner:run', function () { fs.unlinkSync('testout.csv'); } }) - .it('When --oufile is provided and no violations are detected, output file should not be created', ctx => { - expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.csv'])); - expect(fs.existsSync('testout.csv')).to.be.false; + .it('When --outfile is provided and no violations are detected, CSV file with no violations is created', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.csv'])); + + const fileContents = fs.readFileSync('testout.csv').toString(); + expect(fs.existsSync('testout.csv')).to.be.true; + validateNoViolationsCsvOutput(fileContents, false); }); }); @@ -290,6 +331,15 @@ describe('scanner:run', function () { expect(rows[1]['ruleName']).to.equal('ApexUnitTestClassShouldHaveAsserts', 'Violation #2 should be of the expected type'); } + function validateNoViolationsHtmlOutput(html: string): void { + // there should be no instance of a filled violations object + const result = html.match(/const violations = (\[.+\]);/); + expect(result).to.be.null; + // there should be an empty violations object + const emptyResult = html.match(/const violations = \[\];/); + expect(emptyResult).to.be.not.null; + } + setupCommandTest .command(['scanner:run', '--target', path.join('test', 'code-fixtures', 'apex', 'SomeTestClass.cls'), @@ -297,7 +347,7 @@ describe('scanner:run', function () { '--format', 'html' ]) .it('Properly writes HTML to console', ctx => { - // Parse out the JSON results + // Parse out the HTML results validateHtmlOutput(ctx.stdout); }); @@ -316,7 +366,6 @@ describe('scanner:run', function () { .it('Properly writes HTML to file', ctx => { // Verify that the correct message is displayed to user expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', [outputFile])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.noViolationsDetected', [])); // Verify that the file we wanted was actually created. expect(fs.existsSync(outputFile)).to.equal(true, 'The command should have created the expected output file'); @@ -346,10 +395,11 @@ describe('scanner:run', function () { fs.unlinkSync(outputFile); } }) - .it('When --oufile is provided and no violations are detected, output file should not be created', ctx => { - expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.writtenToOutFile', [outputFile])); - expect(fs.existsSync(outputFile)).to.be.false; + .it('When --outfile is provided and no violations are detected, HTML file with no violations should be created', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', [outputFile])); + expect(fs.existsSync(outputFile)).to.be.true; + const fileContents = fs.readFileSync(outputFile).toString(); + validateNoViolationsHtmlOutput(fileContents); }); }); @@ -365,6 +415,13 @@ describe('scanner:run', function () { expect(output[0].violations[1].line).to.equal(19, 'Violation #2 should occur on the expected line'); } + function validateNoViolationsJsonOutput(json: string): void { + const output = JSON.parse(json); + // There should be no violations. + expect(output.length).to.equal(0, 'Should be no violations from one engine'); + } + + setupCommandTest .command(['scanner:run', '--target', path.join('test', 'code-fixtures', 'apex', 'SomeTestClass.cls'), @@ -392,7 +449,6 @@ describe('scanner:run', function () { // Verify that the correct message is displayed to user expect(ctx.stdout).to.contain(processorMessages.getMessage('output.engineSummaryTemplate', ['pmd', 2, 1]), 'Expected summary to be correct'); expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.json'])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.noViolationsDetected', [])); // Verify that the file we wanted was actually created. expect(fs.existsSync('testout.json')).to.equal(true, 'The command should have created the expected output file'); @@ -422,10 +478,11 @@ describe('scanner:run', function () { fs.unlinkSync('testout.json'); } }) - .it('When --oufile is provided and no violations are detected, output file should not be created', ctx => { - expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, retire-js'])); - expect(ctx.stdout).to.not.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.json'])); - expect(fs.existsSync('testout.json')).to.be.false; + .it('When --outfile is provided and no violations are detected, a JSON file with no violations is created', ctx => { + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.json'])); + expect(fs.existsSync('testout.json')).to.be.true; + const fileContents = fs.readFileSync('testout.json').toString(); + validateNoViolationsJsonOutput(fileContents); }); }); @@ -454,8 +511,13 @@ describe('scanner:run', function () { '--ruleset', 'ApexUnit', '--format', 'table' ]) - .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'])); + .it('When no violations are detected, an empty table is logged to the console', ctx => { + // Split the output by newline characters and throw away the first two rows, which are the column names and a separator. + // That will leave us with just the rows. + const rows = ctx.stdout.trim().split('\n'); + + // Expect to find no violations listing this class. + expect(rows.find(r => r.indexOf("SomeTestClass.cls") > 0)).to.equal(undefined, "more rows??"); }); }); @@ -519,7 +581,6 @@ describe('scanner:run', function () { expect(output.status).to.equal(0, 'Should finish properly'); const result = output.result; expect(result).to.contain(processorMessages.getMessage('output.writtenToOutFile', ['testout.xml'])); - expect(result).to.not.contain(processorMessages.getMessage('output.noViolationsDetected', [])); // 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(); @@ -629,6 +690,12 @@ describe('scanner:run', function () { setupCommandTest .command(['scanner:run', '--target', 'path/that/does/not/matter', '--format', 'csv', '--outfile', 'notcsv.xml']) + .finally(ctx => { + // Regardless of what happens in the test itself, we need to delete the file we created. + if (fs.existsSync('notcsv.xml')) { + fs.unlinkSync('notcsv.xml'); + } + }) .it('Warning logged when output file format does not match format', ctx => { expect(ctx.stdout).to.contain(commonMessages.getMessage('validations.outfileFormatMismatch', ['csv', 'xml'])); }); @@ -664,7 +731,7 @@ describe('scanner:run', function () { ]) .it('The baseConfig enables the usage of default Javascript Types', ctx => { // There should be no violations. - expect(ctx.stdout).to.contains('No rule violations found.', 'Should be no violations found in the file.'); + 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. @@ -691,7 +758,7 @@ describe('scanner:run', function () { '--env', '{"qunit": true}' ]) .it('Providing qunit in the --env override should resolve errors about that framework', ctx => { - expect(ctx.stdout).to.contain('No rule violations found.', 'Should be no violations found in the file.'); + expect(ctx.stdout).to.contain(processorMessages.getMessage('output.noViolationsDetected', ['pmd, eslint, retire-js'])); }); }); diff --git a/test/lib/RuleManager.test.ts b/test/lib/RuleManager.test.ts index f426f9881..1a5d03d3c 100644 --- a/test/lib/RuleManager.test.ts +++ b/test/lib/RuleManager.test.ts @@ -273,7 +273,7 @@ describe('RuleManager', () => { const {results} = await ruleManager.runRulesMatchingCriteria(filters, invalidTarget, runOptions, EMPTY_ENGINE_OPTIONS); - expect(results).to.equal(''); + expect(results).to.equal('[]'); Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, messages.getMessage('warning.targetSkipped', [invalidTarget.join(', ')])); Sinon.assert.callCount(telemetrySpy, 1); }); @@ -389,13 +389,13 @@ describe('RuleManager', () => { }) describe('Edge Cases', () => { - it('When no rules match the given criteria, an empty string is returned', async () => { + it('When no rules match the given criteria, an empty summary is returned', async () => { // Define our preposterous filter array. const filters = [new CategoryFilter(['beebleborp'])]; const {results} = await ruleManager.runRulesMatchingCriteria(filters, ['app'], runOptions, EMPTY_ENGINE_OPTIONS); expect(typeof results).to.equal('string', `Output ${results} should have been a string`); - expect(results).to.equal('', `Output ${results} should have been an empty string`); + expect(results).to.equal('[]', `Output ${results} should have been an empty summary (empty array)`); Sinon.assert.callCount(telemetrySpy, 1); }); @@ -407,7 +407,7 @@ describe('RuleManager', () => { const {results} = await ruleManager.runRulesMatchingCriteria(filters, invalidTarget, runOptions, EMPTY_ENGINE_OPTIONS); - expect(results).to.equal(''); + expect(results).to.equal('[]'); Sinon.assert.callCount(uxSpy, 1); Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, messages.getMessage("warning.targetSkipped", [invalidTarget.join(', ')])); Sinon.assert.callCount(telemetrySpy, 1); @@ -422,7 +422,7 @@ describe('RuleManager', () => { const {results} = await ruleManager.runRulesMatchingCriteria(filters, invalidTarget, runOptions, EMPTY_ENGINE_OPTIONS); - expect(results).to.equal(''); + expect(results).to.equal('[]'); Sinon.assert.callCount(uxSpy, 1); Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, messages.getMessage("warning.targetSkipped", [invalidTarget.join(', ')])); Sinon.assert.callCount(telemetrySpy, 1); @@ -436,7 +436,7 @@ describe('RuleManager', () => { const {results} = await ruleManager.runRulesMatchingCriteria(filters, invalidTargets, runOptions, EMPTY_ENGINE_OPTIONS); - expect(results).to.equal(''); + expect(results).to.equal('[]'); Sinon.assert.callCount(uxSpy, 1); Sinon.assert.calledWith(uxSpy, EVENTS.WARNING_ALWAYS, messages.getMessage("warning.targetsSkipped", [invalidTargets.join(', ')])); Sinon.assert.callCount(telemetrySpy, 1); diff --git a/test/lib/util/RunOutputProcessor.test.ts b/test/lib/util/RunOutputProcessor.test.ts index bfff52f8d..4265aa40d 100644 --- a/test/lib/util/RunOutputProcessor.test.ts +++ b/test/lib/util/RunOutputProcessor.test.ts @@ -248,7 +248,7 @@ ${processorMessages.getMessage('output.writtenToConsole')}`; // NOTE: This next test is based on the implementation of run-summary messages in W-8388246, which was known // to be incomplete. When we flesh out that implementation with summaries for other formats, this test might // need to change. - it('JSON-type output should NOT be followed by summary', async () => { + it('JSON-type output with no violations should output be an empty violation set', async () => { const opts: RunOutputOptions = { format: OUTPUT_FORMAT.JSON }; @@ -297,17 +297,20 @@ ${processorMessages.getMessage('output.writtenToConsole')}`; const summaryMap: Map = new Map(); summaryMap.set('pmd', {fileCount: 0, violationCount: 0}); summaryMap.set('eslint', {fileCount: 0, violationCount: 0}); - const fakeRes: RecombinedRuleResults = {minSev: 0, summaryMap, results: ''}; + const fakeRes: RecombinedRuleResults = {minSev: 0, summaryMap, results: '"Problem","Severity","File","Line","Column","Rule","Description","URL","Category","Engine"'}; // THIS IS THE PART BEING TESTED. const output: AnyJson = rop.processRunOutput(fakeRes); - // 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']); + // We expect the empty CSV output followed by the default engine summary and written-to-file messages are logged to the console + const expectedMsg = `${processorMessages.getMessage('output.engineSummaryTemplate', ['pmd', 0, 0])} +${processorMessages.getMessage('output.engineSummaryTemplate', ['eslint', 0, 0])} +${processorMessages.getMessage('output.writtenToOutFile', [fakeFilePath])}`; + Sinon.assert.callCount(logSpy, 1); Sinon.assert.callCount(tableSpy, 0); Sinon.assert.calledWith(logSpy, expectedMsg); - expect(fakeFiles.length).to.equal(0, 'No files should be created'); + expect(fakeFiles.length).to.equal(1, 'A CSV file with only a header should be created'); expect(output).to.equal(expectedMsg, 'Should have returned expected message'); }); diff --git a/test/lib/util/VersionUpgradeManager.test.ts b/test/lib/util/VersionUpgradeManager.test.ts index 2f5f35e45..a31f41f05 100644 --- a/test/lib/util/VersionUpgradeManager.test.ts +++ b/test/lib/util/VersionUpgradeManager.test.ts @@ -329,5 +329,31 @@ describe('VersionUpgradeManager', () => { expect(clonedConfig.currentVersion).to.equal('v3.6.0'); }); }); + + describe('v3.17.0', () => { + // Spoof a config that: + // - Appears to predate v3.17.0 + // - Has a PMD config with '!**/*-meta.xml' in its target patterns + // - Has a RetireJS config with '!**/*-meta.xml' in its target patterns + const spoofedConfig: ConfigContent = { + currentVersion: '3.15.0', + engines: [{ + name: ENGINE.PMD, + targetPatterns: ['**/*.apex', '**/*.xml', '!**/*-meta.xml'], + disabled: false + }, { + name: ENGINE.RETIRE_JS, + targetPatterns: ['**/beep/*.js', '!**/*-meta.xml', '!**/boop.js'], + disabled: false + }] + }; + + const testManagerAsAny = new VersionUpgradeManager() as any; + it('"!**/*-meta.xml" is removed from PMD config', async () => { + await testManagerAsAny.upgrade(spoofedConfig, spoofedConfig.currentVersion, 'v3.17.0'); + expect(spoofedConfig.engines[0].targetPatterns).to.not.include('!**/*-meta.xml', 'Entry should be removed from PMD config'); + expect(spoofedConfig.engines[1].targetPatterns).to.include('!**/*-meta.xml', 'Entry should not be removed from non-PMD config'); + }); + }) }); });