From 9fa2c398e7f2968a8d280f476647f39f4efbdd1a Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 4 Dec 2019 10:44:32 -0800 Subject: [PATCH] fix(compiler): switch to modern diagnostic formatting (#34234) The compiler exports a `formatDiagnostics` function which consumers can use to print both ts and ng diagnostics. However, this function was previously using the "old" style TypeScript diagnostics, as opposed to the modern diagnostic printer which uses terminal colors and prints additional context information. This commit updates `formatDiagnostics` to use the modern formatter, plus to update Ivy's negative error codes to Angular 'NG' errors. The Angular CLI needs a little more work to use this function for printing TS diagnostics, but this commit alone should fix Bazel builds as ngc-wrapped goes through `formatDiagnostics`. PR Close #34234 --- packages/compiler-cli/src/main.ts | 11 +------ packages/compiler-cli/src/perform_compile.ts | 6 +++- packages/compiler-cli/test/ngc_spec.ts | 30 +++++++++++-------- packages/compiler-cli/test/test_support.ts | 6 ++++ .../test/transformers/program_spec.ts | 10 +++---- 5 files changed, 35 insertions(+), 28 deletions(-) diff --git a/packages/compiler-cli/src/main.ts b/packages/compiler-cli/src/main.ts index 1d42fa6b27..64f03358fb 100644 --- a/packages/compiler-cli/src/main.ts +++ b/packages/compiler-cli/src/main.ts @@ -240,17 +240,8 @@ function printDiagnostics( if (diagnostics.length === 0) { return; } - const formatHost = getFormatDiagnosticsHost(options); - if (options && options.enableIvy !== false) { - const ngDiagnostics = diagnostics.filter(api.isNgDiagnostic); - const tsDiagnostics = diagnostics.filter(api.isTsDiagnostic); - consoleError(replaceTsWithNgInErrors( - ts.formatDiagnosticsWithColorAndContext(tsDiagnostics, formatHost))); - consoleError(formatDiagnostics(ngDiagnostics, formatHost)); - } else { - consoleError(formatDiagnostics(diagnostics, formatHost)); - } + consoleError(formatDiagnostics(diagnostics, formatHost)); } // CLI entry point diff --git a/packages/compiler-cli/src/perform_compile.ts b/packages/compiler-cli/src/perform_compile.ts index 800f13cdd7..b746e6ced5 100644 --- a/packages/compiler-cli/src/perform_compile.ts +++ b/packages/compiler-cli/src/perform_compile.ts @@ -8,7 +8,10 @@ import {Position, isSyntaxError} from '@angular/compiler'; import * as ts from 'typescript'; + import {AbsoluteFsPath, absoluteFrom, getFileSystem, relative, resolve} from '../src/ngtsc/file_system'; + +import {replaceTsWithNgInErrors} from './ngtsc/diagnostics'; import * as api from './transformers/api'; import * as ng from './transformers/entry_points'; import {createMessageDiagnostic} from './transformers/util'; @@ -94,7 +97,8 @@ export function formatDiagnostics( return diags .map(diagnostic => { if (api.isTsDiagnostic(diagnostic)) { - return ts.formatDiagnostics([diagnostic], host); + return replaceTsWithNgInErrors( + ts.formatDiagnosticsWithColorAndContext([diagnostic], host)); } else { return formatDiagnostic(diagnostic, host); } diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index e1529e0098..d972374208 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -11,7 +11,7 @@ import * as path from 'path'; import * as ts from 'typescript'; import {main, readCommandLineAndConfiguration, watchMode} from '../src/main'; -import {setup} from './test_support'; +import {setup, stripAnsi} from './test_support'; describe('ngc transformer command-line', () => { let basePath: string; @@ -89,8 +89,9 @@ describe('ngc transformer command-line', () => { errorSpy.and.stub(); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( - `test.ts(1,1): error TS1128: Declaration or statement expected.\r\n`); + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( + `test.ts:1:1 - error TS1128: Declaration or statement expected.\r\n`); expect(exitCode).toBe(1); }); @@ -105,7 +106,8 @@ describe('ngc transformer command-line', () => { }`); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( `error TS6053: File '` + path.posix.join(basePath, 'test.ts') + `' not found.` + '\n'); expect(exitCode).toEqual(1); @@ -116,8 +118,9 @@ describe('ngc transformer command-line', () => { write('test.ts', 'foo;'); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( - `test.ts(1,1): error TS2304: Cannot find name 'foo'.` + + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( + `test.ts:1:1 - error TS2304: Cannot find name 'foo'.` + '\n'); expect(exitCode).toEqual(1); }); @@ -127,8 +130,9 @@ describe('ngc transformer command-line', () => { write('test.ts', `import {MyClass} from './not-exist-deps';`); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( - `test.ts(1,23): error TS2307: Cannot find module './not-exist-deps'.` + + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( + `test.ts:1:23 - error TS2307: Cannot find module './not-exist-deps'.` + '\n'); expect(exitCode).toEqual(1); }); @@ -139,8 +143,9 @@ describe('ngc transformer command-line', () => { write('test.ts', `import {MyClass} from './empty-deps';`); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( - `test.ts(1,9): error TS2305: Module '"./empty-deps"' has no exported member 'MyClass'.\n`); + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( + `test.ts:1:9 - error TS2305: Module '"./empty-deps"' has no exported member 'MyClass'.\n`); expect(exitCode).toEqual(1); }); @@ -153,8 +158,9 @@ describe('ngc transformer command-line', () => { `); const exitCode = main(['-p', basePath], errorSpy); - expect(errorSpy).toHaveBeenCalledWith( - 'test.ts(3,9): error TS2349: This expression is not callable.\n' + + const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]); + expect(errorText).toContain( + 'test.ts:3:9 - error TS2349: This expression is not callable.\n' + ' Type \'String\' has no call signatures.\n'); expect(exitCode).toEqual(1); }); diff --git a/packages/compiler-cli/test/test_support.ts b/packages/compiler-cli/test/test_support.ts index 98e4f4207a..84f07874af 100644 --- a/packages/compiler-cli/test/test_support.ts +++ b/packages/compiler-cli/test/test_support.ts @@ -171,3 +171,9 @@ export function expectNoDiagnosticsInProgram(options: ng.CompilerOptions, p: ng. export function normalizeSeparators(path: string): string { return path.replace(/\\/g, '/'); } + +const STRIP_ANSI = /\x1B\x5B\d+m/g; + +export function stripAnsi(diags: string): string { + return diags.replace(STRIP_ANSI, ''); +} diff --git a/packages/compiler-cli/test/transformers/program_spec.ts b/packages/compiler-cli/test/transformers/program_spec.ts index 55842da44a..a701354005 100644 --- a/packages/compiler-cli/test/transformers/program_spec.ts +++ b/packages/compiler-cli/test/transformers/program_spec.ts @@ -14,7 +14,7 @@ import {formatDiagnostics} from '../../src/perform_compile'; import {CompilerHost, EmitFlags, LazyRoute} from '../../src/transformers/api'; import {createSrcToOutPathMapper} from '../../src/transformers/program'; import {StructureIsReused, tsStructureIsReused} from '../../src/transformers/util'; -import {TestSupport, expectNoDiagnosticsInProgram, setup} from '../test_support'; +import {TestSupport, expectNoDiagnosticsInProgram, setup, stripAnsi} from '../test_support'; describe('ng program', () => { let testSupport: TestSupport; @@ -986,11 +986,11 @@ describe('ng program', () => { {rootNames: [path.resolve(testSupport.basePath, 'src/main.ts')], options, host}); const errorDiags = program1.emit().diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error); - expect(formatDiagnostics(errorDiags)) - .toContain(`src/main.ts(5,13): error TS2322: Type '1' is not assignable to type 'string'.`); - expect(formatDiagnostics(errorDiags)) + expect(stripAnsi(formatDiagnostics(errorDiags))) + .toContain(`src/main.ts:5:13 - error TS2322: Type '1' is not assignable to type 'string'.`); + expect(stripAnsi(formatDiagnostics(errorDiags))) .toContain( - `src/main.html(1,1): error TS100: Property 'nonExistent' does not exist on type 'MyComp'.`); + `src/main.html:1:1 - error TS100: Property 'nonExistent' does not exist on type 'MyComp'.`); }); it('should not report emit errors with noEmitOnError=false', () => {