Skip to content

Commit 5199b39

Browse files
rbargholzortaOrta Therox
authored andcommitted
fix(45713) Improve error report summaries (microsoft#45742)
* Improve error report summaries (microsoft#45713) * fixup! Improve error report summaries (microsoft#45713) * fixup! fixup! Improve error report summaries (microsoft#45713) * Adds support for handling localization renaming the 'files' header due to localization * fixup! Adds support for handling localization renaming the 'files' header due to localization - Fixed baseline error - Fixed linter error Co-authored-by: Orta <[email protected]> Co-authored-by: Orta Therox <[email protected]>
1 parent afa371c commit 5199b39

File tree

50 files changed

+575
-98
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+575
-98
lines changed

src/compiler/diagnosticMessages.json

+18
Original file line numberDiff line numberDiff line change
@@ -4156,6 +4156,12 @@
41564156
"category": "Message",
41574157
"code": 6040
41584158
},
4159+
"Errors Files": {
4160+
"_locale_notes": "There is a double space, and the order cannot be changed (they're table headings) ^",
4161+
"category": "Message",
4162+
"code": 6041
4163+
},
4164+
41594165
"Generates corresponding '.map' file.": {
41604166
"category": "Message",
41614167
"code": 6043
@@ -4935,6 +4941,18 @@
49354941
"category": "Error",
49364942
"code": 6258
49374943
},
4944+
"Found 1 error in {1}": {
4945+
"category": "Message",
4946+
"code": 6259
4947+
},
4948+
"Found {0} errors in 1 file.": {
4949+
"category": "Message",
4950+
"code": 6260
4951+
},
4952+
"Found {0} errors in {1} files.": {
4953+
"category": "Message",
4954+
"code": 6261
4955+
},
49384956

49394957
"Directory '{0}' has no containing package.json scope. Imports will not resolve.": {
49404958
"category": "Message",

src/compiler/tsbuildPublic.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ namespace ts {
7676
return fileExtensionIs(fileName, Extension.Dts);
7777
}
7878

79-
export type ReportEmitErrorSummary = (errorCount: number) => void;
79+
export type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
80+
81+
export interface ReportFileInError {
82+
fileName: string;
83+
line: number;
84+
}
8085

8186
export interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
8287
createDirectory?(path: string): void;
@@ -2003,10 +2008,12 @@ namespace ts {
20032008
const canReportSummary = state.watch || !!state.host.reportErrorSummary;
20042009
const { diagnostics } = state;
20052010
let totalErrors = 0;
2011+
let filesInError: (ReportFileInError | undefined)[] = [];
20062012
if (isCircularBuildOrder(buildOrder)) {
20072013
reportBuildQueue(state, buildOrder.buildOrder);
20082014
reportErrors(state, buildOrder.circularDiagnostics);
20092015
if (canReportSummary) totalErrors += getErrorCountForSummary(buildOrder.circularDiagnostics);
2016+
if (canReportSummary) filesInError = [...filesInError, ...getFilesInErrorForSummary(buildOrder.circularDiagnostics)];
20102017
}
20112018
else {
20122019
// Report errors from the other projects
@@ -2017,13 +2024,14 @@ namespace ts {
20172024
}
20182025
});
20192026
if (canReportSummary) diagnostics.forEach(singleProjectErrors => totalErrors += getErrorCountForSummary(singleProjectErrors));
2027+
if (canReportSummary) diagnostics.forEach(singleProjectErrors => [...filesInError, ...getFilesInErrorForSummary(singleProjectErrors)]);
20202028
}
20212029

20222030
if (state.watch) {
20232031
reportWatchStatus(state, getWatchErrorSummaryDiagnosticMessage(totalErrors), totalErrors);
20242032
}
20252033
else if (state.host.reportErrorSummary) {
2026-
state.host.reportErrorSummary(totalErrors);
2034+
state.host.reportErrorSummary(totalErrors, filesInError);
20272035
}
20282036
}
20292037

src/compiler/watch.ts

+77-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace ts {
77
} : undefined;
88

