fix(compiler): translate emit diagnostics with `noEmitOnError: true`. (#19953)

This prevents errors reported against `.ngfactory.ts` files show up
as the result of running `ngc`.

Closes #19935
PR Close #19953
This commit is contained in:
Tobias Bosch 2017-10-26 09:45:01 -07:00 committed by Matias Niemelä
parent a869aeecd2
commit 18e9d86a3b
7 changed files with 116 additions and 20 deletions

View File

@ -38,10 +38,10 @@ export function translateDiagnostics(host: TypeCheckHost, untranslatedDiagnostic
source: SOURCE,
code: DEFAULT_ERROR_CODE
});
return;
}
} else {
ts.push(diagnostic);
}
ts.push(diagnostic);
});
return {ts, ng};
}

View File

@ -18,7 +18,7 @@ import {CompilerHost, CompilerOptions, CustomTransformers, DEFAULT_ERROR_CODE, D
import {CodeGenerator, TsCompilerAotCompilerTypeCheckHostAdapter, getOriginalReferences} from './compiler_host';
import {LowerMetadataCache, getExpressionLoweringTransformFactory} from './lower_expressions';
import {getAngularEmitterTransformFactory} from './node_emitter_transform';
import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, tsStructureIsReused} from './util';
import {GENERATED_FILES, StructureIsReused, createMessageDiagnostic, isInRootDir, ngToTsDiagnostic, tsStructureIsReused} from './util';
@ -149,11 +149,9 @@ class AngularCompilerProgram implements Program {
getTsSemanticDiagnostics(sourceFile?: ts.SourceFile, cancellationToken?: ts.CancellationToken):
ts.Diagnostic[] {
if (sourceFile) {
return this.tsProgram.getSemanticDiagnostics(sourceFile, cancellationToken);
}
const sourceFiles = sourceFile ? [sourceFile] : this.tsProgram.getSourceFiles();
let diags: ts.Diagnostic[] = [];
this.tsProgram.getSourceFiles().forEach(sf => {
sourceFiles.forEach(sf => {
if (!GENERATED_FILES.test(sf.fileName)) {
diags.push(...this.tsProgram.getSemanticDiagnostics(sf, cancellationToken));
}
@ -300,6 +298,10 @@ class AngularCompilerProgram implements Program {
}
}
this.emittedSourceFiles = emittedSourceFiles;
// translate the diagnostics in the emitResult as well.
const translatedEmitDiags = translateDiagnostics(this.hostAdapter, emitResult.diagnostics);
emitResult.diagnostics = translatedEmitDiags.ts.concat(
this.structuralDiagnostics.concat(translatedEmitDiags.ng).map(ngToTsDiagnostic));
if (!outSrcMapping.length) {
// if no files were emitted by TypeScript, also don't emit .json files

View File

@ -51,3 +51,29 @@ function pathStartsWithPrefix(prefix: string, fullPath: string): string|null {
const rel = path.relative(prefix, fullPath);
return rel.startsWith('..') ? null : rel;
}
/**
* Converts a ng.Diagnostic into a ts.Diagnostic.
* This looses some information, and also uses an incomplete object as `file`.
*
* I.e. only use this where the API allows only a ts.Diagnostic.
*/
export function ngToTsDiagnostic(ng: Diagnostic): ts.Diagnostic {
let file: ts.SourceFile|undefined;
let start: number|undefined;
let length: number|undefined;
if (ng.span) {
// Note: We can't use a real ts.SourceFile,
// but we can at least mirror the properties `fileName` and `text`, which
// are mostly used for error reporting.
file = { fileName: ng.span.start.file.url, text: ng.span.start.file.content } as ts.SourceFile;
start = ng.span.start.offset;
length = ng.span.end.offset - start;
}
return {
file,
messageText: ng.messageText,
category: ng.category,
code: ng.code, start, length,
};
}

View File

@ -1467,5 +1467,37 @@ describe('ngc transformer command-line', () => {
expect(exitCode).toBe(2, 'Compile was expected to fail');
expect(messages[0]).toContain(['Tagged template expressions are not supported in metadata']);
});
it('should allow using 2 classes with the same name in declarations with noEmitOnError=true',
() => {
write('src/tsconfig.json', `{
"extends": "../tsconfig-base.json",
"compilerOptions": {
"noEmitOnError": true
},
"files": ["test-module.ts"]
}`);
function writeComp(fileName: string) {
write(fileName, `
import {Component} from '@angular/core';
@Component({selector: 'comp', template: ''})
export class TestComponent {}
`);
}
writeComp('src/comp1.ts');
writeComp('src/comp2.ts');
write('src/test-module.ts', `
import {NgModule} from '@angular/core';
import {TestComponent as Comp1} from './comp1';
import {TestComponent as Comp2} from './comp2';
@NgModule({
declarations: [Comp1, Comp2],
})
export class MyModule {}
`);
expect(main(['-p', path.join(basePath, 'src/tsconfig.json')])).toBe(0);
});
});
});

View File

@ -11,6 +11,7 @@ import * as fs from 'fs';
import * as path from 'path';
import * as ts from 'typescript';
import {formatDiagnostics} from '../../src/perform_compile';
import {CompilerHost, EmitFlags, LazyRoute} from '../../src/transformers/api';
import {createSrcToOutPathMapper} from '../../src/transformers/program';
import {GENERATED_FILES, StructureIsReused, tsStructureIsReused} from '../../src/transformers/util';
@ -858,4 +859,34 @@ describe('ng program', () => {
}]);
});
});
it('should report errors for ts and ng errors on emit with noEmitOnError=true', () => {
testSupport.writeFiles({
'src/main.ts': `
import {Component, NgModule} from '@angular/core';
// Ts error
let x: string = 1;
// Ng error
@Component({selector: 'comp', templateUrl: './main.html'})
export class MyComp {}
@NgModule({declarations: [MyComp]})
export class MyModule {}
`,
'src/main.html': '{{nonExistent}}'
});
const options = testSupport.createCompilerOptions({noEmitOnError: true});
const host = ng.createCompilerHost({options});
const program1 = ng.createProgram(
{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))
.toContain(
`src/main.html(1,1): error TS100: Property 'nonExistent' does not exist on type 'MyComp'.`);
});
});

