CHANGE: @W-17272495@: Polish detail view output

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'.
 

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);
 	}
 }

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

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

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

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
 	};
 }