From 40c956d5ebcb47eed4af546c7de68b9f2deb6478 Mon Sep 17 00:00:00 2001 From: Joshua Feingold Date: Wed, 13 Nov 2024 17:17:41 -0600 Subject: [PATCH] NEW @W-17042397@ Detail output now has multilocation support --- package.json | 12 ++--- src/lib/utils/StylingUtil.ts | 13 +++-- src/lib/viewers/ResultsViewer.ts | 42 +++++++++++++---- .../one-multilocation-violation-details.txt | 10 ++++ .../factories/EnginePluginsFactory.test.ts | 2 +- test/lib/viewers/ResultsViewer.test.ts | 47 +++++++++++++++++++ 6 files changed, 106 insertions(+), 20 deletions(-) create mode 100644 test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/one-multilocation-violation-details.txt diff --git a/package.json b/package.json index b7f6f06bd..0b54578b8 100644 --- a/package.json +++ b/package.json @@ -6,12 +6,12 @@ "bugs": "https://github.com/forcedotcom/sfdx-scanner/issues", "dependencies": { "@oclif/core": "^3.3.2", - "@salesforce/code-analyzer-core": "0.14.1", - "@salesforce/code-analyzer-engine-api": "0.11.1", - "@salesforce/code-analyzer-eslint-engine": "0.11.1", - "@salesforce/code-analyzer-pmd-engine": "0.11.1", - "@salesforce/code-analyzer-regex-engine": "0.11.1", - "@salesforce/code-analyzer-retirejs-engine": "0.11.1", + "@salesforce/code-analyzer-core": "0.16.0", + "@salesforce/code-analyzer-engine-api": "0.13.0", + "@salesforce/code-analyzer-eslint-engine": "0.13.0", + "@salesforce/code-analyzer-pmd-engine": "0.13.0", + "@salesforce/code-analyzer-regex-engine": "0.13.0", + "@salesforce/code-analyzer-retirejs-engine": "0.13.0", "@salesforce/core": "^5", "@salesforce/sf-plugins-core": "^5.0.4", "@salesforce/ts-types": "^2.0.9", diff --git a/src/lib/utils/StylingUtil.ts b/src/lib/utils/StylingUtil.ts index 85478e9ba..cb3f43788 100644 --- a/src/lib/utils/StylingUtil.ts +++ b/src/lib/utils/StylingUtil.ts @@ -4,7 +4,7 @@ import ansis from 'ansis'; * For now, the styling methods only accept objects if all of their keys correspond to string values. This puts the * burden of formatting non-string properties on the caller. */ -type Styleable = null | undefined | {[key: string]: string}; +type Styleable = null | undefined | {[key: string]: string|string[]}; export function toStyledHeaderAndBody(header: string, body: Styleable, keys?: string[]): string { const styledHeader: string = toStyledHeader(header); @@ -27,11 +27,16 @@ export function toStyledPropertyList(body: Styleable, selectedKeys?: string[]): const keysToPrint = selectedKeys || [...Object.keys(body)]; const longestKeyLength = Math.max(...keysToPrint.map(k => k.length)); - const styleProperty = (key: string, value: string): string => { + const styleProperty = (key: string, value: string|string[]): string => { const keyPortion = `${ansis.blue(key)}:`; const keyValueGap = ' '.repeat(longestKeyLength - key.length + 1); - const valuePortion = value.replace('\n', `\n${' '.repeat(longestKeyLength + 2)}`); - return `${keyPortion}${keyValueGap}${valuePortion}`; + if (typeof value === 'string') { + const valuePortion = value.replace('\n', `\n${' '.repeat(longestKeyLength + 2)}`); + return `${keyPortion}${keyValueGap}${valuePortion}`; + } else { + const valuePortion: string = value.map(v => `${indent(v, 4)}`).join('\n'); + return `${keyPortion}\n${valuePortion}`; + } } const output = keysToPrint.map(key => styleProperty(key, body[key] || '')); diff --git a/src/lib/viewers/ResultsViewer.ts b/src/lib/viewers/ResultsViewer.ts index 34d795467..5f51f7c66 100644 --- a/src/lib/viewers/ResultsViewer.ts +++ b/src/lib/viewers/ResultsViewer.ts @@ -65,18 +65,42 @@ export class ResultsDetailDisplayer extends AbstractResultsDisplayer { 'summary.detail.violation-header', [idx + 1, rule.getName()] ); - const body = { - severity: `${sev.valueOf()} (${SeverityLevel[sev]})`, - engine: rule.getEngineName(), - message: violation.getMessage(), - location: `${primaryLocation.getFile()}:${primaryLocation.getStartLine()}:${primaryLocation.getStartColumn()}`, - resources: violation.getResourceUrls().join(',') - }; - const keys = ['severity', 'engine', 'message', 'location', 'resources']; - return toStyledHeaderAndBody(header, body, keys); + if (violation.getCodeLocations().length > 1) { + const body = { + severity: `${sev.valueOf()} (${SeverityLevel[sev]})`, + engine: rule.getEngineName(), + message: violation.getMessage(), + locations: stringifyLocations(violation.getCodeLocations(), violation.getPrimaryLocationIndex()), + resources: violation.getResourceUrls().join(',') + }; + const keys = ['severity', 'engine', 'message', 'locations', 'resources']; + return toStyledHeaderAndBody(header, body, keys); + } else { + const body = { + severity: `${sev.valueOf()} (${SeverityLevel[sev]})`, + engine: rule.getEngineName(), + message: violation.getMessage(), + location: `${primaryLocation.getFile()}:${primaryLocation.getStartLine()}:${primaryLocation.getStartColumn()}`, + resources: violation.getResourceUrls().join(',') + }; + const keys = ['severity', 'engine', 'message', 'location', 'resources']; + return toStyledHeaderAndBody(header, body, keys); + } } } +function stringifyLocations(codeLocations: CodeLocation[], primaryIndex: number): string[] { + const locationStrings: string[] = []; + + codeLocations.forEach((loc, idx) => { + const commentPortion: string = loc.getComment() ? ` ${loc.getComment()}` : ''; + const rawLocationString: string = `${loc.getFile()}:${loc.getStartLine()}:${loc.getStartColumn()}${commentPortion}`; + locationStrings.push(idx === primaryIndex ? `**${rawLocationString}**` : rawLocationString); + }); + + return locationStrings; +} + type ResultRow = { num: number; location: string; diff --git a/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/one-multilocation-violation-details.txt b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/one-multilocation-violation-details.txt new file mode 100644 index 000000000..c82aeb8aa --- /dev/null +++ b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/one-multilocation-violation-details.txt @@ -0,0 +1,10 @@ +Found 1 violation(s) across 1 file(s): +=== 1. stub1RuleA + severity: 4 (Low) + engine: stubEngine1 + message: This is a message + locations: + __PATH_TO_FILE_A__:20:1 + **__PATH_TO_FILE_Z__:2:1 This is a comment at Location 2** + __PATH_TO_FILE_A__:1:1 This is a comment at Location 3 + resources: https://example.com/stub1RuleA diff --git a/test/lib/factories/EnginePluginsFactory.test.ts b/test/lib/factories/EnginePluginsFactory.test.ts index 72559c364..7b645c793 100644 --- a/test/lib/factories/EnginePluginsFactory.test.ts +++ b/test/lib/factories/EnginePluginsFactory.test.ts @@ -8,7 +8,7 @@ describe('EnginePluginsFactoryImpl', () => { expect(enginePlugins).toHaveLength(4); expect(enginePlugins[0].getAvailableEngineNames()).toEqual(['eslint']); - expect(enginePlugins[1].getAvailableEngineNames()).toEqual(['pmd']); + expect(enginePlugins[1].getAvailableEngineNames()).toEqual(['pmd', 'cpd']); expect(enginePlugins[2].getAvailableEngineNames()).toEqual(['retire-js']); expect(enginePlugins[3].getAvailableEngineNames()).toEqual(['regex']); }); diff --git a/test/lib/viewers/ResultsViewer.test.ts b/test/lib/viewers/ResultsViewer.test.ts index cb4c74ce9..fa0044a4c 100644 --- a/test/lib/viewers/ResultsViewer.test.ts +++ b/test/lib/viewers/ResultsViewer.test.ts @@ -132,6 +132,53 @@ describe('ResultsViewer implementations', () => { .replace(/__PATH_TO_FILE_Z__/g, PATH_TO_FILE_Z); expect(actualEventText).toContain(expectedViolationDetails); }); + + it('Multi-location violations are correctly displayed', async () => { + // ==== TEST SETUP ==== + // Populate the engine with: + const violations: Violation[] = [ + // A violation. + createViolation(rule1.name, PATH_TO_FILE_A, 20, 1), + ]; + + // Add some additional locations to the violation. + violations[0].codeLocations.push({ + file: PATH_TO_FILE_Z, + startLine: 2, + startColumn: 1, + comment: 'This is a comment at Location 2' + }, { + file: PATH_TO_FILE_A, + startLine: 1, + startColumn: 1, + comment: 'This is a comment at Location 3' + }); + // Declare the second location to be the primary. + violations[0].primaryLocationIndex = 1; + engine1.resultsToReturn = {violations}; + + // "Run" the plugin. + const workspace = await codeAnalyzerCore.createWorkspace(['package.json']); + const rules = await codeAnalyzerCore.selectRules(['all'], {workspace}); + const results = await codeAnalyzerCore.run(rules, {workspace}); + + // ==== TESTED METHOD ==== + // Pass the result object into the viewer. + viewer.view(results); + + // ==== ASSERTIONS ==== + // Compare the text in the events with the text in our comparison file. + const actualDisplayEvents: DisplayEvent[] = spyDisplay.getDisplayEvents(); + for (const event of actualDisplayEvents) { + expect(event.type).toEqual(DisplayEventType.LOG); + } + // Rip off all of ansis's styling, so we're just comparing plain text. + const actualEventText = ansis.strip(actualDisplayEvents.map(e => e.data).join('\n')); + const expectedViolationDetails = (await readComparisonFile('one-multilocation-violation-details.txt')) + .replace(/__PATH_TO_FILE_A__/g, PATH_TO_FILE_A) + .replace(/__PATH_TO_FILE_Z__/g, PATH_TO_FILE_Z); + expect(actualEventText).toContain(expectedViolationDetails); + }) }); describe('ResultsTableDisplayer', () => {