99
/**
10-
* Create a function that reports error by writing to the system and handles the formating of the diagnostic
10+
* Create a function that reports error by writing to the system and handles the formatting of the diagnostic
1111
*/
1212
export function createDiagnosticReporter(system: System, pretty?: boolean): DiagnosticReporter {
1313
const host: FormatDiagnosticsHost = system === sys && sysFormatDiagnosticsHost ? sysFormatDiagnosticsHost : {
@@ -101,16 +101,87 @@ namespace ts {
101101
return countWhere(diagnostics, diagnostic => diagnostic.category === DiagnosticCategory.Error);
102102
}
103103

104+
export function getFilesInErrorForSummary(diagnostics: readonly Diagnostic[]): (ReportFileInError | undefined)[] {
105+
const filesInError =
106+
filter(diagnostics, diagnostic => diagnostic.category === DiagnosticCategory.Error)
107+
.map(
108+
errorDiagnostic => {
109+
if(errorDiagnostic.file === undefined) return;
110+
return `${errorDiagnostic.file.fileName}`;
111+
});
112+
return filesInError.map((fileName: string) => {
113+
const diagnosticForFileName = find(diagnostics, diagnostic =>
114+
diagnostic.file !== undefined && diagnostic.file.fileName === fileName
115+
);
116+
117+
if(diagnosticForFileName !== undefined) {
118+
const { line } = getLineAndCharacterOfPosition(diagnosticForFileName.file!, diagnosticForFileName.start!);
119+
return {
120+
fileName,
121+
line: line + 1,
122+
};
123+
}
124+
});
125+
}
126+
104127
export function getWatchErrorSummaryDiagnosticMessage(errorCount: number) {
105128
return errorCount === 1 ?
106129
Diagnostics.Found_1_error_Watching_for_file_changes :
107130
Diagnostics.Found_0_errors_Watching_for_file_changes;
108131
}
109132

110-
export function getErrorSummaryText(errorCount: number, newLine: string) {
133+
export function getErrorSummaryText(
134+
errorCount: number,
135+
filesInError: readonly (ReportFileInError | undefined)[],
136+
newLine: string
137+
) {
111138
if (errorCount === 0) return "";
112-
const d = createCompilerDiagnostic(errorCount === 1 ? Diagnostics.Found_1_error : Diagnostics.Found_0_errors, errorCount);
113-
return `${newLine}${flattenDiagnosticMessageText(d.messageText, newLine)}${newLine}${newLine}`;
139+
const nonNilFiles = filesInError.filter(fileInError => fileInError !== undefined);
140+
const distinctFileNamesWithLines = nonNilFiles.map(fileInError => `${fileInError!.fileName}:${fileInError!.line}`)
141+
.filter((value, index, self) => self.indexOf(value) === index);
142+
const d = errorCount === 1 ?
143+
createCompilerDiagnostic(
144+
filesInError[0] !== undefined ?
145+
Diagnostics.Found_1_error_in_1 :
146+
Diagnostics.Found_1_error,
147+
errorCount,
148+
distinctFileNamesWithLines[0]) :
149+
createCompilerDiagnostic(
150+
distinctFileNamesWithLines.length === 0 ?
151+
Diagnostics.Found_0_errors :
152+
distinctFileNamesWithLines.length === 1 ?
153+
Diagnostics.Found_0_errors_in_1_file :
154+
Diagnostics.Found_0_errors_in_1_files,
155+
errorCount,
156+
distinctFileNamesWithLines.length);
157+
return `${newLine}${flattenDiagnosticMessageText(d.messageText, newLine)}${newLine}${newLine}${errorCount > 1 ? createTabularErrorsDisplay(nonNilFiles) : ""}`;
158+
}
159+
160+
function createTabularErrorsDisplay(filesInError: (ReportFileInError | undefined)[]) {
161+
const distinctFiles = filesInError.filter((value, index, self) => index === self.findIndex(file => file?.fileName === value?.fileName));
162+
if (distinctFiles.length === 0) return "";
163+
164+
const numberLength = (num: number) => Math.log(num) * Math.LOG10E + 1;
165+
const fileToErrorCount = distinctFiles.map(file => ([file, countWhere(filesInError, fileInError => fileInError!.fileName === file!.fileName)] as const));
166+
const maxErrors = fileToErrorCount.reduce((acc, value) => Math.max(acc, value[1] || 0), 0);
167+
168+
const headerRow = Diagnostics.Errors_Files.message;
169+
const leftColumnHeadingLength = headerRow.split(" ")[0].length;
170+
const leftPaddingGoal = Math.max(leftColumnHeadingLength, numberLength(maxErrors));
171+
const headerPadding = Math.max(numberLength(maxErrors) - leftColumnHeadingLength, 0);
172+
173+
let tabularData = "";
174+
tabularData += " ".repeat(headerPadding) + headerRow + "\n";
175+
fileToErrorCount.forEach((row) => {
176+
const [file, errorCount] = row;
177+
const errorCountDigitsLength = Math.log(errorCount) * Math.LOG10E + 1 | 0;
178+
const leftPadding = errorCountDigitsLength < leftPaddingGoal ?
179+
" ".repeat(leftPaddingGoal - errorCountDigitsLength)
180+
: "";
181+
tabularData += `${leftPadding}${errorCount} ${file!.fileName}:${file!.line}\n`;
182+
});
183+
184+
return tabularData;
114185
}
115186

116187
export function isBuilderProgram(program: Program | BuilderProgram): program is BuilderProgram {
@@ -350,7 +421,7 @@ namespace ts {
350421
}
351422

352423
if (reportSummary) {
353-
reportSummary(getErrorCountForSummary(diagnostics));
424+
reportSummary(getErrorCountForSummary(diagnostics), getFilesInErrorForSummary(diagnostics));
354425
}
355426

356427
return {
@@ -656,7 +727,7 @@ namespace ts {
656727
builderProgram,
657728
input.reportDiagnostic || createDiagnosticReporter(system),
658729
s => host.trace && host.trace(s),
659-
input.reportErrorSummary || input.options.pretty ? errorCount => system.write(getErrorSummaryText(errorCount, system.newLine)) : undefined
730+
input.reportErrorSummary || input.options.pretty ? (errorCount, filesInError) => system.write(getErrorSummaryText(errorCount, filesInError, system.newLine)) : undefined
660731
);
661732
if (input.afterProgramEmitAndDiagnostics) input.afterProgramEmitAndDiagnostics(builderProgram);
662733
return exitStatus;

src/executeCommandLine/executeCommandLine.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ namespace ts {
773773

774774
function createReportErrorSummary(sys: System, options: CompilerOptions | BuildOptions): ReportEmitErrorSummary | undefined {
775775
return shouldBePretty(sys, options) ?
776-
errorCount => sys.write(getErrorSummaryText(errorCount, sys.newLine)) :
776+
(errorCount, filesInError) => sys.write(getErrorSummaryText(errorCount, filesInError, sys.newLine)) :
777777
undefined;
778778
}
779779

src/harness/harnessIO.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ namespace Harness {
537537
outputLines += content;
538538
}
539539
if (pretty) {
540-
outputLines += ts.getErrorSummaryText(ts.getErrorCountForSummary(diagnostics), IO.newLine());
540+
outputLines += ts.getErrorSummaryText(ts.getErrorCountForSummary(diagnostics), ts.getFilesInErrorForSummary(diagnostics), IO.newLine());
541541
}
542542
return outputLines;
543543
}

src/testRunner/unittests/tsbuild/publicApi.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export function f22() { } // trailing`,
5454
/*createProgram*/ undefined,
5555
createDiagnosticReporter(sys, /*pretty*/ true),
5656
createBuilderStatusReporter(sys, /*pretty*/ true),
57-
errorCount => sys.write(getErrorSummaryText(errorCount, sys.newLine))
57+
(errorCount, filesInError) => sys.write(getErrorSummaryText(errorCount, filesInError, sys.newLine))
5858
);
5959
buildHost.afterProgramEmitAndDiagnostics = cb;
6060
buildHost.afterEmitBundle = cb;

src/testRunner/unittests/tscWatch/helpers.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,18 @@ namespace ts.tscWatch {
204204
assert.equal(host.exitCode, expectedExitCode);
205205
}
206206

207-
export function checkNormalBuildErrors(host: WatchedSystem, errors: readonly Diagnostic[] | readonly string[], reportErrorSummary?: boolean) {
207+
export function checkNormalBuildErrors(
208+
host: WatchedSystem,
209+
errors: readonly Diagnostic[] | readonly string[],
210+
files: readonly ReportFileInError[],
211+
reportErrorSummary?: boolean
212+
) {
208213
checkOutputErrors(
209214
host,
210215
[
211216
...map(errors, hostOutputDiagnostic),
212217
...reportErrorSummary ?
213-
[hostOutputWatchDiagnostic(getErrorSummaryText(errors.length, host.newLine))] :
218+
[hostOutputWatchDiagnostic(getErrorSummaryText(errors.length, files, host.newLine))] :
214219
emptyArray
215220
]
216221
);

tests/baselines/reference/api/tsserverlibrary.d.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -5352,7 +5352,11 @@ declare namespace ts {
53525352
traceResolution?: boolean;
53535353
[option: string]: CompilerOptionsValue | undefined;
53545354
}
5355-
type ReportEmitErrorSummary = (errorCount: number) => void;
5355+
type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
5356+
interface ReportFileInError {
5357+
fileName: string;
5358+
line: number;
5359+
}
53565360
interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
53575361
createDirectory?(path: string): void;
53585362
/**

tests/baselines/reference/api/typescript.d.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -5352,7 +5352,11 @@ declare namespace ts {
53525352
traceResolution?: boolean;
53535353
[option: string]: CompilerOptionsValue | undefined;
53545354
}
5355-
type ReportEmitErrorSummary = (errorCount: number) => void;
5355+
type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
5356+
interface ReportFileInError {
5357+
fileName: string;
5358+
line: number;
5359+
}
53565360
interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
53575361
createDirectory?(path: string): void;
53585362
/**

tests/baselines/reference/deeplyNestedAssignabilityIssue.errors.txt

+3-1
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,7 @@
6565
}
6666
}
6767

68-
Found 2 errors.
68+
Found 2 errors in 1 file.
6969

70+
Errors Files
71+
2 tests/cases/compiler/deeplyNestedAssignabilityIssue.ts:22

tests/baselines/reference/duplicateIdentifierRelatedSpans1.errors.txt

+5-1
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,9 @@
9292
!!! error TS2451: Cannot redeclare block-scoped variable 'Bar'.
9393
!!! related TS6203 tests/cases/compiler/file1.ts:2:7: 'Bar' was also declared here.
9494

95-
Found 6 errors.
95+
Found 6 errors in 3 files.
9696

97+
Errors Files
98+
2 tests/cases/compiler/file1.ts:1
99+
2 tests/cases/compiler/file2.ts:1
100+
2 tests/cases/compiler/file3.ts:1

tests/baselines/reference/duplicateIdentifierRelatedSpans2.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,8 @@
4545
class H { }
4646
class I { }
4747

48-
Found 2 errors.
48+
Found 2 errors in 2 files.
4949

50+
Errors Files
51+
1 tests/cases/compiler/file1.ts:1
52+
1 tests/cases/compiler/file2.ts:1

tests/baselines/reference/duplicateIdentifierRelatedSpans3.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,8 @@
8585
!!! related TS6203 tests/cases/compiler/file1.ts:4:5: 'duplicate3' was also declared here.
8686
}
8787

88-
Found 6 errors.
88+
Found 6 errors in 2 files.
8989

90+
Errors Files
91+
3 tests/cases/compiler/file1.ts:2
92+
3 tests/cases/compiler/file2.ts:2

tests/baselines/reference/duplicateIdentifierRelatedSpans4.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,8 @@
4747
duplicate8(): number;
4848
}
4949

50-
Found 2 errors.
50+
Found 2 errors in 2 files.
5151

52+
Errors Files
53+
1 tests/cases/compiler/file1.ts:1
54+
1 tests/cases/compiler/file2.ts:1

tests/baselines/reference/duplicateIdentifierRelatedSpans5.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,8 @@
9292
}
9393
export {}
9494

95-
Found 6 errors.
95+
Found 6 errors in 2 files.
9696

97+
Errors Files
98+
3 tests/cases/compiler/file1.ts:3
99+
3 tests/cases/compiler/file2.ts:4

tests/baselines/reference/duplicateIdentifierRelatedSpans6.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,8 @@
9292
!!! related TS6203 tests/cases/compiler/file2.ts:7:9: 'duplicate3' was also declared here.
9393
}
9494
}
95-
Found 6 errors.
95+
Found 6 errors in 2 files.
9696

97+
Errors Files
98+
3 tests/cases/compiler/file1.ts:3
99+
3 tests/cases/compiler/file2.ts:5

tests/baselines/reference/duplicateIdentifierRelatedSpans7.errors.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,8 @@
5656
duplicate9: () => string;
5757
}
5858
}
59-
Found 2 errors.
59+
Found 2 errors in 2 files.
6060

61+
Errors Files
62+
1 tests/cases/compiler/file1.ts:1
63+
1 tests/cases/compiler/file2.ts:3

tests/baselines/reference/esModuleInteropPrettyErrorRelatedInformation.errors.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@
2323
!!! error TS2345: Type '{ default: () => void; }' provides no match for the signature '(): void'.
2424
!!! related TS7038 tests/cases/compiler/index.ts:1:1: Type originates at this import. A namespace-style import cannot be called or constructed, and will cause a failure at runtime. Consider using a default import or import require here instead.
2525

26-
Found 1 error.
26+
Found 1 error in tests/cases/compiler/index.ts:3
2727

0 commit comments

Comments
 (0)