View File

@ -193,6 +193,7 @@ export class AotCompiler {
private _createNgFactoryStub(
outputCtx: OutputContext, file: NgAnalyzedFile, emitFlags: StubEmitFlags) {
let componentId = 0;
file.ngModules.forEach((ngModuleMeta, ngModuleIndex) => {
// Note: the code below needs to executed for StubEmitFlags.Basic and StubEmitFlags.TypeCheck,
// so we don't change the .ngfactory file too much when adding the typecheck block.
@ -230,12 +231,14 @@ export class AotCompiler {
if (!compMeta.isComponent) {
return;
}
componentId++;
this._createTypeCheckBlock(
outputCtx, ngModuleMeta, this._metadataResolver.getHostComponentMetadata(compMeta),
[compMeta.type], externalReferenceVars);
this._createTypeCheckBlock(
outputCtx, ngModuleMeta, compMeta, ngModuleMeta.transitiveModule.directives,
outputCtx, `${compMeta.type.reference.name}_Host_${componentId}`, ngModuleMeta,
this._metadataResolver.getHostComponentMetadata(compMeta), [compMeta.type],
externalReferenceVars);
this._createTypeCheckBlock(
outputCtx, `${compMeta.type.reference.name}_${componentId}`, ngModuleMeta, compMeta,
ngModuleMeta.transitiveModule.directives, externalReferenceVars);
});
}
});
@ -246,12 +249,13 @@ export class AotCompiler {
}
private _createTypeCheckBlock(
ctx: OutputContext, moduleMeta: CompileNgModuleMetadata, compMeta: CompileDirectiveMetadata,
directives: CompileIdentifierMetadata[], externalReferenceVars: Map<any, string>) {
ctx: OutputContext, componentId: string, moduleMeta: CompileNgModuleMetadata,
compMeta: CompileDirectiveMetadata, directives: CompileIdentifierMetadata[],
externalReferenceVars: Map<any, string>) {
const {template: parsedTemplate, pipes: usedPipes} =
this._parseTemplate(compMeta, moduleMeta, directives);
ctx.statements.push(...this._typeCheckCompiler.compileComponent(
compMeta, parsedTemplate, usedPipes, externalReferenceVars));
componentId, compMeta, parsedTemplate, usedPipes, externalReferenceVars));
}
emitMessageBundle(analyzeResult: NgAnalyzedModules, locale: string|null): MessageBundle {

View File

@ -9,7 +9,7 @@
import {AotCompilerOptions} from '../aot/compiler_options';
import {StaticReflector} from '../aot/static_reflector';
import {StaticSymbol} from '../aot/static_symbol';
import {CompileDiDependencyMetadata, CompileDirectiveMetadata, CompilePipeSummary, viewClassName} from '../compile_metadata';
import {CompileDiDependencyMetadata, CompileDirectiveMetadata, CompilePipeSummary} from '../compile_metadata';
import {BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter';
import {AST, ASTWithSource, Interpolation} from '../expression_parser/ast';
import {Identifiers} from '../identifiers';
@ -33,7 +33,8 @@ export class TypeCheckCompiler {
* and also violate the point above.
*/
compileComponent(
component: CompileDirectiveMetadata, template: TemplateAst[], usedPipes: CompilePipeSummary[],
componentId: string, component: CompileDirectiveMetadata, template: TemplateAst[],
usedPipes: CompilePipeSummary[],
externalReferenceVars: Map<StaticSymbol, string>): o.Statement[] {
const pipes = new Map<string, StaticSymbol>();
usedPipes.forEach(p => pipes.set(p.name, p.type.reference));
@ -48,7 +49,7 @@ export class TypeCheckCompiler {
const visitor = viewBuilderFactory(null);
visitor.visitAll([], template);
return visitor.build();
return visitor.build(componentId);
}
}
@ -103,8 +104,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
templateVisitAll(this, astNodes);
}
build(targetStatements: o.Statement[] = []): o.Statement[] {
this.children.forEach((child) => child.build(targetStatements));
build(componentId: string, targetStatements: o.Statement[] = []): o.Statement[] {
this.children.forEach((child) => child.build(componentId, targetStatements));
const viewStmts: o.Statement[] =
[o.variable(DYNAMIC_VAR_NAME).set(o.NULL_EXPR).toDeclStmt(o.DYNAMIC_TYPE)];
let bindingCount = 0;
@ -128,7 +129,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
(stmt: o.Statement) => o.applySourceSpanToStatementIfNeeded(stmt, sourceSpan)));
});
const viewName = `_View_${this.component.name}_${this.embeddedViewIndex}`;
const viewName = `_View_${componentId}_${this.embeddedViewIndex}`;
const viewFactory = new o.DeclareFunctionStmt(viewName, [], viewStmts);
targetStatements.push(viewFactory);
return targetStatements;