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
This commit is contained in:
parent
13c2fad240
commit
9fa2c398e7
|
@ -240,17 +240,8 @@ function printDiagnostics(
|
||||||
if (diagnostics.length === 0) {
|
if (diagnostics.length === 0) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const formatHost = getFormatDiagnosticsHost(options);
|
const formatHost = getFormatDiagnosticsHost(options);
|
||||||
if (options && options.enableIvy !== false) {
|
consoleError(formatDiagnostics(diagnostics, formatHost));
|
||||||
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));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// CLI entry point
|
// CLI entry point
|
||||||
|
|
|
@ -8,7 +8,10 @@
|
||||||
|
|
||||||
import {Position, isSyntaxError} from '@angular/compiler';
|
import {Position, isSyntaxError} from '@angular/compiler';
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {AbsoluteFsPath, absoluteFrom, getFileSystem, relative, resolve} from '../src/ngtsc/file_system';
|
import {AbsoluteFsPath, absoluteFrom, getFileSystem, relative, resolve} from '../src/ngtsc/file_system';
|
||||||
|
|
||||||
|
import {replaceTsWithNgInErrors} from './ngtsc/diagnostics';
|
||||||
import * as api from './transformers/api';
|
import * as api from './transformers/api';
|
||||||
import * as ng from './transformers/entry_points';
|
import * as ng from './transformers/entry_points';
|
||||||
import {createMessageDiagnostic} from './transformers/util';
|
import {createMessageDiagnostic} from './transformers/util';
|
||||||
|
@ -94,7 +97,8 @@ export function formatDiagnostics(
|
||||||
return diags
|
return diags
|
||||||
.map(diagnostic => {
|
.map(diagnostic => {
|
||||||
if (api.isTsDiagnostic(diagnostic)) {
|
if (api.isTsDiagnostic(diagnostic)) {
|
||||||
return ts.formatDiagnostics([diagnostic], host);
|
return replaceTsWithNgInErrors(
|
||||||
|
ts.formatDiagnosticsWithColorAndContext([diagnostic], host));
|
||||||
} else {
|
} else {
|
||||||
return formatDiagnostic(diagnostic, host);
|
return formatDiagnostic(diagnostic, host);
|
||||||
}
|
}
|
||||||
|
|
|
@ -11,7 +11,7 @@ import * as path from 'path';
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {main, readCommandLineAndConfiguration, watchMode} from '../src/main';
|
import {main, readCommandLineAndConfiguration, watchMode} from '../src/main';
|
||||||
import {setup} from './test_support';
|
import {setup, stripAnsi} from './test_support';
|
||||||
|
|
||||||
describe('ngc transformer command-line', () => {
|
describe('ngc transformer command-line', () => {
|
||||||
let basePath: string;
|
let basePath: string;
|
||||||
|
@ -89,8 +89,9 @@ describe('ngc transformer command-line', () => {
|
||||||
errorSpy.and.stub();
|
errorSpy.and.stub();
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
const exitCode = main(['-p', basePath], errorSpy);
|
||||||
expect(errorSpy).toHaveBeenCalledWith(
|
const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]);
|
||||||
`test.ts(1,1): error TS1128: Declaration or statement expected.\r\n`);
|
expect(errorText).toContain(
|
||||||
|
`test.ts:1:1 - error TS1128: Declaration or statement expected.\r\n`);
|
||||||
expect(exitCode).toBe(1);
|
expect(exitCode).toBe(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -105,7 +106,8 @@ describe('ngc transformer command-line', () => {
|
||||||
}`);
|
}`);
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
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.` +
|
`error TS6053: File '` + path.posix.join(basePath, 'test.ts') + `' not found.` +
|
||||||
'\n');
|
'\n');
|
||||||
expect(exitCode).toEqual(1);
|
expect(exitCode).toEqual(1);
|
||||||
|
@ -116,8 +118,9 @@ describe('ngc transformer command-line', () => {
|
||||||
write('test.ts', 'foo;');
|
write('test.ts', 'foo;');
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
const exitCode = main(['-p', basePath], errorSpy);
|
||||||
expect(errorSpy).toHaveBeenCalledWith(
|
const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]);
|
||||||
`test.ts(1,1): error TS2304: Cannot find name 'foo'.` +
|
expect(errorText).toContain(
|
||||||
|
`test.ts:1:1 - error TS2304: Cannot find name 'foo'.` +
|
||||||
'\n');
|
'\n');
|
||||||
expect(exitCode).toEqual(1);
|
expect(exitCode).toEqual(1);
|
||||||
});
|
});
|
||||||
|
@ -127,8 +130,9 @@ describe('ngc transformer command-line', () => {
|
||||||
write('test.ts', `import {MyClass} from './not-exist-deps';`);
|
write('test.ts', `import {MyClass} from './not-exist-deps';`);
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
const exitCode = main(['-p', basePath], errorSpy);
|
||||||
expect(errorSpy).toHaveBeenCalledWith(
|
const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]);
|
||||||
`test.ts(1,23): error TS2307: Cannot find module './not-exist-deps'.` +
|
expect(errorText).toContain(
|
||||||
|
`test.ts:1:23 - error TS2307: Cannot find module './not-exist-deps'.` +
|
||||||
'\n');
|
'\n');
|
||||||
expect(exitCode).toEqual(1);
|
expect(exitCode).toEqual(1);
|
||||||
});
|
});
|
||||||
|
@ -139,8 +143,9 @@ describe('ngc transformer command-line', () => {
|
||||||
write('test.ts', `import {MyClass} from './empty-deps';`);
|
write('test.ts', `import {MyClass} from './empty-deps';`);
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
const exitCode = main(['-p', basePath], errorSpy);
|
||||||
expect(errorSpy).toHaveBeenCalledWith(
|
const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]);
|
||||||
`test.ts(1,9): error TS2305: Module '"./empty-deps"' has no exported member 'MyClass'.\n`);
|
expect(errorText).toContain(
|
||||||
|
`test.ts:1:9 - error TS2305: Module '"./empty-deps"' has no exported member 'MyClass'.\n`);
|
||||||
expect(exitCode).toEqual(1);
|
expect(exitCode).toEqual(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -153,8 +158,9 @@ describe('ngc transformer command-line', () => {
|
||||||
`);
|
`);
|
||||||
|
|
||||||
const exitCode = main(['-p', basePath], errorSpy);
|
const exitCode = main(['-p', basePath], errorSpy);
|
||||||
expect(errorSpy).toHaveBeenCalledWith(
|
const errorText = stripAnsi(errorSpy.calls.mostRecent().args[0]);
|
||||||
'test.ts(3,9): error TS2349: This expression is not callable.\n' +
|
expect(errorText).toContain(
|
||||||
|
'test.ts:3:9 - error TS2349: This expression is not callable.\n' +
|
||||||
' Type \'String\' has no call signatures.\n');
|
' Type \'String\' has no call signatures.\n');
|
||||||
expect(exitCode).toEqual(1);
|
expect(exitCode).toEqual(1);
|
||||||
});
|
});
|
||||||
|
|
|
@ -171,3 +171,9 @@ export function expectNoDiagnosticsInProgram(options: ng.CompilerOptions, p: ng.
|
||||||
export function normalizeSeparators(path: string): string {
|
export function normalizeSeparators(path: string): string {
|
||||||
return path.replace(/\\/g, '/');
|
return path.replace(/\\/g, '/');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const STRIP_ANSI = /\x1B\x5B\d+m/g;
|
||||||
|
|
||||||
|
export function stripAnsi(diags: string): string {
|
||||||
|
return diags.replace(STRIP_ANSI, '');
|
||||||
|
}
|
||||||
|
|
|
@ -14,7 +14,7 @@ import {formatDiagnostics} from '../../src/perform_compile';
|
||||||
import {CompilerHost, EmitFlags, LazyRoute} from '../../src/transformers/api';
|
import {CompilerHost, EmitFlags, LazyRoute} from '../../src/transformers/api';
|
||||||
import {createSrcToOutPathMapper} from '../../src/transformers/program';
|
import {createSrcToOutPathMapper} from '../../src/transformers/program';
|
||||||
import {StructureIsReused, tsStructureIsReused} from '../../src/transformers/util';
|
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', () => {
|
describe('ng program', () => {
|
||||||
let testSupport: TestSupport;
|
let testSupport: TestSupport;
|
||||||
|
@ -986,11 +986,11 @@ describe('ng program', () => {
|
||||||
{rootNames: [path.resolve(testSupport.basePath, 'src/main.ts')], options, host});
|
{rootNames: [path.resolve(testSupport.basePath, 'src/main.ts')], options, host});
|
||||||
const errorDiags =
|
const errorDiags =
|
||||||
program1.emit().diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
|
program1.emit().diagnostics.filter(d => d.category === ts.DiagnosticCategory.Error);
|
||||||
expect(formatDiagnostics(errorDiags))
|
expect(stripAnsi(formatDiagnostics(errorDiags)))
|
||||||
.toContain(`src/main.ts(5,13): error TS2322: Type '1' is not assignable to type 'string'.`);
|
.toContain(`src/main.ts:5:13 - error TS2322: Type '1' is not assignable to type 'string'.`);
|
||||||
expect(formatDiagnostics(errorDiags))
|
expect(stripAnsi(formatDiagnostics(errorDiags)))
|
||||||
.toContain(
|
.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', () => {
|
it('should not report emit errors with noEmitOnError=false', () => {
|
||||||
|
|
Loading…
Reference in New Issue