Skip to content

Commit

Permalink
fix(compiler): generate the correct imports for summary type-check
Browse files Browse the repository at this point in the history 
Summaries should be ignored when importing the types used in a
type-check block.
  • Loading branch information
@chuckjaz @alxhub
chuckjaz authored and alxhub committed Dec 15, 2017
1 parent d213a20 commit d91ff17
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 41 deletions.
10 changes: 7 additions & 3 deletions packages/compiler-cli/test/test_support.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -21,16 +22,19 @@ 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);
return fn;
}

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;
}
Expand Down
68 changes: 35 additions & 33 deletions packages/compiler/src/aot/compiler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,15 +218,14 @@ export class AotCompiler {

const externalReferenceVars = new Map<any, string>();
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) {
Expand Down Expand Up @@ -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);

// 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.Expression>o.importExpr(
new o.ExternalReference(moduleName, name, null), allTypeParams));
};
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;

// 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.Expression>o.importExpr(
new o.ExternalReference(moduleName, name, null), allTypeParams));
};

return {statements: [], genFilePath, importExpr};
}
Expand Down
8 changes: 4 additions & 4 deletions packages/compiler/src/aot/static_symbol_resolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) !;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions packages/compiler/test/aot/static_symbol_resolver_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit d91ff17

Please sign in to comment.