Skip to content

Commit 065608a

Browse files
stephen-carter-at-sfstephen-carter-at-sf
committed
CHANGE: @W-17272495@: Polish detail view output
1 parent 0b225e1 commit 065608a

File tree

5 files changed

+84
-67
lines changed

5 files changed

+84
-67
lines changed

‎src/lib/viewers/ResultsViewer.ts‎

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,41 +62,47 @@ export class ResultsDetailDisplayer extends AbstractResultsDisplayer {
6262
'summary.detail.violation-header',
6363
[idx + 1, rule.getName()]
6464
);
65-
if (violation.getCodeLocations().length > 1) {
66-
const body = {
67-
severity: `${sev.valueOf()} (${SeverityLevel[sev]})`,
68-
engine: rule.getEngineName(),
69-
message: violation.getMessage(),
70-
locations: stringifyLocations(violation.getCodeLocations(), violation.getPrimaryLocationIndex()),
71-
resources: violation.getResourceUrls().join(',')
72-
};
73-
const keys = ['severity', 'engine', 'message', 'locations', 'resources'];
74-
return toStyledHeaderAndBody(header, body, keys);
75-
} else {
76-
const body = {
77-
severity: `${sev.valueOf()} (${SeverityLevel[sev]})`,
78-
engine: rule.getEngineName(),
79-
message: violation.getMessage(),
80-
location: stringifyLocations(violation.getCodeLocations())[0],
81-
resources: violation.getResourceUrls().join(',')
82-
};
83-
const keys = ['severity', 'engine', 'message', 'location', 'resources'];
84-
return toStyledHeaderAndBody(header, body, keys);
65+
const body = {
66+
severity: `${sev.valueOf()} (${SeverityLevel[sev]})`,
67+
engine: rule.getEngineName(),
68+
message: violation.getMessage()
69+
}
70+
const keys: string[] = ['severity', 'engine', 'message'];
71+
if (violation.getCodeLocations().length == 1) {
72+
body['location'] = stringifyLocation(violation.getCodeLocations()[0], false);
73+
keys.push('location');
74+
} else if (violation.getCodeLocations().length > 1) {
75+
body['locations'] = stringifyLocations(violation.getCodeLocations(), violation.getPrimaryLocationIndex());
76+
keys.push('locations');
8577
}
78+
if (violation.getResourceUrls().length == 1) {
79+
body['resource'] = violation.getResourceUrls()[0];
80+
keys.push('resource');
81+
} else if (violation.getResourceUrls().length > 1) {
82+
body['resources'] = violation.getResourceUrls();
83+
keys.push('resources');
84+
}
85+
return toStyledHeaderAndBody(header, body, keys);
8686
}
8787
}
8888

