diff --git a/messages/results-viewer.md b/messages/results-viewer.md index 8ed4253fe..e9e526005 100644 --- a/messages/results-viewer.md +++ b/messages/results-viewer.md @@ -2,7 +2,7 @@ %d. %s -# summary.table.results-relative-to +# summary.shared.results-relative-to Violation file paths relative to '%s'. diff --git a/src/lib/viewers/ResultsViewer.ts b/src/lib/viewers/ResultsViewer.ts index d4ed113a5..1034533d8 100644 --- a/src/lib/viewers/ResultsViewer.ts +++ b/src/lib/viewers/ResultsViewer.ts @@ -43,17 +43,18 @@ abstract class AbstractResultsDisplayer implements ResultsViewer { export class ResultsDetailDisplayer extends AbstractResultsDisplayer { protected _view(results: RunResults): void { const violations = sortViolations(results.getViolations()); - - this.displayDetails(violations); + const runDir: string = results.getRunDirectory(); + this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.shared.results-relative-to', [runDir]) + "\n"); + this.displayDetails(violations, runDir); } - private displayDetails(violations: Violation[]): void { + private displayDetails(violations: Violation[], runDir: string): void { const styledViolations: string[] = violations - .map((violation, idx) => this.styleViolation(violation, idx)); + .map((violation, idx) => this.styleViolation(violation, idx, runDir)); this.display.displayLog(styledViolations.join('\n\n')); } - private styleViolation(violation: Violation, idx: number): string { + private styleViolation(violation: Violation, idx: number, runDir: string): string { const rule = violation.getRule(); const sev = rule.getSeverityLevel(); @@ -62,41 +63,52 @@ export class ResultsDetailDisplayer extends AbstractResultsDisplayer { 'summary.detail.violation-header', [idx + 1, rule.getName()] ); - 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: stringifyLocations(violation.getCodeLocations())[0], - resources: violation.getResourceUrls().join(',') - }; - const keys = ['severity', 'engine', 'message', 'location', 'resources']; - return toStyledHeaderAndBody(header, body, keys); + const body = { + severity: `${sev.valueOf()} (${SeverityLevel[sev]})`, + engine: rule.getEngineName(), + message: violation.getMessage() + } + const keys: string[] = ['severity', 'engine', 'message']; + if (violation.getCodeLocations().length == 1) { + body['location'] = stringifyLocation(violation.getCodeLocations()[0], false, runDir); + keys.push('location'); + } else if (violation.getCodeLocations().length > 1) { + body['locations'] = stringifyLocations(violation.getCodeLocations(), violation.getPrimaryLocationIndex(), runDir); + keys.push('locations'); } + if (violation.getResourceUrls().length == 1) { + body['resource'] = violation.getResourceUrls()[0]; + keys.push('resource'); + } else if (violation.getResourceUrls().length > 1) { + body['resources'] = violation.getResourceUrls(); + keys.push('resources'); + } + return toStyledHeaderAndBody(header, body, keys); } } -function stringifyLocations(codeLocations: CodeLocation[], primaryIndex?: number): string[] { - const locationStrings: string[] = []; +function stringifyLocations(codeLocations: CodeLocation[], primaryIndex: number, runDir: string): string[] { + return codeLocations.map((loc, idx) => + stringifyLocation(loc, codeLocations.length > 1 && primaryIndex === idx, runDir)); +} - codeLocations.forEach((loc, idx) => { - const commentPortion: string = loc.getComment() ? ` ${loc.getComment()}` : ''; - const locationString: string = `${loc.getFile()}:${loc.getStartLine()}:${loc.getStartColumn()}${commentPortion}`; - const mainPortion: string = primaryIndex != null && primaryIndex === idx ? '(main) ' : ''; - locationStrings.push(`${mainPortion}${locationString}`); - }); +function stringifyLocation(loc: CodeLocation, displayMain: boolean, runDir: string): string { + const mainPortion: string = displayMain ? '(main) ' : ''; + let filePortion: string | undefined = loc.getFile(); + if (filePortion && filePortion.startsWith(runDir)) { + filePortion = filePortion.slice(runDir.length); + } + let rangePortion: string = ''; + if (loc.getStartLine()) { + rangePortion += ` (${loc.getStartLine()}:${loc.getStartColumn() || 1}`; + if (loc.getEndLine()) { + rangePortion += `-${loc.getEndLine()}:${loc.getEndColumn() || 1}`; + } + rangePortion += ')'; + } + const commentPortion: string = loc.getComment() ? ` "${loc.getComment()}"` : ''; - return locationStrings; + return `${mainPortion}${filePortion}${rangePortion}${commentPortion}`; } type ResultRow = { @@ -145,7 +157,7 @@ export class ResultsTableDisplayer extends AbstractResultsDisplayer { } }); - this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.table.results-relative-to', [parentFolder])); + this.display.displayLog(getMessage(BundleName.ResultsViewer, 'summary.shared.results-relative-to', [parentFolder])); this.display.displayTable(resultRows, TABLE_COLUMNS); } } diff --git a/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-identical-violations-details.txt b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-identical-violations-details.txt index adb9b67b1..a31e21211 100644 --- a/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-identical-violations-details.txt +++ b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-identical-violations-details.txt @@ -1,28 +1,38 @@ +Violation file paths relative to '{{RUNDIR}}'. + === 1. stub1RuleA severity: 4 (Low) engine: stubEngine1 message: This is a message - location: __PATH_TO_SOME_FILE__:1:1 - resources: https://example.com/stub1RuleA + location: test{{PATHSEP}}sample-code{{PATHSEP}}someFile.cls (1:1) + resources: + https://example.com/stub1RuleA + https://violation_specific.url === 2. stub1RuleA severity: 4 (Low) engine: stubEngine1 message: This is a message - location: __PATH_TO_SOME_FILE__:1:1 - resources: https://example.com/stub1RuleA + location: test{{PATHSEP}}sample-code{{PATHSEP}}someFile.cls (1:1) + resources: + https://example.com/stub1RuleA + https://violation_specific.url === 3. stub1RuleA severity: 4 (Low) engine: stubEngine1 message: This is a message - location: __PATH_TO_SOME_FILE__:1:1 - resources: https://example.com/stub1RuleA + location: test{{PATHSEP}}sample-code{{PATHSEP}}someFile.cls (1:1) + resources: + https://example.com/stub1RuleA + https://violation_specific.url === 4. stub1RuleA severity: 4 (Low) engine: stubEngine1 message: This is a message - location: __PATH_TO_SOME_FILE__:1:1 - resources: https://example.com/stub1RuleA + location: test{{PATHSEP}}sample-code{{PATHSEP}}someFile.cls (1:1) + resources: + https://example.com/stub1RuleA + https://violation_specific.url diff --git a/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-unique-violations-details.txt b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-unique-violations-details.txt index 63f537b94..1bf2b5575 100644 --- a/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-unique-violations-details.txt +++ b/test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-unique-violations-details.txt @@ -1,28 +1,30 @@ +Violation file paths relative to '{{RUNDIR}}'. + === 1. stub1RuleB - severity: 2 (High) - engine: stubEngine1 - message: This is a message - location: __PATH_TO_FILE_Z__:20:1 - resources: https://example.com/stub1RuleB + severity: 2 (High) + engine: stubEngine1 + message: This is a message + location: test{{PATHSEP}}sample-code{{PATHSEP}}fileZ.cls (20:1) + resource: https://example.com/stub1RuleB === 2. stub1RuleA - severity: 4 (Low) - engine: stubEngine1 - message: This is a message - location: __PATH_TO_FILE_A__:1:1 - resources: https://example.com/stub1RuleA + severity: 4 (Low) + engine: stubEngine1 + message: This is a message + location: test{{PATHSEP}}sample-code{{PATHSEP}}fileA.cls (1:1) + resource: https://example.com/stub1RuleA === 3. stub1RuleA - severity: 4 (Low) - engine: stubEngine1 - message: This is a message - location: __PATH_TO_FILE_A__:20:1 - resources: https://example.com/stub1RuleA + severity: 4 (Low) + engine: stubEngine1 + message: This is a message + location: test{{PATHSEP}}sample-code{{PATHSEP}}fileA.cls (20:1) + resource: https://example.com/stub1RuleA === 4. stub1RuleA - severity: 4 (Low) - engine: stubEngine1 - message: This is a message - location: __PATH_TO_FILE_Z__:1:1 - resources: https://example.com/stub1RuleA + severity: 4 (Low) + engine: stubEngine1 + message: This is a message + location: test{{PATHSEP}}sample-code{{PATHSEP}}fileZ.cls (1:1) + resource: https://example.com/stub1RuleA 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 index d7fe30fca..fce989c28 100644 --- 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 @@ -1,10 +1,12 @@ +Violation file paths relative to '{{RUNDIR}}'. + === 1. stub1RuleA severity: 4 (Low) engine: stubEngine1 message: This is a message locations: - __PATH_TO_FILE_A__:20:1 - (main) __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 + test{{PATHSEP}}sample-code{{PATHSEP}}fileA.cls (20:1) + (main) test{{PATHSEP}}sample-code{{PATHSEP}}fileZ.cls (2:1) "This is a comment at Location 2" + test{{PATHSEP}}sample-code{{PATHSEP}}fileA.cls (1:1-3:1) "This is a comment at Location 3" + resource: https://example.com/stub1RuleA diff --git a/test/lib/viewers/ResultsViewer.test.ts b/test/lib/viewers/ResultsViewer.test.ts index 6e642ce87..83ac512e6 100644 --- a/test/lib/viewers/ResultsViewer.test.ts +++ b/test/lib/viewers/ResultsViewer.test.ts @@ -69,7 +69,7 @@ describe('ResultsViewer implementations', () => { // ==== TEST SETUP ==== // This test doesn't care about sorting, so just assign our engine several copies of the same violation. const violations: Violation[] = repeatViolation( - createViolation(rule1.name, PATH_TO_SOME_FILE, 1, 1), + createViolation(rule1.name, PATH_TO_SOME_FILE, 1, 1, ['https://violation_specific.url']), 4); engine1.resultsToReturn = {violations}; const workspace = await codeAnalyzerCore.createWorkspace([PATH_TO_SOME_FILE]); @@ -90,8 +90,9 @@ describe('ResultsViewer implementations', () => { // 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('four-identical-violations-details.txt')) - .replace(/__PATH_TO_SOME_FILE__/g, PATH_TO_SOME_FILE); - expect(actualEventText).toContain(expectedViolationDetails); + .replaceAll("{{PATHSEP}}", path.sep) + .replace("{{RUNDIR}}", results.getRunDirectory()); + expect(actualEventText).toEqual(expectedViolationDetails); }); // The reasoning behind this sorting order is so that the Detail view can function as a "show me the N most @@ -128,9 +129,9 @@ describe('ResultsViewer implementations', () => { // 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('four-unique-violations-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); + .replaceAll("{{PATHSEP}}", path.sep) + .replace("{{RUNDIR}}", results.getRunDirectory()); + expect(actualEventText).toEqual(expectedViolationDetails); }); it('Multi-location violations are correctly displayed', async () => { @@ -146,11 +147,13 @@ describe('ResultsViewer implementations', () => { file: PATH_TO_FILE_Z, startLine: 2, startColumn: 1, - comment: 'This is a comment at Location 2' + endColumn: 7, + comment: 'This is a comment at Location 2', }, { file: PATH_TO_FILE_A, startLine: 1, startColumn: 1, + endLine: 3, comment: 'This is a comment at Location 3' }); // Declare the second location to be the primary. @@ -175,9 +178,9 @@ describe('ResultsViewer implementations', () => { // 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); + .replaceAll("{{PATHSEP}}", path.sep) + .replace("{{RUNDIR}}", results.getRunDirectory()); + expect(actualEventText).toEqual(expectedViolationDetails); }) }); @@ -227,7 +230,7 @@ describe('ResultsViewer implementations', () => { expect(displayEvents[0].type).toEqual(DisplayEventType.LOG); expect(displayEvents[0].data).toEqual(''); expect(displayEvents[1].type).toEqual(DisplayEventType.LOG); - expect(displayEvents[1].data).toEqual(getMessage(BundleName.ResultsViewer, 'summary.table.results-relative-to', [PATH_TO_SAMPLE_CODE])); + expect(displayEvents[1].data).toEqual(getMessage(BundleName.ResultsViewer, 'summary.shared.results-relative-to', [PATH_TO_SAMPLE_CODE])); expect(displayEvents[2].type).toEqual(DisplayEventType.TABLE); expect(displayEvents[2].data).toEqual(`{"columns":["#","Severity","Rule","Location","Message"],"rows":[{"num":1,"location":"someFile.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"},{"num":2,"location":"someFile.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"},{"num":3,"location":"someFile.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"},{"num":4,"location":"someFile.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"}]}`); }); @@ -261,7 +264,7 @@ describe('ResultsViewer implementations', () => { expect(displayEvents[0].type).toEqual(DisplayEventType.LOG); expect(displayEvents[0].data).toEqual(''); expect(displayEvents[1].type).toEqual(DisplayEventType.LOG); - expect(displayEvents[1].data).toEqual(getMessage(BundleName.ResultsViewer, 'summary.table.results-relative-to', [PATH_TO_SAMPLE_CODE])); + expect(displayEvents[1].data).toEqual(getMessage(BundleName.ResultsViewer, 'summary.shared.results-relative-to', [PATH_TO_SAMPLE_CODE])); expect(displayEvents[2].type).toEqual(DisplayEventType.TABLE); expect(displayEvents[2].data).toEqual(`{"columns":["#","Severity","Rule","Location","Message"],"rows":[{"num":1,"location":"fileZ.cls:20:1","rule":"stubEngine1:stub1RuleB","severity":"2 (High)","message":"This is a message"},{"num":2,"location":"fileA.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"},{"num":3,"location":"fileA.cls:20:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"},{"num":4,"location":"fileZ.cls:1:1","rule":"stubEngine1:stub1RuleA","severity":"4 (Low)","message":"This is a message"}]}`); }); @@ -301,7 +304,7 @@ describe('Tests for the findLongestCommonParentFolderOf helper function', () => } }); -function createViolation(ruleName: string, file: string, startLine: number, startColumn: number): Violation { +function createViolation(ruleName: string, file: string, startLine: number, startColumn: number, resourceUrls: string[] = []): Violation { return { ruleName, message: 'This is a message', @@ -310,7 +313,8 @@ function createViolation(ruleName: string, file: string, startLine: number, star startLine, startColumn }], - primaryLocationIndex: 0 + primaryLocationIndex: 0, + resourceUrls: resourceUrls }; }