From f7471eea3c09dff38c57acf0286cd01b455243e8 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 4 Aug 2019 19:20:38 +0200 Subject: [PATCH] fix(ngcc): handle compilation diagnostics (#31996) Previously, any diagnostics reported during the compilation of an entry-point would not be shown to the user, but either be ignored or cause a hard crash in case of a `FatalDiagnosticError`. This is unfortunate, as such error instances contain information on which code was responsible for producing the error, whereas only its error message would not. Therefore, it was quite hard to determine where the error originates from. This commit introduces behavior to deal with error diagnostics in a more graceful way. Such diagnostics will still cause the compilation to fail, however the error message now contains formatted diagnostics. Closes #31977 Resolves FW-1374 PR Close #31996 --- .../ngcc/src/analysis/decoration_analyzer.ts | 8 ++++- .../compiler-cli/ngcc/src/analysis/util.ts | 19 +++++++--- packages/compiler-cli/ngcc/src/main.ts | 16 +++++++-- .../ngcc/src/packages/transformer.ts | 33 +++++++++++++---- .../ngcc/test/integration/ngcc_spec.ts | 35 +++++++++++++++++++ 5 files changed, 97 insertions(+), 14 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index b8b50f21bfe5b..5705636e3c331 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -130,7 +130,13 @@ export class DecorationAnalyzer { protected analyzeClass(symbol: ClassSymbol): AnalyzedClass|null { const decorators = this.reflectionHost.getDecoratorsOfSymbol(symbol); - return analyzeDecorators(symbol, decorators, this.handlers); + const analyzedClass = analyzeDecorators(symbol, decorators, this.handlers); + if (analyzedClass !== null && analyzedClass.diagnostics !== undefined) { + for (const diagnostic of analyzedClass.diagnostics) { + this.diagnosticHandler(diagnostic); + } + } + return analyzedClass; } protected migrateFile(migrationHost: MigrationHost, analyzedFile: AnalyzedFile): void { diff --git a/packages/compiler-cli/ngcc/src/analysis/util.ts b/packages/compiler-cli/ngcc/src/analysis/util.ts index 63e854a720fae..b617199ed7815 100644 --- a/packages/compiler-cli/ngcc/src/analysis/util.ts +++ b/packages/compiler-cli/ngcc/src/analysis/util.ts @@ -6,9 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; + +import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics'; import {AbsoluteFsPath, absoluteFromSourceFile, relative} from '../../../src/ngtsc/file_system'; import {ClassSymbol, Decorator} from '../../../src/ngtsc/reflection'; import {DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform'; + import {AnalyzedClass, MatchingHandler} from './types'; export function isWithinPackage(packagePath: AbsoluteFsPath, sourceFile: ts.SourceFile): boolean { @@ -59,11 +62,19 @@ export function analyzeDecorators( const matches: {handler: DecoratorHandler, analysis: any}[] = []; const allDiagnostics: ts.Diagnostic[] = []; for (const {handler, detected} of detections) { - const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata); - if (diagnostics !== undefined) { - allDiagnostics.push(...diagnostics); + try { + const {analysis, diagnostics} = handler.analyze(declaration, detected.metadata); + if (diagnostics !== undefined) { + allDiagnostics.push(...diagnostics); + } + matches.push({handler, analysis}); + } catch (e) { + if (isFatalDiagnosticError(e)) { + allDiagnostics.push(e.toDiagnostic()); + } else { + throw e; + } } - matches.push({handler, analysis}); } return { name: symbol.name, diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 408e8f4067073..679bdfb8f326c 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -5,6 +5,8 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import * as ts from 'typescript'; + import {AbsoluteFsPath, FileSystem, absoluteFrom, dirname, getFileSystem, resolve} from '../../src/ngtsc/file_system'; import {CommonJsDependencyHost} from './dependencies/commonjs_dependency_host'; @@ -27,7 +29,6 @@ import {FileWriter} from './writing/file_writer'; import {InPlaceFileWriter} from './writing/in_place_file_writer'; import {NewEntryPointFileWriter} from './writing/new_entry_point_file_writer'; - /** * The options to configure the ngcc compiler. */ @@ -171,8 +172,17 @@ export function mainNgcc( logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`); - const transformedFiles = transformer.transform(bundle); - fileWriter.writeBundle(bundle, transformedFiles, formatPropertiesToMarkAsProcessed); + const result = transformer.transform(bundle); + if (result.success) { + if (result.diagnostics.length > 0) { + logger.warn(ts.formatDiagnostics(result.diagnostics, bundle.src.host)); + } + fileWriter.writeBundle(bundle, result.transformedFiles, formatPropertiesToMarkAsProcessed); + } else { + const errors = ts.formatDiagnostics(result.diagnostics, bundle.src.host); + throw new Error( + `Failed to compile entry-point ${entryPoint.name} due to compilation errors:\n${errors}`); + } onTaskCompleted(task, TaskProcessingOutcome.Processed); }; diff --git a/packages/compiler-cli/ngcc/src/packages/transformer.ts b/packages/compiler-cli/ngcc/src/packages/transformer.ts index 11bbc4900a1c1..34480ad3e72e8 100644 --- a/packages/compiler-cli/ngcc/src/packages/transformer.ts +++ b/packages/compiler-cli/ngcc/src/packages/transformer.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; + import {FileSystem} from '../../../src/ngtsc/file_system'; import {DecorationAnalyzer} from '../analysis/decoration_analyzer'; import {ModuleWithProvidersAnalyses, ModuleWithProvidersAnalyzer} from '../analysis/module_with_providers_analyzer'; @@ -27,8 +28,17 @@ import {Renderer} from '../rendering/renderer'; import {RenderingFormatter} from '../rendering/rendering_formatter'; import {UmdRenderingFormatter} from '../rendering/umd_rendering_formatter'; import {FileToWrite} from '../rendering/utils'; + import {EntryPointBundle} from './entry_point_bundle'; +export type TransformResult = { + success: true; diagnostics: ts.Diagnostic[]; transformedFiles: FileToWrite[]; +} | +{ + success: false; + diagnostics: ts.Diagnostic[]; +}; + /** * A Package is stored in a directory on disk and that directory can contain one or more package * formats - e.g. fesm2015, UMD, etc. Additionally, each package provides typings (`.d.ts` files). @@ -58,12 +68,17 @@ export class Transformer { * @param bundle the bundle to transform. * @returns information about the files that were transformed. */ - transform(bundle: EntryPointBundle): FileToWrite[] { + transform(bundle: EntryPointBundle): TransformResult { const reflectionHost = this.getHost(bundle); // Parse and analyze the files. const {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, - moduleWithProvidersAnalyses} = this.analyzeProgram(reflectionHost, bundle); + moduleWithProvidersAnalyses, diagnostics} = this.analyzeProgram(reflectionHost, bundle); + + // Bail if the analysis produced any errors. + if (hasErrors(diagnostics)) { + return {success: false, diagnostics}; + } // Transform the source files and source maps. const srcFormatter = this.getRenderingFormatter(reflectionHost, bundle); @@ -81,7 +96,7 @@ export class Transformer { renderedFiles = renderedFiles.concat(renderedDtsFiles); } - return renderedFiles; + return {success: true, diagnostics, transformedFiles: renderedFiles}; } getHost(bundle: EntryPointBundle): NgccReflectionHost { @@ -127,8 +142,10 @@ export class Transformer { new SwitchMarkerAnalyzer(reflectionHost, bundle.entryPoint.package); const switchMarkerAnalyses = switchMarkerAnalyzer.analyzeProgram(bundle.src.program); - const decorationAnalyzer = - new DecorationAnalyzer(this.fs, bundle, reflectionHost, referencesRegistry); + const diagnostics: ts.Diagnostic[] = []; + const decorationAnalyzer = new DecorationAnalyzer( + this.fs, bundle, reflectionHost, referencesRegistry, + diagnostic => diagnostics.push(diagnostic)); const decorationAnalyses = decorationAnalyzer.analyzeProgram(); const moduleWithProvidersAnalyzer = @@ -142,14 +159,18 @@ export class Transformer { privateDeclarationsAnalyzer.analyzeProgram(bundle.src.program); return {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses, - moduleWithProvidersAnalyses}; + moduleWithProvidersAnalyses, diagnostics}; } } +export function hasErrors(diagnostics: ts.Diagnostic[]) { + return diagnostics.some(d => d.category === ts.DiagnosticCategory.Error); +} interface ProgramAnalyses { decorationAnalyses: Map; switchMarkerAnalyses: SwitchMarkerAnalyses; privateDeclarationsAnalyses: ExportInfo[]; moduleWithProvidersAnalyses: ModuleWithProvidersAnalyses|null; + diagnostics: ts.Diagnostic[]; } diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 3600befbb16f4..aa789fb06967e 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -406,6 +406,41 @@ runInEachFileSystem(() => { }); }); + describe('diagnostics', () => { + it('should fail with formatted diagnostics when an error diagnostic is produced', () => { + loadTestFiles([ + { + name: _('/node_modules/fatal-error/package.json'), + contents: '{"name": "fatal-error", "es2015": "./index.js", "typings": "./index.d.ts"}', + }, + {name: _('/node_modules/fatal-error/index.metadata.json'), contents: 'DUMMY DATA'}, + { + name: _('/node_modules/fatal-error/index.js'), + contents: ` + import {Component} from '@angular/core'; + export class FatalError {} + FatalError.decorators = [ + {type: Component, args: [{selector: 'fatal-error'}]} + ]; + `, + }, + { + name: _('/node_modules/fatal-error/index.d.ts'), + contents: ` + export declare class FatalError {} + `, + }, + ]); + expect(() => mainNgcc({ + basePath: '/node_modules', + targetEntryPointPath: 'fatal-error', + propertiesToConsider: ['es2015'] + })) + .toThrowError( + /^Failed to compile entry-point fatal-error due to compilation errors:\nnode_modules\/fatal-error\/index\.js\(5,17\): error TS-992001: component is missing a template\r?\n$/); + }); + }); + describe('logger', () => { it('should log info message to the console by default', () => { const consoleInfoSpy = spyOn(console, 'info');