diff --git a/src/lib/viewers/ResultsViewer.ts b/src/lib/viewers/ResultsViewer.ts index d4ed113a5..daa81f739 100644 --- a/src/lib/viewers/ResultsViewer.ts +++ b/src/lib/viewers/ResultsViewer.ts @@ -62,41 +62,47 @@ 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); + keys.push('location'); + } else if (violation.getCodeLocations().length > 1) { + body['locations'] = stringifyLocations(violation.getCodeLocations(), violation.getPrimaryLocationIndex()); + 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[] = []; - - 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 stringifyLocations(codeLocations: CodeLocation[], primaryIndex: number): string[] { + return codeLocations.map((loc, idx) => + stringifyLocation(loc, codeLocations.length > 1 && primaryIndex === idx)); +} - return locationStrings; +function stringifyLocation(loc: CodeLocation, displayMain: boolean): string { + const mainPortion: string = displayMain ? '(main) ' : ''; + const commentPortion: string = loc.getComment() ? ` "${loc.getComment()}"` : ''; + let rangePortion: string = ''; + if (loc.getStartLine()) { + rangePortion += ` (${loc.getStartLine()}:${loc.getStartColumn() || 1}`; + if (loc.getEndLine()) { + rangePortion += `-${loc.getEndLine()}:${loc.getEndColumn() || 1}`; + } + rangePortion += ')'; + } + return `${mainPortion}${loc.getFile()}${rangePortion}${commentPortion}`; } type ResultRow = { 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..d35c56933 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 @@ -3,26 +3,34 @@ severity: 4 (Low) engine: stubEngine1 message: This is a message - location: __PATH_TO_SOME_FILE__:1:1 - resources: https://example.com/stub1RuleA + location: __PATH_TO_SOME_FILE__ (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: __PATH_TO_SOME_FILE__ (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: __PATH_TO_SOME_FILE__ (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: __PATH_TO_SOME_FILE__ (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..c0ff38a99 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,28 @@ === 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: __PATH_TO_FILE_Z__ (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: __PATH_TO_FILE_A__ (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: __PATH_TO_FILE_A__ (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: __PATH_TO_FILE_Z__ (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..2f67e5936 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 @@ -4,7 +4,7 @@ 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 + __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-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..876ab59ee 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]); @@ -146,11 +146,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. @@ -177,7 +179,7 @@ describe('ResultsViewer implementations', () => { 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); + expect(actualEventText).toEqual(expectedViolationDetails); }) }); @@ -301,7 +303,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 +312,8 @@ function createViolation(ruleName: string, file: string, startLine: number, star startLine, startColumn }], - primaryLocationIndex: 0 + primaryLocationIndex: 0, + resourceUrls: resourceUrls }; }