From e0d067b48f71e545c9bef49ad5fceb0a2a17a0ea Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 28 Feb 2018 15:16:44 -0800 Subject: [PATCH 1/6] Added an errors summary for --pretty --watch results Reports a "Found X error(s) across Y file(s)." message on each recompile if in both pretty and watch modes. This commit intentionally doesn't include tests as I have a few questions about implementation details. Will ask in PR first. --- src/compiler/diagnosticMessages.json | 12 +++++ src/compiler/tsc.ts | 3 +- src/compiler/watch.ts | 72 ++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 9801da54a5f..edb2917f7eb 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3472,6 +3472,18 @@ "category": "Message", "code": 6190 }, + "Found 1 error in 1 file.": { + "category": "Message", + "code": 6191 + }, + "Found {0} errors in 1 file.": { + "category": "Message", + "code": 6192 + }, + "Found {0} errors in {1} files.": { + "category": "Message", + "code": 6193 + }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", "code": 7005 diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 15e4867d7f7..2b0008f19c6 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -144,7 +144,8 @@ namespace ts { enableStatistics(compilerOptions); const program = createProgram(rootFileNames, compilerOptions, compilerHost); - const exitStatus = emitFilesAndReportErrors(program, reportDiagnostic, s => sys.write(s + sys.newLine)); + const diagnosticsAndEmit = getProgramDiagnosticsAndEmit(program); + const exitStatus = reportDiagnosticErrors(program, diagnosticsAndEmit, reportDiagnostic, s => sys.write(s + sys.newLine)); reportStatistics(program); return sys.exit(exitStatus); } diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index ff21c750089..138ac628e6d 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -31,11 +31,18 @@ namespace ts { }; } + const nonClearingMessageCodes: number[] = [ + Diagnostics.Compilation_complete_Watching_for_file_changes.code, + Diagnostics.Found_1_error_in_1_file.code, + Diagnostics.Found_0_errors_in_1_file.code, + Diagnostics.Found_0_errors_in_1_files.code, + ]; + function clearScreenIfNotWatchingForFileChanges(system: System, diagnostic: Diagnostic, options: CompilerOptions) { if (system.clearScreen && - diagnostic.code !== Diagnostics.Compilation_complete_Watching_for_file_changes.code && !options.extendedDiagnostics && - !options.diagnostics) { + !options.diagnostics && + !contains(nonClearingMessageCodes, diagnostic.code)) { system.clearScreen(); } } @@ -120,10 +127,15 @@ namespace ts { emit(): EmitResult; } - /** - * Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options - */ - export function emitFilesAndReportErrors(program: ProgramToEmitFilesAndReportErrors, reportDiagnostic: DiagnosticReporter, writeFileName?: (s: string) => void) { + /** @internal */ + export interface ProgramDiagnosticsAndEmit { + diagnostics: Diagnostic[]; + emittedFiles: string[]; + emitSkipped: boolean; + } + + /** @internal */ + export function getProgramDiagnosticsAndEmit(program: ProgramToEmitFilesAndReportErrors): ProgramDiagnosticsAndEmit { // First get and report any syntactic errors. const diagnostics = program.getSyntacticDiagnostics().slice(); let reportSemanticDiagnostics = false; @@ -147,6 +159,15 @@ namespace ts { addRange(diagnostics, program.getSemanticDiagnostics()); } + return { diagnostics, emittedFiles, emitSkipped }; + } + + /** + * Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options + */ + export function reportDiagnosticErrors(program: ProgramToEmitFilesAndReportErrors, diagnosticsAndEmit: ProgramDiagnosticsAndEmit, reportDiagnostic: DiagnosticReporter, writeFileName?: (s: string) => void) { + const { diagnostics, emittedFiles, emitSkipped } = diagnosticsAndEmit; + sortAndDeduplicateDiagnostics(diagnostics).forEach(reportDiagnostic); if (writeFileName) { const currentDir = program.getCurrentDirectory(); @@ -174,6 +195,34 @@ namespace ts { return ExitStatus.Success; } + function summarizeDiagnosticsAcrossFiles(diagnostics: Diagnostic[], reporter: WatchStatusReporter, newLine: string, compilerOptions: CompilerOptions): void { + if (diagnostics.length === 1) { + reporter(createCompilerDiagnostic(Diagnostics.Found_1_error_in_1_file), newLine, compilerOptions); + return; + } + + const uniqueFileNamesCount = countUniqueDiagnosticFileNames(diagnostics); + + if (uniqueFileNamesCount === 1) { + reporter(createCompilerDiagnostic(Diagnostics.Found_0_errors_in_1_file, diagnostics.length), newLine, compilerOptions); + } + else if (uniqueFileNamesCount !== 0) { + reporter(createCompilerDiagnostic(Diagnostics.Found_0_errors_in_1_files, diagnostics.length, uniqueFileNamesCount), newLine, compilerOptions); + } + } + + function countUniqueDiagnosticFileNames(diagnostics: Diagnostic[]): number { + const fileNames = createMap(); + + for (const diagnostic of diagnostics) { + if (diagnostic.file) { + fileNames.set(diagnostic.file.fileName, true); + } + } + + return fileNames.size; + } + const noopFileWatcher: FileWatcher = { close: noop }; /** @@ -187,6 +236,7 @@ namespace ts { let host: DirectoryStructureHost = system; const useCaseSensitiveFileNames = () => system.useCaseSensitiveFileNames; const writeFileName = (s: string) => system.write(s + system.newLine); + const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system); return { useCaseSensitiveFileNames, getNewLine: () => system.newLine, @@ -205,7 +255,7 @@ namespace ts { setTimeout: system.setTimeout ? ((callback, ms, ...args: any[]) => system.setTimeout.call(system, callback, ms, ...args)) : noop, clearTimeout: system.clearTimeout ? (timeoutId => system.clearTimeout(timeoutId)) : noop, trace: s => system.write(s), - onWatchStatusChange: reportWatchStatus || createWatchStatusReporter(system), + onWatchStatusChange, createDirectory: path => system.createDirectory(path), writeFile: (path, data, writeByteOrderMark) => system.writeFile(path, data, writeByteOrderMark), onCachedDirectoryStructureHostCreate: cacheHost => host = cacheHost || system, @@ -219,7 +269,13 @@ namespace ts { } function emitFilesAndReportErrorUsingBuilder(builderProgram: BuilderProgram) { - emitFilesAndReportErrors(builderProgram, reportDiagnostic, writeFileName); + const diagnosticsAndEmit = getProgramDiagnosticsAndEmit(builderProgram); + reportDiagnosticErrors(builderProgram, diagnosticsAndEmit, reportDiagnostic, writeFileName); + + const compilerOptions = builderProgram.getCompilerOptions(); + if (compilerOptions.pretty) { + summarizeDiagnosticsAcrossFiles(diagnosticsAndEmit.diagnostics, onWatchStatusChange, system.newLine, compilerOptions); + } } } From 9cf553876330f9299a4c3c73337b7647d4dff5ef Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Thu, 8 Mar 2018 20:47:45 -0800 Subject: [PATCH 2/6] Simplified error message counts; internalized reporting to createWatchCompilerHost Instead of modifying the logic in both `tsc.ts`to `watch.ts` by splitting `emitFilesAndReportErrors` in two, this adds an optional function parameter to report them. Removes file counting for diagnostic messages, as they might not be coming from files. Next commit will include tests. --- src/compiler/diagnosticMessages.json | 8 +--- src/compiler/tsc.ts | 3 +- src/compiler/watch.ts | 69 ++++++++++------------------ 3 files changed, 27 insertions(+), 53 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index edb2917f7eb..dcb17d34723 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3472,18 +3472,14 @@ "category": "Message", "code": 6190 }, - "Found 1 error in 1 file.": { + "Found 1 error.": { "category": "Message", "code": 6191 }, - "Found {0} errors in 1 file.": { + "Found {0} errors.": { "category": "Message", "code": 6192 }, - "Found {0} errors in {1} files.": { - "category": "Message", - "code": 6193 - }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", "code": 7005 diff --git a/src/compiler/tsc.ts b/src/compiler/tsc.ts index 2b0008f19c6..15e4867d7f7 100644 --- a/src/compiler/tsc.ts +++ b/src/compiler/tsc.ts @@ -144,8 +144,7 @@ namespace ts { enableStatistics(compilerOptions); const program = createProgram(rootFileNames, compilerOptions, compilerHost); - const diagnosticsAndEmit = getProgramDiagnosticsAndEmit(program); - const exitStatus = reportDiagnosticErrors(program, diagnosticsAndEmit, reportDiagnostic, s => sys.write(s + sys.newLine)); + const exitStatus = emitFilesAndReportErrors(program, reportDiagnostic, s => sys.write(s + sys.newLine)); reportStatistics(program); return sys.exit(exitStatus); } diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 138ac628e6d..0382f17f9dc 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -33,9 +33,8 @@ namespace ts { const nonClearingMessageCodes: number[] = [ Diagnostics.Compilation_complete_Watching_for_file_changes.code, - Diagnostics.Found_1_error_in_1_file.code, - Diagnostics.Found_0_errors_in_1_file.code, - Diagnostics.Found_0_errors_in_1_files.code, + Diagnostics.Found_1_error.code, + Diagnostics.Found_0_errors.code ]; function clearScreenIfNotWatchingForFileChanges(system: System, diagnostic: Diagnostic, options: CompilerOptions) { @@ -135,7 +134,12 @@ namespace ts { } /** @internal */ - export function getProgramDiagnosticsAndEmit(program: ProgramToEmitFilesAndReportErrors): ProgramDiagnosticsAndEmit { + export type ReportEmitErrorSummary = (errorCount: number) => void; + + /** + * Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options + */ + export function emitFilesAndReportErrors(program: ProgramToEmitFilesAndReportErrors, reportDiagnostic: DiagnosticReporter, reportSummary?: ReportEmitErrorSummary, writeFileName?: (s: string) => void) { // First get and report any syntactic errors. const diagnostics = program.getSyntacticDiagnostics().slice(); let reportSemanticDiagnostics = false; @@ -159,15 +163,6 @@ namespace ts { addRange(diagnostics, program.getSemanticDiagnostics()); } - return { diagnostics, emittedFiles, emitSkipped }; - } - - /** - * Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options - */ - export function reportDiagnosticErrors(program: ProgramToEmitFilesAndReportErrors, diagnosticsAndEmit: ProgramDiagnosticsAndEmit, reportDiagnostic: DiagnosticReporter, writeFileName?: (s: string) => void) { - const { diagnostics, emittedFiles, emitSkipped } = diagnosticsAndEmit; - sortAndDeduplicateDiagnostics(diagnostics).forEach(reportDiagnostic); if (writeFileName) { const currentDir = program.getCurrentDirectory(); @@ -183,6 +178,10 @@ namespace ts { } } + if (reportSummary) { + reportSummary(diagnostics.filter(diagnostic => diagnostic.category === DiagnosticCategory.Error).length); + } + if (emitSkipped && diagnostics.length > 0) { // If the emitter didn't emit anything, then pass that value along. return ExitStatus.DiagnosticsPresent_OutputsSkipped; @@ -195,34 +194,6 @@ namespace ts { return ExitStatus.Success; } - function summarizeDiagnosticsAcrossFiles(diagnostics: Diagnostic[], reporter: WatchStatusReporter, newLine: string, compilerOptions: CompilerOptions): void { - if (diagnostics.length === 1) { - reporter(createCompilerDiagnostic(Diagnostics.Found_1_error_in_1_file), newLine, compilerOptions); - return; - } - - const uniqueFileNamesCount = countUniqueDiagnosticFileNames(diagnostics); - - if (uniqueFileNamesCount === 1) { - reporter(createCompilerDiagnostic(Diagnostics.Found_0_errors_in_1_file, diagnostics.length), newLine, compilerOptions); - } - else if (uniqueFileNamesCount !== 0) { - reporter(createCompilerDiagnostic(Diagnostics.Found_0_errors_in_1_files, diagnostics.length, uniqueFileNamesCount), newLine, compilerOptions); - } - } - - function countUniqueDiagnosticFileNames(diagnostics: Diagnostic[]): number { - const fileNames = createMap(); - - for (const diagnostic of diagnostics) { - if (diagnostic.file) { - fileNames.set(diagnostic.file.fileName, true); - } - } - - return fileNames.size; - } - const noopFileWatcher: FileWatcher = { close: noop }; /** @@ -269,13 +240,21 @@ namespace ts { } function emitFilesAndReportErrorUsingBuilder(builderProgram: BuilderProgram) { - const diagnosticsAndEmit = getProgramDiagnosticsAndEmit(builderProgram); - reportDiagnosticErrors(builderProgram, diagnosticsAndEmit, reportDiagnostic, writeFileName); - + let reportSummary: ReportEmitErrorSummary | undefined; const compilerOptions = builderProgram.getCompilerOptions(); + if (compilerOptions.pretty) { - summarizeDiagnosticsAcrossFiles(diagnosticsAndEmit.diagnostics, onWatchStatusChange, system.newLine, compilerOptions); + reportSummary = (errorCount: number) => { + if (errorCount === 1) { + onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_1_error, errorCount), sys.newLine, compilerOptions); + } + else { + onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_0_errors, errorCount, errorCount), sys.newLine, compilerOptions); + } + }; } + + emitFilesAndReportErrors(builderProgram, reportDiagnostic, reportSummary, writeFileName); } } From a4ca0716eb8806c010ef8bf20c8cd104d077fdb1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 9 Mar 2018 09:41:11 -0800 Subject: [PATCH 3/6] Reverted now-unused ProgramDiagnosticsAndEmit --- src/compiler/watch.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index aeeab45f9d4..ac46e28c42e 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -127,13 +127,6 @@ namespace ts { emit(): EmitResult; } - /** @internal */ - export interface ProgramDiagnosticsAndEmit { - diagnostics: Diagnostic[]; - emittedFiles: string[]; - emitSkipped: boolean; - } - /** @internal */ export type ReportEmitErrorSummary = (errorCount: number) => void; From 15b61dcd6b0d36b48805acfd47db16b5e1b602fb Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 9 Mar 2018 14:09:38 -0800 Subject: [PATCH 4/6] Made watch mode always report summary --- src/compiler/watch.ts | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index ac46e28c42e..550a15ba9a0 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -127,7 +127,6 @@ namespace ts { emit(): EmitResult; } - /** @internal */ export type ReportEmitErrorSummary = (errorCount: number) => void; /** @@ -234,19 +233,15 @@ namespace ts { } function emitFilesAndReportErrorUsingBuilder(builderProgram: BuilderProgram) { - let reportSummary: ReportEmitErrorSummary | undefined; const compilerOptions = builderProgram.getCompilerOptions(); - - if (compilerOptions.pretty) { - reportSummary = (errorCount: number) => { - if (errorCount === 1) { - onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_1_error, errorCount), sys.newLine, compilerOptions); - } - else { - onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_0_errors, errorCount, errorCount), sys.newLine, compilerOptions); - } - }; - } + const reportSummary = (errorCount: number) => { + if (errorCount === 1) { + onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_1_error, errorCount), sys.newLine, compilerOptions); + } + else { + onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_0_errors, errorCount, errorCount), sys.newLine, compilerOptions); + } + }; emitFilesAndReportErrors(builderProgram, reportDiagnostic, reportSummary, writeFileName); } From cf0a0ec0011fe53507fbc40baf8b4d42bd0f63e1 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 1 Apr 2018 22:57:38 -0700 Subject: [PATCH 5/6] Fixed unit tests for error counts --- src/harness/unittests/tscWatchMode.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index 8d9938bf8f5..9965489ca71 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -84,7 +84,7 @@ namespace ts.tscWatch { ) { let screenClears = 0; const outputs = host.getOutput(); - const expectedOutputCount = 2 + errors.length + postErrorsWatchDiagnostics.length + + const expectedOutputCount = 1 + errors.length + postErrorsWatchDiagnostics.length + (logsBeforeWatchDiagnostic ? logsBeforeWatchDiagnostic.length : 0) + (logsBeforeErrors ? logsBeforeErrors.length : 0); assert.equal(outputs.length, expectedOutputCount, JSON.stringify(outputs)); let index = 0; From 0dbebec28f2ff4b92fa809ec58571a7d10038a08 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 4 Apr 2018 13:21:19 -0400 Subject: [PATCH 6/6] Feedback: correct order in watch.ts; DRY tests --- src/compiler/watch.ts | 4 ++-- src/harness/unittests/tscWatchMode.ts | 14 ++++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 530eb4d31e0..f53f01e3eb2 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -127,7 +127,7 @@ namespace ts { /** * Helper that emit files, report diagnostics and lists emitted and/or source files depending on compiler options */ - export function emitFilesAndReportErrors(program: ProgramToEmitFilesAndReportErrors, reportDiagnostic: DiagnosticReporter, reportSummary?: ReportEmitErrorSummary, writeFileName?: (s: string) => void) { + export function emitFilesAndReportErrors(program: ProgramToEmitFilesAndReportErrors, reportDiagnostic: DiagnosticReporter, writeFileName?: (s: string) => void, reportSummary?: ReportEmitErrorSummary) { // First get and report any syntactic errors. const diagnostics = program.getConfigFileParsingDiagnostics().slice(); const configFileParsingDiagnosticsLength = diagnostics.length; @@ -242,7 +242,7 @@ namespace ts { } }; - emitFilesAndReportErrors(builderProgram, reportDiagnostic, reportSummary, writeFileName); + emitFilesAndReportErrors(builderProgram, reportDiagnostic, writeFileName, reportSummary); } } diff --git a/src/harness/unittests/tscWatchMode.ts b/src/harness/unittests/tscWatchMode.ts index 9965489ca71..99bad3e3091 100644 --- a/src/harness/unittests/tscWatchMode.ts +++ b/src/harness/unittests/tscWatchMode.ts @@ -128,6 +128,12 @@ namespace ts.tscWatch { } } + function createErrorsFoundCompilerDiagnostic(errors: ReadonlyArray) { + return errors.length === 1 + ? createCompilerDiagnostic(Diagnostics.Found_1_error) + : createCompilerDiagnostic(Diagnostics.Found_0_errors, errors.length); + } + function checkOutputErrorsInitial(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeErrors?: string[]) { checkOutputErrors( host, @@ -136,9 +142,7 @@ namespace ts.tscWatch { logsBeforeErrors, errors, disableConsoleClears, - errors.length === 1 - ? createCompilerDiagnostic(Diagnostics.Found_1_error) - : createCompilerDiagnostic(Diagnostics.Found_0_errors, errors.length), + createErrorsFoundCompilerDiagnostic(errors), createCompilerDiagnostic(Diagnostics.Compilation_complete_Watching_for_file_changes)); } @@ -150,9 +154,7 @@ namespace ts.tscWatch { logsBeforeErrors, errors, disableConsoleClears, - errors.length === 1 - ? createCompilerDiagnostic(Diagnostics.Found_1_error) - : createCompilerDiagnostic(Diagnostics.Found_0_errors, errors.length), + createErrorsFoundCompilerDiagnostic(errors), createCompilerDiagnostic(Diagnostics.Compilation_complete_Watching_for_file_changes)); }