From d91ff17adca0e38444712adf20ceaa2351a7baab Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 11 Dec 2017 08:50:46 -0800 Subject: [PATCH] fix(compiler): generate the correct imports for summary type-check Summaries should be ignored when importing the types used in a type-check block. --- packages/compiler-cli/test/test_support.ts | 10 ++- packages/compiler/src/aot/compiler.ts | 64 ++++++++++--------- .../src/aot/static_symbol_resolver.ts | 8 +-- packages/compiler/src/util.ts | 2 +- .../test/aot/static_symbol_resolver_spec.ts | 19 ++++++ 5 files changed, 64 insertions(+), 39 deletions(-) diff --git a/packages/compiler-cli/test/test_support.ts b/packages/compiler-cli/test/test_support.ts index c64be8b91f..35cf8661b7 100644 --- a/packages/compiler-cli/test/test_support.ts +++ b/packages/compiler-cli/test/test_support.ts @@ -12,6 +12,7 @@ import * as path from 'path'; import * as ts from 'typescript'; import * as ng from '../index'; +// TEST_TMPDIR is set by bazel. const tmpdir = process.env.TEST_TMPDIR || os.tmpdir(); function getNgRootDir() { @@ -21,7 +22,6 @@ function getNgRootDir() { } export function writeTempFile(name: string, contents: string): string { - // TEST_TMPDIR is set by bazel. const id = (Math.random() * 1000000).toFixed(0); const fn = path.join(tmpdir, `tmp.${id}.${name}`); fs.writeFileSync(fn, contents); @@ -29,8 +29,12 @@ export function writeTempFile(name: string, contents: string): string { } export function makeTempDir(): string { - const id = (Math.random() * 1000000).toFixed(0); - const dir = path.join(tmpdir, `tmp.${id}`); + let dir: string; + while (true) { + const id = (Math.random() * 1000000).toFixed(0); + dir = path.join(tmpdir, `tmp.${id}`); + if (!fs.existsSync(dir)) break; + } fs.mkdirSync(dir); return dir; } diff --git a/packages/compiler/src/aot/compiler.ts b/packages/compiler/src/aot/compiler.ts index e2c11de6a1..499c66e966 100644 --- a/packages/compiler/src/aot/compiler.ts +++ b/packages/compiler/src/aot/compiler.ts @@ -218,15 +218,14 @@ export class AotCompiler { const externalReferenceVars = new Map(); externalReferences.forEach((ref, typeIndex) => { - if (this._host.isSourceFile(ref.filePath)) { - externalReferenceVars.set(ref, `_decl${ngModuleIndex}_${typeIndex}`); - } + externalReferenceVars.set(ref, `_decl${ngModuleIndex}_${typeIndex}`); }); externalReferenceVars.forEach((varName, reference) => { outputCtx.statements.push( o.variable(varName) .set(o.NULL_EXPR.cast(o.DYNAMIC_TYPE)) - .toDeclStmt(o.expressionType(outputCtx.importExpr(reference)))); + .toDeclStmt(o.expressionType(outputCtx.importExpr( + reference, /* typeParams */ null, /* useSummaries */ false)))); }); if (emitFlags & StubEmitFlags.TypeCheck) { @@ -515,35 +514,38 @@ export class AotCompiler { } private _createOutputContext(genFilePath: string): OutputContext { - const importExpr = (symbol: StaticSymbol, typeParams: o.Type[] | null = null) => { - if (!(symbol instanceof StaticSymbol)) { - throw new Error(`Internal error: unknown identifier ${JSON.stringify(symbol)}`); - } - const arity = this._symbolResolver.getTypeArity(symbol) || 0; - const {filePath, name, members} = this._symbolResolver.getImportAs(symbol) || symbol; - const importModule = this._fileNameToModuleName(filePath, genFilePath); + const importExpr = + (symbol: StaticSymbol, typeParams: o.Type[] | null = null, + useSummaries: boolean = true) => { + if (!(symbol instanceof StaticSymbol)) { + throw new Error(`Internal error: unknown identifier ${JSON.stringify(symbol)}`); + } + const arity = this._symbolResolver.getTypeArity(symbol) || 0; + const {filePath, name, members} = + this._symbolResolver.getImportAs(symbol, useSummaries) || symbol; + const importModule = this._fileNameToModuleName(filePath, genFilePath); - // It should be good enough to compare filePath to genFilePath and if they are equal - // there is a self reference. However, ngfactory files generate to .ts but their - // symbols have .d.ts so a simple compare is insufficient. They should be canonical - // and is tracked by #17705. - const selfReference = this._fileNameToModuleName(genFilePath, genFilePath); - const moduleName = importModule === selfReference ? null : importModule; + // It should be good enough to compare filePath to genFilePath and if they are equal + // there is a self reference. However, ngfactory files generate to .ts but their + // symbols have .d.ts so a simple compare is insufficient. They should be canonical + // and is tracked by #17705. + const selfReference = this._fileNameToModuleName(genFilePath, genFilePath); + const moduleName = importModule === selfReference ? null : importModule; - // If we are in a type expression that refers to a generic type then supply - // the required type parameters. If there were not enough type parameters - // supplied, supply any as the type. Outside a type expression the reference - // should not supply type parameters and be treated as a simple value reference - // to the constructor function itself. - const suppliedTypeParams = typeParams || []; - const missingTypeParamsCount = arity - suppliedTypeParams.length; - const allTypeParams = - suppliedTypeParams.concat(new Array(missingTypeParamsCount).fill(o.DYNAMIC_TYPE)); - return members.reduce( - (expr, memberName) => expr.prop(memberName), - o.importExpr( - new o.ExternalReference(moduleName, name, null), allTypeParams)); - }; + // If we are in a type expression that refers to a generic type then supply + // the required type parameters. If there were not enough type parameters + // supplied, supply any as the type. Outside a type expression the reference + // should not supply type parameters and be treated as a simple value reference + // to the constructor function itself. + const suppliedTypeParams = typeParams || []; + const missingTypeParamsCount = arity - suppliedTypeParams.length; + const allTypeParams = + suppliedTypeParams.concat(new Array(missingTypeParamsCount).fill(o.DYNAMIC_TYPE)); + return members.reduce( + (expr, memberName) => expr.prop(memberName), + o.importExpr( + new o.ExternalReference(moduleName, name, null), allTypeParams)); + }; return {statements: [], genFilePath, importExpr}; } diff --git a/packages/compiler/src/aot/static_symbol_resolver.ts b/packages/compiler/src/aot/static_symbol_resolver.ts index 2ae58e28c3..da6fbda521 100644 --- a/packages/compiler/src/aot/static_symbol_resolver.ts +++ b/packages/compiler/src/aot/static_symbol_resolver.ts @@ -98,10 +98,10 @@ export class StaticSymbolResolver { * * @param staticSymbol the symbol for which to generate a import symbol */ - getImportAs(staticSymbol: StaticSymbol): StaticSymbol|null { + getImportAs(staticSymbol: StaticSymbol, useSummaries: boolean = true): StaticSymbol|null { if (staticSymbol.members.length) { const baseSymbol = this.getStaticSymbol(staticSymbol.filePath, staticSymbol.name); - const baseImportAs = this.getImportAs(baseSymbol); + const baseImportAs = this.getImportAs(baseSymbol, useSummaries); return baseImportAs ? this.getStaticSymbol(baseImportAs.filePath, baseImportAs.name, staticSymbol.members) : null; @@ -111,14 +111,14 @@ export class StaticSymbolResolver { const summarizedName = stripSummaryForJitNameSuffix(staticSymbol.name); const baseSymbol = this.getStaticSymbol(summarizedFileName, summarizedName, staticSymbol.members); - const baseImportAs = this.getImportAs(baseSymbol); + const baseImportAs = this.getImportAs(baseSymbol, useSummaries); return baseImportAs ? this.getStaticSymbol( summaryForJitFileName(baseImportAs.filePath), summaryForJitName(baseImportAs.name), baseSymbol.members) : null; } - let result = this.summaryResolver.getImportAs(staticSymbol); + let result = (useSummaries && this.summaryResolver.getImportAs(staticSymbol)) || null; if (!result) { result = this.importAs.get(staticSymbol) !; } diff --git a/packages/compiler/src/util.ts b/packages/compiler/src/util.ts index 3f23252b95..064c703030 100644 --- a/packages/compiler/src/util.ts +++ b/packages/compiler/src/util.ts @@ -152,7 +152,7 @@ export function utf8Encode(str: string): string { export interface OutputContext { genFilePath: string; statements: o.Statement[]; - importExpr(reference: any, typeParams?: o.Type[]|null): o.Expression; + importExpr(reference: any, typeParams?: o.Type[]|null, useSummaries?: boolean): o.Expression; } export function stringify(token: any): string { diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index 2359d0fff9..82e2cc8f76 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -196,6 +196,25 @@ describe('StaticSymbolResolver', () => { .toBe(symbolCache.get('/test3.d.ts', 'b')); }); + it('should ignore summaries for inputAs if requested', () => { + init( + { + '/test.ts': ` + export {a} from './test2'; + ` + }, + [], [{ + symbol: symbolCache.get('/test2.d.ts', 'a'), + importAs: symbolCache.get('/test3.d.ts', 'b') + }]); + + symbolResolver.getSymbolsOf('/test.ts'); + + expect( + symbolResolver.getImportAs(symbolCache.get('/test2.d.ts', 'a'), /* useSummaries */ false)) + .toBeUndefined(); + }); + it('should calculate importAs for symbols with members based on importAs for symbols without', () => { init(