fix(45713) Improve error report summaries (#45742)

* Improve error report summaries (#45713) * fixup! Improve error report summaries (#45713) * fixup! fixup! Improve error report summaries (#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 <git@orta.io> Co-authored-by: Orta Therox <ortam@microsoft.com>
microsoft · Dec 7, 2021 · 7a12909 · 7a12909
1 parent 305842b
commit 7a12909
Show file tree

Hide file tree

Showing 50 changed files with 575 additions and 98 deletions.
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -4156,6 +4156,12 @@
         "category": "Message",
         "code": 6040
     },
+    "Errors  Files": {
+        "_locale_notes": "There is a double space, and the order cannot be changed (they're table headings) ^",
+        "category": "Message",
+        "code": 6041
+    },
+
     "Generates corresponding '.map' file.": {
         "category": "Message",
         "code": 6043
@@ -4935,6 +4941,18 @@
         "category": "Error",
         "code": 6258
     },
+      "Found 1 error in {1}": {
+        "category": "Message",
+        "code": 6259
+    },
+    "Found {0} errors in 1 file.": {
+        "category": "Message",
+        "code": 6260
+    },
+    "Found {0} errors in {1} files.": {
+        "category": "Message",
+        "code": 6261
+    },
 
     "Directory '{0}' has no containing package.json scope. Imports will not resolve.": {
         "category": "Message",

diff --git a/src/compiler/tsbuildPublic.ts b/src/compiler/tsbuildPublic.ts
@@ -76,7 +76,12 @@ namespace ts {
         return fileExtensionIs(fileName, Extension.Dts);
     }
 
-    export type ReportEmitErrorSummary = (errorCount: number) => void;
+    export type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
+
+    export interface ReportFileInError {
+        fileName: string;
+        line: number;
+    }
 
     export interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
         createDirectory?(path: string): void;
@@ -2003,10 +2008,12 @@ namespace ts {
         const canReportSummary = state.watch || !!state.host.reportErrorSummary;
         const { diagnostics } = state;
         let totalErrors = 0;
+        let filesInError: (ReportFileInError | undefined)[] = [];
         if (isCircularBuildOrder(buildOrder)) {
             reportBuildQueue(state, buildOrder.buildOrder);
             reportErrors(state, buildOrder.circularDiagnostics);
             if (canReportSummary) totalErrors += getErrorCountForSummary(buildOrder.circularDiagnostics);
+            if (canReportSummary) filesInError = [...filesInError, ...getFilesInErrorForSummary(buildOrder.circularDiagnostics)];
         }
         else {
             // Report errors from the other projects
@@ -2017,13 +2024,14 @@ namespace ts {
                 }
             });
             if (canReportSummary) diagnostics.forEach(singleProjectErrors => totalErrors += getErrorCountForSummary(singleProjectErrors));
+            if (canReportSummary) diagnostics.forEach(singleProjectErrors => [...filesInError, ...getFilesInErrorForSummary(singleProjectErrors)]);
         }
 
         if (state.watch) {
             reportWatchStatus(state, getWatchErrorSummaryDiagnosticMessage(totalErrors), totalErrors);
         }
         else if (state.host.reportErrorSummary) {
-            state.host.reportErrorSummary(totalErrors);
+            state.host.reportErrorSummary(totalErrors, filesInError);
         }
     }
 

diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts
@@ -7,7 +7,7 @@ namespace ts {
     } : undefined;
 
     /**
-     * Create a function that reports error by writing to the system and handles the formating of the diagnostic
+     * Create a function that reports error by writing to the system and handles the formatting of the diagnostic
      */
     export function createDiagnosticReporter(system: System, pretty?: boolean): DiagnosticReporter {
         const host: FormatDiagnosticsHost = system === sys && sysFormatDiagnosticsHost ? sysFormatDiagnosticsHost : {
@@ -101,16 +101,87 @@ namespace ts {
         return countWhere(diagnostics, diagnostic => diagnostic.category === DiagnosticCategory.Error);
     }
 
+    export function getFilesInErrorForSummary(diagnostics: readonly Diagnostic[]): (ReportFileInError | undefined)[] {
+        const filesInError =
+            filter(diagnostics, diagnostic => diagnostic.category === DiagnosticCategory.Error)
+            .map(
+                errorDiagnostic => {
+                    if(errorDiagnostic.file === undefined) return;
+                    return `${errorDiagnostic.file.fileName}`;
+            });
+        return filesInError.map((fileName: string) => {
+            const diagnosticForFileName = find(diagnostics, diagnostic =>
+                diagnostic.file !== undefined && diagnostic.file.fileName === fileName
+            );
+
+            if(diagnosticForFileName !== undefined) {
+                const { line } = getLineAndCharacterOfPosition(diagnosticForFileName.file!, diagnosticForFileName.start!);
+                return {
+                    fileName,
+                    line: line + 1,
+                };
+            }
+        });
+    }
+
     export function getWatchErrorSummaryDiagnosticMessage(errorCount: number) {
         return errorCount === 1 ?
             Diagnostics.Found_1_error_Watching_for_file_changes :
             Diagnostics.Found_0_errors_Watching_for_file_changes;
     }
 
-    export function getErrorSummaryText(errorCount: number, newLine: string) {
+    export function getErrorSummaryText(
+        errorCount: number,
+        filesInError: readonly (ReportFileInError | undefined)[],
+        newLine: string
+    ) {
         if (errorCount === 0) return "";
-        const d = createCompilerDiagnostic(errorCount === 1 ? Diagnostics.Found_1_error : Diagnostics.Found_0_errors, errorCount);
-        return `${newLine}${flattenDiagnosticMessageText(d.messageText, newLine)}${newLine}${newLine}`;
+        const nonNilFiles = filesInError.filter(fileInError => fileInError !== undefined);
+        const distinctFileNamesWithLines = nonNilFiles.map(fileInError => `${fileInError!.fileName}:${fileInError!.line}`)
+            .filter((value, index, self) => self.indexOf(value) === index);
+        const d = errorCount === 1 ?
+            createCompilerDiagnostic(
+                filesInError[0] !== undefined ?
+                    Diagnostics.Found_1_error_in_1 :
+                    Diagnostics.Found_1_error,
+                errorCount,
+                distinctFileNamesWithLines[0]) :
+            createCompilerDiagnostic(
+                distinctFileNamesWithLines.length === 0 ?
+                    Diagnostics.Found_0_errors :
+                    distinctFileNamesWithLines.length === 1 ?
+                        Diagnostics.Found_0_errors_in_1_file :
+                        Diagnostics.Found_0_errors_in_1_files,
+                errorCount,
+                distinctFileNamesWithLines.length);
+        return `${newLine}${flattenDiagnosticMessageText(d.messageText, newLine)}${newLine}${newLine}${errorCount > 1 ? createTabularErrorsDisplay(nonNilFiles) : ""}`;
+    }
+
+    function createTabularErrorsDisplay(filesInError: (ReportFileInError | undefined)[]) {
+        const distinctFiles = filesInError.filter((value, index, self) => index === self.findIndex(file => file?.fileName === value?.fileName));
+        if (distinctFiles.length === 0) return "";
+
+        const numberLength = (num: number) => Math.log(num) * Math.LOG10E + 1;
+        const fileToErrorCount = distinctFiles.map(file => ([file, countWhere(filesInError, fileInError => fileInError!.fileName === file!.fileName)] as const));
+        const maxErrors = fileToErrorCount.reduce((acc, value) => Math.max(acc, value[1] || 0), 0);
+
+        const headerRow = Diagnostics.Errors_Files.message;
+        const leftColumnHeadingLength = headerRow.split(" ")[0].length;
+        const leftPaddingGoal = Math.max(leftColumnHeadingLength, numberLength(maxErrors));
+        const headerPadding = Math.max(numberLength(maxErrors) - leftColumnHeadingLength, 0);
+
+        let tabularData = "";
+        tabularData += " ".repeat(headerPadding) + headerRow + "\n";
+        fileToErrorCount.forEach((row) => {
+            const [file, errorCount] = row;
+            const errorCountDigitsLength = Math.log(errorCount) * Math.LOG10E + 1 | 0;
+            const leftPadding = errorCountDigitsLength < leftPaddingGoal ?
+                " ".repeat(leftPaddingGoal - errorCountDigitsLength)
+                : "";
+            tabularData += `${leftPadding}${errorCount}  ${file!.fileName}:${file!.line}\n`;
+        });
+
+        return tabularData;
     }
 
     export function isBuilderProgram(program: Program | BuilderProgram): program is BuilderProgram {
@@ -350,7 +421,7 @@ namespace ts {
         }
 
         if (reportSummary) {
-            reportSummary(getErrorCountForSummary(diagnostics));
+            reportSummary(getErrorCountForSummary(diagnostics), getFilesInErrorForSummary(diagnostics));
         }
 
         return {
@@ -656,7 +727,7 @@ namespace ts {
             builderProgram,
             input.reportDiagnostic || createDiagnosticReporter(system),
             s => host.trace && host.trace(s),
-            input.reportErrorSummary || input.options.pretty ? errorCount => system.write(getErrorSummaryText(errorCount, system.newLine)) : undefined
+            input.reportErrorSummary || input.options.pretty ? (errorCount, filesInError) => system.write(getErrorSummaryText(errorCount, filesInError, system.newLine)) : undefined
         );
         if (input.afterProgramEmitAndDiagnostics) input.afterProgramEmitAndDiagnostics(builderProgram);
         return exitStatus;

diff --git a/src/executeCommandLine/executeCommandLine.ts b/src/executeCommandLine/executeCommandLine.ts
@@ -773,7 +773,7 @@ namespace ts {
 
     function createReportErrorSummary(sys: System, options: CompilerOptions | BuildOptions): ReportEmitErrorSummary | undefined {
         return shouldBePretty(sys, options) ?
-            errorCount => sys.write(getErrorSummaryText(errorCount, sys.newLine)) :
+            (errorCount, filesInError) => sys.write(getErrorSummaryText(errorCount, filesInError, sys.newLine)) :
             undefined;
     }
 

diff --git a/src/harness/harnessIO.ts b/src/harness/harnessIO.ts
@@ -537,7 +537,7 @@ namespace Harness {
                 outputLines += content;
             }
             if (pretty) {
-                outputLines += ts.getErrorSummaryText(ts.getErrorCountForSummary(diagnostics), IO.newLine());
+                outputLines += ts.getErrorSummaryText(ts.getErrorCountForSummary(diagnostics), ts.getFilesInErrorForSummary(diagnostics), IO.newLine());
             }
             return outputLines;
         }

diff --git a/src/testRunner/unittests/tsbuild/publicApi.ts b/src/testRunner/unittests/tsbuild/publicApi.ts
@@ -54,7 +54,7 @@ export function f22() { } // trailing`,
                     /*createProgram*/ undefined,
                 createDiagnosticReporter(sys, /*pretty*/ true),
                 createBuilderStatusReporter(sys, /*pretty*/ true),
-                errorCount => sys.write(getErrorSummaryText(errorCount, sys.newLine))
+                (errorCount, filesInError) => sys.write(getErrorSummaryText(errorCount, filesInError, sys.newLine))
             );
             buildHost.afterProgramEmitAndDiagnostics = cb;
             buildHost.afterEmitBundle = cb;

diff --git a/src/testRunner/unittests/tscWatch/helpers.ts b/src/testRunner/unittests/tscWatch/helpers.ts
@@ -204,13 +204,18 @@ namespace ts.tscWatch {
         assert.equal(host.exitCode, expectedExitCode);
     }
 
-    export function checkNormalBuildErrors(host: WatchedSystem, errors: readonly Diagnostic[] | readonly string[], reportErrorSummary?: boolean) {
+    export function checkNormalBuildErrors(
+        host: WatchedSystem,
+        errors: readonly Diagnostic[] | readonly string[],
+        files: readonly ReportFileInError[],
+        reportErrorSummary?: boolean
+    ) {
         checkOutputErrors(
             host,
             [
                 ...map(errors, hostOutputDiagnostic),
                 ...reportErrorSummary ?
-                    [hostOutputWatchDiagnostic(getErrorSummaryText(errors.length, host.newLine))] :
+                    [hostOutputWatchDiagnostic(getErrorSummaryText(errors.length, files, host.newLine))] :
                     emptyArray
             ]
         );

diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -5352,7 +5352,11 @@ declare namespace ts {
         traceResolution?: boolean;
         [option: string]: CompilerOptionsValue | undefined;
     }
-    type ReportEmitErrorSummary = (errorCount: number) => void;
+    type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
+    interface ReportFileInError {
+        fileName: string;
+        line: number;
+    }
     interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
         createDirectory?(path: string): void;
         /**

diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -5352,7 +5352,11 @@ declare namespace ts {
         traceResolution?: boolean;
         [option: string]: CompilerOptionsValue | undefined;
     }
-    type ReportEmitErrorSummary = (errorCount: number) => void;
+    type ReportEmitErrorSummary = (errorCount: number, filesInError: (ReportFileInError | undefined)[]) => void;
+    interface ReportFileInError {
+        fileName: string;
+        line: number;
+    }
     interface SolutionBuilderHostBase<T extends BuilderProgram> extends ProgramHost<T> {
         createDirectory?(path: string): void;
         /**

diff --git a/tests/baselines/reference/deeplyNestedAssignabilityIssue.errors.txt b/tests/baselines/reference/deeplyNestedAssignabilityIssue.errors.txt
@@ -65,5 +65,7 @@
         }
     }
 
-Found 2 errors.
+Found 2 errors in 1 file.
 
+Errors  Files
+     2  tests/cases/compiler/deeplyNestedAssignabilityIssue.ts:22
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans1.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans1.errors.txt
@@ -92,5 +92,9 @@
 !!! error TS2451: Cannot redeclare block-scoped variable 'Bar'.
 !!! related TS6203 tests/cases/compiler/file1.ts:2:7: 'Bar' was also declared here.
 
-Found 6 errors.
+Found 6 errors in 3 files.
 
+Errors  Files
+     2  tests/cases/compiler/file1.ts:1
+     2  tests/cases/compiler/file2.ts:1
+     2  tests/cases/compiler/file3.ts:1
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans2.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans2.errors.txt
@@ -45,5 +45,8 @@
     class H { }
     class I { }
 
-Found 2 errors.
+Found 2 errors in 2 files.
 
+Errors  Files
+     1  tests/cases/compiler/file1.ts:1
+     1  tests/cases/compiler/file2.ts:1
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans3.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans3.errors.txt
@@ -85,5 +85,8 @@
 !!! related TS6203 tests/cases/compiler/file1.ts:4:5: 'duplicate3' was also declared here.
     }
 
-Found 6 errors.
+Found 6 errors in 2 files.
 
+Errors  Files
+     3  tests/cases/compiler/file1.ts:2
+     3  tests/cases/compiler/file2.ts:2
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans4.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans4.errors.txt
@@ -47,5 +47,8 @@
         duplicate8(): number;
     }
 
-Found 2 errors.
+Found 2 errors in 2 files.
 
+Errors  Files
+     1  tests/cases/compiler/file1.ts:1
+     1  tests/cases/compiler/file2.ts:1
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans5.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans5.errors.txt
@@ -92,5 +92,8 @@
     }
     export {}
 
-Found 6 errors.
+Found 6 errors in 2 files.
 
+Errors  Files
+     3  tests/cases/compiler/file1.ts:3
+     3  tests/cases/compiler/file2.ts:4
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans6.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans6.errors.txt
@@ -92,5 +92,8 @@
 !!! related TS6203 tests/cases/compiler/file2.ts:7:9: 'duplicate3' was also declared here.
         }
     }
-Found 6 errors.
+Found 6 errors in 2 files.
 
+Errors  Files
+     3  tests/cases/compiler/file1.ts:3
+     3  tests/cases/compiler/file2.ts:5
diff --git a/tests/baselines/reference/duplicateIdentifierRelatedSpans7.errors.txt b/tests/baselines/reference/duplicateIdentifierRelatedSpans7.errors.txt
@@ -56,5 +56,8 @@
             duplicate9: () => string;
         }
     }
-Found 2 errors.
+Found 2 errors in 2 files.
 
+Errors  Files
+     1  tests/cases/compiler/file1.ts:1
+     1  tests/cases/compiler/file2.ts:3
diff --git a/tests/baselines/reference/esModuleInteropPrettyErrorRelatedInformation.errors.txt b/tests/baselines/reference/esModuleInteropPrettyErrorRelatedInformation.errors.txt
@@ -23,5 +23,5 @@
 !!! error TS2345:   Type '{ default: () => void; }' provides no match for the signature '(): void'.
 !!! 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.
 
-Found 1 error.
+Found 1 error in tests/cases/compiler/index.ts:3