Skip to content

Commit

Permalink
fix(ivy): ngcc - handle compilation diagnostics
Browse files Browse the repository at this point in the history
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 angular#31977
Resolves FW-1374
  • Loading branch information
JoostK committed Aug 4, 2019
1 parent 2a6e6c0 commit 00035a1
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 15 deletions.
Expand Up @@ -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 {
Expand Down
19 changes: 15 additions & 4 deletions packages/compiler-cli/ngcc/src/analysis/util.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -59,11 +62,19 @@ export function analyzeDecorators(
const matches: {handler: DecoratorHandler<any, any>, 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,
Expand Down
24 changes: 24 additions & 0 deletions packages/compiler-cli/ngcc/src/diagnostics.ts
@@ -0,0 +1,24 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* 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 {getFileSystem} from '../../src/ngtsc/file_system';

const defaultFormatHost: ts.FormatDiagnosticsHost = {
getCurrentDirectory: () => getFileSystem().pwd(),
getCanonicalFileName: fileName => fileName,
getNewLine: () => ts.sys.newLine,
};

export function hasErrors(diagnostics: ts.Diagnostic[]) {
return diagnostics.some(d => d.category === ts.DiagnosticCategory.Error);
}

export function formatDiagnostics(diagnostics: ts.Diagnostic[]): string {
return ts.formatDiagnostics(diagnostics, defaultFormatHost);
}
18 changes: 15 additions & 3 deletions packages/compiler-cli/ngcc/src/main.ts
Expand Up @@ -6,11 +6,13 @@
* found in the LICENSE file at https://angular.io/license
*/
import {AbsoluteFsPath, FileSystem, absoluteFrom, dirname, getFileSystem, resolve} from '../../src/ngtsc/file_system';

import {CommonJsDependencyHost} from './dependencies/commonjs_dependency_host';
import {DependencyResolver, InvalidEntryPoint, SortedEntryPointsInfo} from './dependencies/dependency_resolver';
import {EsmDependencyHost} from './dependencies/esm_dependency_host';
import {ModuleResolver} from './dependencies/module_resolver';
import {UmdDependencyHost} from './dependencies/umd_dependency_host';
import {formatDiagnostics} from './diagnostics';
import {DirectoryWalkerEntryPointFinder} from './entry_point_finder/directory_walker_entry_point_finder';
import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_point_finder';
import {ConsoleLogger, LogLevel} from './logging/console_logger';
Expand All @@ -25,6 +27,7 @@ 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.
*/
Expand Down Expand Up @@ -135,9 +138,18 @@ export function mainNgcc(
true);
if (bundle) {
logger.info(`Compiling ${entryPoint.name} : ${property} as ${format}`);
const transformedFiles = transformer.transform(bundle);
fileWriter.writeBundle(entryPoint, bundle, transformedFiles);
compiledFormats.add(formatPath);
const result = transformer.transform(bundle);
if (result.success) {
if (result.diagnostics.length > 0) {
logger.warn(formatDiagnostics(result.diagnostics));
}
fileWriter.writeBundle(entryPoint, bundle, result.transformedFiles);
compiledFormats.add(formatPath);
} else {
const errors = formatDiagnostics(result.diagnostics);
throw new Error(
`Failed to compile entry-point ${entryPoint.name} due to compilation errors:\n${errors}`);
}
} else {
logger.warn(
`Skipping ${entryPoint.name} : ${format} (no valid entry point file for this format).`);
Expand Down
32 changes: 25 additions & 7 deletions packages/compiler-cli/ngcc/src/packages/transformer.ts
Expand Up @@ -6,13 +6,15 @@
* 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';
import {NgccReferencesRegistry} from '../analysis/ngcc_references_registry';
import {ExportInfo, PrivateDeclarationsAnalyzer} from '../analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyses, SwitchMarkerAnalyzer} from '../analysis/switch_marker_analyzer';
import {CompiledFile} from '../analysis/types';
import {hasErrors} from '../diagnostics';
import {CommonJsReflectionHost} from '../host/commonjs_host';
import {Esm2015ReflectionHost} from '../host/esm2015_host';
import {Esm5ReflectionHost} from '../host/esm5_host';
Expand All @@ -27,8 +29,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).
Expand Down Expand Up @@ -58,12 +69,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);
Expand All @@ -81,7 +97,7 @@ export class Transformer {
renderedFiles = renderedFiles.concat(renderedDtsFiles);
}

return renderedFiles;
return {success: true, diagnostics, transformedFiles: renderedFiles};
}

getHost(bundle: EntryPointBundle): NgccReflectionHost {
Expand Down Expand Up @@ -127,8 +143,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 =
Expand All @@ -142,14 +160,14 @@ export class Transformer {
privateDeclarationsAnalyzer.analyzeProgram(bundle.src.program);

return {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses,
moduleWithProvidersAnalyses};
moduleWithProvidersAnalyses, diagnostics};
}
}


interface ProgramAnalyses {
decorationAnalyses: Map<ts.SourceFile, CompiledFile>;
switchMarkerAnalyses: SwitchMarkerAnalyses;
privateDeclarationsAnalyses: ExportInfo[];
moduleWithProvidersAnalyses: ModuleWithProvidersAnalyses|null;
diagnostics: ts.Diagnostic[];
}
36 changes: 36 additions & 0 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -315,6 +315,42 @@ 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:\n' +
'node_modules/fatal-error/index.js(5,17): error TS-992001: component is missing a template\n');
});
});

describe('logger', () => {
it('should log info message to the console by default', () => {
const consoleInfoSpy = spyOn(console, 'info');
Expand Down

0 comments on commit 00035a1

Please sign in to comment.