Skip to content

Commit

Permalink
NEW @W-17042397@ Detail output now has multilocation support
Browse files Browse the repository at this point in the history
  • Loading branch information
jfeingold35 committed Nov 13, 2024
1 parent 865619b commit 40c956d
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 20 deletions.
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 9 additions & 4 deletions src/lib/utils/StylingUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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] || ''));
Expand Down
42 changes: 33 additions & 9 deletions src/lib/viewers/ResultsViewer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion test/lib/factories/EnginePluginsFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
});
Expand Down
47 changes: 47 additions & 0 deletions test/lib/viewers/ResultsViewer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 40c956d

Please sign in to comment.