89-
function stringifyLocations(codeLocations: CodeLocation[], primaryIndex?: number): string[] {
90-
const locationStrings: string[] = [];
91-
92-
codeLocations.forEach((loc, idx) => {
93-
const commentPortion: string = loc.getComment() ? ` ${loc.getComment()}` : '';
94-
const locationString: string = `${loc.getFile()}:${loc.getStartLine()}:${loc.getStartColumn()}${commentPortion}`;
95-
const mainPortion: string = primaryIndex != null && primaryIndex === idx ? '(main) ' : '';
96-
locationStrings.push(`${mainPortion}${locationString}`);
97-
});
89+
function stringifyLocations(codeLocations: CodeLocation[], primaryIndex: number): string[] {
90+
return codeLocations.map((loc, idx) =>
91+
stringifyLocation(loc, codeLocations.length > 1 && primaryIndex === idx));
92+
}
9893

99-
return locationStrings;
94+
function stringifyLocation(loc: CodeLocation, displayMain: boolean): string {
95+
const mainPortion: string = displayMain ? '(main) ' : '';
96+
const commentPortion: string = loc.getComment() ? ` "${loc.getComment()}"` : '';
97+
let rangePortion: string = '';
98+
if (loc.getStartLine()) {
99+
rangePortion += ` (${loc.getStartLine()}:${loc.getStartColumn() || 1}`;
100+
if (loc.getEndLine()) {
101+
rangePortion += `-${loc.getEndLine()}:${loc.getEndColumn() || 1}`;
102+
}
103+
rangePortion += ')';
104+
}
105+
return `${mainPortion}${loc.getFile()}${rangePortion}${commentPortion}`;
100106
}
101107

102108
type ResultRow = {

‎test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-identical-violations-details.txt‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,26 +3,34 @@
33
severity: 4 (Low)
44
engine: stubEngine1
55
message: This is a message
6-
location: __PATH_TO_SOME_FILE__:1:1
7-
resources: https://example.com/stub1RuleA
6+
location: __PATH_TO_SOME_FILE__ (1:1)
7+
resources:
8+
https://example.com/stub1RuleA
9+
https://violation_specific.url
810

911
=== 2. stub1RuleA
1012
severity: 4 (Low)
1113
engine: stubEngine1
1214
message: This is a message
13-
location: __PATH_TO_SOME_FILE__:1:1
14-
resources: https://example.com/stub1RuleA
15+
location: __PATH_TO_SOME_FILE__ (1:1)
16+
resources:
17+
https://example.com/stub1RuleA
18+
https://violation_specific.url
1519

1620
=== 3. stub1RuleA
1721
severity: 4 (Low)
1822
engine: stubEngine1
1923
message: This is a message
20-
location: __PATH_TO_SOME_FILE__:1:1
21-
resources: https://example.com/stub1RuleA
24+
location: __PATH_TO_SOME_FILE__ (1:1)
25+
resources:
26+
https://example.com/stub1RuleA
27+
https://violation_specific.url
2228

2329
=== 4. stub1RuleA
2430
severity: 4 (Low)
2531
engine: stubEngine1
2632
message: This is a message
27-
location: __PATH_TO_SOME_FILE__:1:1
28-
resources: https://example.com/stub1RuleA
33+
location: __PATH_TO_SOME_FILE__ (1:1)
34+
resources:
35+
https://example.com/stub1RuleA
36+
https://violation_specific.url

‎test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/four-unique-violations-details.txt‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,28 @@
11

22
=== 1. stub1RuleB
3-
severity: 2 (High)
4-
engine: stubEngine1
5-
message: This is a message
6-
location: __PATH_TO_FILE_Z__:20:1
7-
resources: https://example.com/stub1RuleB
3+
severity: 2 (High)
4+
engine: stubEngine1
5+
message: This is a message
6+
location: __PATH_TO_FILE_Z__ (20:1)
7+
resource: https://example.com/stub1RuleB
88

99
=== 2. stub1RuleA
10-
severity: 4 (Low)
11-
engine: stubEngine1
12-
message: This is a message
13-
location: __PATH_TO_FILE_A__:1:1
14-
resources: https://example.com/stub1RuleA
10+
severity: 4 (Low)
11+
engine: stubEngine1
12+
message: This is a message
13+
location: __PATH_TO_FILE_A__ (1:1)
14+
resource: https://example.com/stub1RuleA
1515

1616
=== 3. stub1RuleA
17-
severity: 4 (Low)
18-
engine: stubEngine1
19-
message: This is a message
20-
location: __PATH_TO_FILE_A__:20:1
21-
resources: https://example.com/stub1RuleA
17+
severity: 4 (Low)
18+
engine: stubEngine1
19+
message: This is a message
20+
location: __PATH_TO_FILE_A__ (20:1)
21+
resource: https://example.com/stub1RuleA
2222

2323
=== 4. stub1RuleA
24-
severity: 4 (Low)
25-
engine: stubEngine1
26-
message: This is a message
27-
location: __PATH_TO_FILE_Z__:1:1
28-
resources: https://example.com/stub1RuleA
24+
severity: 4 (Low)
25+
engine: stubEngine1
26+
message: This is a message
27+
location: __PATH_TO_FILE_Z__ (1:1)
28+
resource: https://example.com/stub1RuleA

‎test/fixtures/comparison-files/lib/viewers/ResultsViewer.test.ts/one-multilocation-violation-details.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
engine: stubEngine1
55
message: This is a message
66
locations:
7-
__PATH_TO_FILE_A__:20:1
8-
(main) __PATH_TO_FILE_Z__:2:1 This is a comment at Location 2
9-
__PATH_TO_FILE_A__:1:1 This is a comment at Location 3
10-
resources: https://example.com/stub1RuleA
7+
__PATH_TO_FILE_A__ (20:1)
8+
(main) __PATH_TO_FILE_Z__ (2:1) "This is a comment at Location 2"
9+
__PATH_TO_FILE_A__ (1:1-3:1) "This is a comment at Location 3"
10+
resource: https://example.com/stub1RuleA

‎test/lib/viewers/ResultsViewer.test.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('ResultsViewer implementations', () => {
6969
// ==== TEST SETUP ====
7070
// This test doesn't care about sorting, so just assign our engine several copies of the same violation.
7171
const violations: Violation[] = repeatViolation(
72-
createViolation(rule1.name, PATH_TO_SOME_FILE, 1, 1),
72+
createViolation(rule1.name, PATH_TO_SOME_FILE, 1, 1, ['https://violation_specific.url']),
7373
4);
7474
engine1.resultsToReturn = {violations};
7575
const workspace = await codeAnalyzerCore.createWorkspace([PATH_TO_SOME_FILE]);
@@ -146,11 +146,13 @@ describe('ResultsViewer implementations', () => {
146146
file: PATH_TO_FILE_Z,
147147
startLine: 2,
148148
startColumn: 1,
149-
comment: 'This is a comment at Location 2'
149+
endColumn: 7,
150+
comment: 'This is a comment at Location 2',
150151
}, {
151152
file: PATH_TO_FILE_A,
152153
startLine: 1,
153154
startColumn: 1,
155+
endLine: 3,
154156
comment: 'This is a comment at Location 3'
155157
});
156158
// Declare the second location to be the primary.
@@ -177,7 +179,7 @@ describe('ResultsViewer implementations', () => {
177179
const expectedViolationDetails = (await readComparisonFile('one-multilocation-violation-details.txt'))
178180
.replace(/__PATH_TO_FILE_A__/g, PATH_TO_FILE_A)
179181
.replace(/__PATH_TO_FILE_Z__/g, PATH_TO_FILE_Z);
180-
expect(actualEventText).toContain(expectedViolationDetails);
182+
expect(actualEventText).toEqual(expectedViolationDetails);
181183
})
182184
});
183185

@@ -301,7 +303,7 @@ describe('Tests for the findLongestCommonParentFolderOf helper function', () =>
301303
}
302304
});
303305

304-
function createViolation(ruleName: string, file: string, startLine: number, startColumn: number): Violation {
306+
function createViolation(ruleName: string, file: string, startLine: number, startColumn: number, resourceUrls: string[] = []): Violation {
305307
return {
306308
ruleName,
307309
message: 'This is a message',
@@ -310,7 +312,8 @@ function createViolation(ruleName: string, file: string, startLine: number, star
310312
startLine,
311313
startColumn
312314
}],
313-
primaryLocationIndex: 0
315+
primaryLocationIndex: 0,
316+
resourceUrls: resourceUrls
314317
};
315318
}
316319

0 commit comments

Comments
 (0)