feat(compiler): do not evaluate metadata expressions that can use references (#18001)

This commit is contained in:
Chuck Jazdzewski 2017-07-13 17:16:56 -06:00 committed by Igor Minar
parent 72143e80da
commit ddb766e456
10 changed files with 264 additions and 42 deletions

View File

@ -18,6 +18,7 @@ import {StaticSymbol} from './static_symbol';
import {StaticSymbolResolver} from './static_symbol_resolver';
const ANGULAR_CORE = '@angular/core';
const ANGULAR_ROUTER = '@angular/router';
const HIDDEN_KEY = /^\$.*\$$/;
@ -25,6 +26,10 @@ const IGNORE = {
__symbolic: 'ignore'
};
const USE_VALUE = 'useValue';
const PROVIDE = 'provide';
const REFERENCE_SET = new Set([USE_VALUE, 'useFactory', 'data']);
function shouldIgnore(value: any): boolean {
return value && value.__symbolic == 'ignore';
}
@ -41,6 +46,8 @@ export class StaticReflector implements CompileReflector {
private conversionMap = new Map<StaticSymbol, (context: StaticSymbol, args: any[]) => any>();
private injectionToken: StaticSymbol;
private opaqueToken: StaticSymbol;
private ROUTES: StaticSymbol;
private ANALYZE_FOR_ENTRY_COMPONENTS: StaticSymbol;
private annotationForParentClassWithSummaryKind = new Map<CompileSummaryKind, any[]>();
private annotationNames = new Map<any, string>();
@ -88,6 +95,10 @@ export class StaticReflector implements CompileReflector {
this.symbolResolver.getSymbolByModule(moduleUrl, name, containingFile));
}
tryFindDeclaration(moduleUrl: string, name: string): StaticSymbol {
return this.symbolResolver.ignoreErrorsFor(() => this.findDeclaration(moduleUrl, name));
}
findSymbolDeclaration(symbol: StaticSymbol): StaticSymbol {
const resolvedSymbol = this.symbolResolver.resolveSymbol(symbol);
if (resolvedSymbol && resolvedSymbol.metadata instanceof StaticSymbol) {
@ -267,6 +278,9 @@ export class StaticReflector implements CompileReflector {
private initializeConversionMap(): void {
this.injectionToken = this.findDeclaration(ANGULAR_CORE, 'InjectionToken');
this.opaqueToken = this.findDeclaration(ANGULAR_CORE, 'OpaqueToken');
this.ROUTES = this.tryFindDeclaration(ANGULAR_ROUTER, 'ROUTES');
this.ANALYZE_FOR_ENTRY_COMPONENTS =
this.findDeclaration(ANGULAR_CORE, 'ANALYZE_FOR_ENTRY_COMPONENTS');
this._registerDecoratorOrConstructor(this.findDeclaration(ANGULAR_CORE, 'Host'), Host);
this._registerDecoratorOrConstructor(
@ -350,7 +364,8 @@ export class StaticReflector implements CompileReflector {
let scope = BindingScope.empty;
const calling = new Map<StaticSymbol, boolean>();
function simplifyInContext(context: StaticSymbol, value: any, depth: number): any {
function simplifyInContext(
context: StaticSymbol, value: any, depth: number, references: number): any {
function resolveReferenceValue(staticSymbol: StaticSymbol): any {
const resolvedSymbol = self.symbolResolver.resolveSymbol(staticSymbol);
return resolvedSymbol ? resolvedSymbol.metadata : null;
@ -367,7 +382,7 @@ export class StaticReflector implements CompileReflector {
if (value && (depth != 0 || value.__symbolic != 'error')) {
const parameters: string[] = targetFunction['parameters'];
const defaults: any[] = targetFunction.defaults;
args = args.map(arg => simplifyInContext(context, arg, depth + 1))
args = args.map(arg => simplifyInContext(context, arg, depth + 1, references))
.map(arg => shouldIgnore(arg) ? undefined : arg);
if (defaults && defaults.length > args.length) {
args.push(...defaults.slice(args.length).map((value: any) => simplify(value)));
@ -380,7 +395,7 @@ export class StaticReflector implements CompileReflector {
let result: any;
try {
scope = functionScope.done();
result = simplifyInContext(functionSymbol, value, depth + 1);
result = simplifyInContext(functionSymbol, value, depth + 1, references);
} finally {
scope = oldScope;
}
@ -427,15 +442,15 @@ export class StaticReflector implements CompileReflector {
return result;
}
if (expression instanceof StaticSymbol) {
// Stop simplification at builtin symbols
// Stop simplification at builtin symbols or if we are in a reference context
if (expression === self.injectionToken || expression === self.opaqueToken ||
self.conversionMap.has(expression)) {
self.conversionMap.has(expression) || references > 0) {
return expression;
} else {
const staticSymbol = expression;
const declarationValue = resolveReferenceValue(staticSymbol);
if (declarationValue) {
return simplifyInContext(staticSymbol, declarationValue, depth + 1);
return simplifyInContext(staticSymbol, declarationValue, depth + 1, references);
} else {
return staticSymbol;
}
@ -526,13 +541,15 @@ export class StaticReflector implements CompileReflector {
self.getStaticSymbol(selectTarget.filePath, selectTarget.name, members);
const declarationValue = resolveReferenceValue(selectContext);
if (declarationValue) {
return simplifyInContext(selectContext, declarationValue, depth + 1);
return simplifyInContext(
selectContext, declarationValue, depth + 1, references);
} else {
return selectContext;
}
}
if (selectTarget && isPrimitive(member))
return simplifyInContext(selectContext, selectTarget[member], depth + 1);
return simplifyInContext(
selectContext, selectTarget[member], depth + 1, references);
return null;
case 'reference':
// Note: This only has to deal with variable references,
@ -551,7 +568,8 @@ export class StaticReflector implements CompileReflector {
case 'new':
case 'call':
// Determine if the function is a built-in conversion
staticSymbol = simplifyInContext(context, expression['expression'], depth + 1);
staticSymbol = simplifyInContext(
context, expression['expression'], depth + 1, /* references */ 0);
if (staticSymbol instanceof StaticSymbol) {
if (staticSymbol === self.injectionToken || staticSymbol === self.opaqueToken) {
// if somebody calls new InjectionToken, don't create an InjectionToken,
@ -562,7 +580,8 @@ export class StaticReflector implements CompileReflector {
let converter = self.conversionMap.get(staticSymbol);
if (converter) {
const args =
argExpressions.map(arg => simplifyInContext(context, arg, depth + 1))
argExpressions
.map(arg => simplifyInContext(context, arg, depth + 1, references))
.map(arg => shouldIgnore(arg) ? undefined : arg);
return converter(context, args);
} else {
@ -590,7 +609,20 @@ export class StaticReflector implements CompileReflector {
}
return null;
}
return mapStringMap(expression, (value, name) => simplify(value));
return mapStringMap(expression, (value, name) => {
if (REFERENCE_SET.has(name)) {
if (name === USE_VALUE && PROVIDE in expression) {
// If this is a provider expression, check for special tokens that need the value
// during analysis.
const provide = simplify(expression.provide);
if (provide === self.ROUTES || provide == self.ANALYZE_FOR_ENTRY_COMPONENTS) {
return simplify(value);
}
}
return simplifyInContext(context, value, depth, references + 1);
}
return simplify(value)
});
}
return IGNORE;
}
@ -608,16 +640,16 @@ export class StaticReflector implements CompileReflector {
}
}
const recordedSimplifyInContext = (context: StaticSymbol, value: any, depth: number) => {
const recordedSimplifyInContext = (context: StaticSymbol, value: any) => {
try {
return simplifyInContext(context, value, depth);
return simplifyInContext(context, value, 0, 0);
} catch (e) {
this.reportError(e, context);
}
};
const result = this.errorRecorder ? recordedSimplifyInContext(context, value, 0) :
simplifyInContext(context, value, 0);
const result = this.errorRecorder ? recordedSimplifyInContext(context, value) :
simplifyInContext(context, value, 0, 0);
if (shouldIgnore(result)) {
return undefined;
}

View File

@ -191,6 +191,17 @@ export class StaticSymbolResolver {
}
}
/* @internal */
ignoreErrorsFor<T>(cb: () => T) {
const recorder = this.errorRecorder;
this.errorRecorder = () => {};
try {
return cb();
} finally {
this.errorRecorder = recorder;
}
}
private _resolveSymbolMembers(staticSymbol: StaticSymbol): ResolvedStaticSymbol|null {
const members = staticSymbol.members;
const baseResolvedSymbol =
@ -446,6 +457,7 @@ export class StaticSymbolResolver {
return moduleMetadata;
}
getSymbolByModule(module: string, symbolName: string, containingFile?: string): StaticSymbol {
const filePath = this.resolveModule(module, containingFile);
if (!filePath) {

View File

@ -306,6 +306,93 @@ describe('compiler (unbundled Angular)', () => {
expect(genSource.startsWith(genFilePreamble)).toBe(true);
});
it('should be able to use animation macro methods', () => {
const FILES = {
app: {
'app.ts': `
import {Component, NgModule} from '@angular/core';
import {trigger, state, style, transition, animate} from '@angular/animations';
export const EXPANSION_PANEL_ANIMATION_TIMING = '225ms cubic-bezier(0.4,0.0,0.2,1)';
@Component({
selector: 'app-component',
template: '<div></div>',
animations: [
trigger('bodyExpansion', [
state('collapsed', style({height: '0px'})),
state('expanded', style({height: '*'})),
transition('expanded <=> collapsed', animate(EXPANSION_PANEL_ANIMATION_TIMING)),
]),
trigger('displayMode', [
state('collapsed', style({margin: '0'})),
state('default', style({margin: '16px 0'})),
state('flat', style({margin: '0'})),
transition('flat <=> collapsed, default <=> collapsed, flat <=> default',
animate(EXPANSION_PANEL_ANIMATION_TIMING)),
]),
],
})
export class AppComponent { }
@NgModule({ declarations: [ AppComponent ] })
export class AppModule { }
`
}
};
compile([FILES, angularFiles]);
});
it('should detect an entry component via an indirection', () => {
const FILES = {
app: {
'app.ts': `
import {NgModule, ANALYZE_FOR_ENTRY_COMPONENTS} from '@angular/core';
import {AppComponent} from './app.component';
import {COMPONENT_VALUE, MyComponent} from './my-component';
@NgModule({
declarations: [ AppComponent, MyComponent ],
bootstrap: [ AppComponent ],
providers: [{
provide: ANALYZE_FOR_ENTRY_COMPONENTS,
multi: true,
useValue: COMPONENT_VALUE
}],
})
export class AppModule { }
`,
'app.component.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'app-component',
template: '<div></div>',
})
export class AppComponent { }
`,
'my-component.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'my-component',
template: '<div></div>',
})
export class MyComponent {}
export const COMPONENT_VALUE = [{a: 'b', component: MyComponent}];
`
}
};
const result = compile([FILES, angularFiles]);
const appModuleFactory =
result.genFiles.find(f => /my-component\.ngfactory/.test(f.genFileUrl));
expect(appModuleFactory).toBeDefined();
if (appModuleFactory) {
expect(toTypeScript(appModuleFactory)).toContain('MyComponentNgFactory');
}
});
describe('ComponentFactories', () => {
it('should include inputs, outputs and ng-content selectors in the component factory', () => {
const FILES: MockDirectory = {
@ -624,7 +711,7 @@ describe('compiler (unbundled Angular)', () => {
});
describe('compiler (bundled Angular)', () => {
setup({compileAngular: false});
setup({compileAngular: false, compileAnimations: false});
let angularFiles: Map<string, string>;

View File

@ -845,6 +845,33 @@ describe('StaticReflector', () => {
});
});
describe('expression lowering', () => {
it('should be able to accept a lambda in a reference location', () => {
const data = Object.create(DEFAULT_TEST_DATA);
const file = '/tmp/src/my_component.ts';
data[file] = `
import {Component, InjectionToken} from '@angular/core';
export const myLambda = () => [1, 2, 3];
export const NUMBERS = new InjectionToken<number[]>();
@Component({
template: '<div>{{name}}</div>',
providers: [{provide: NUMBERS, useFactory: myLambda}]
})
export class MyComponent {
name = 'Some name';
}
`;
init(data);
expect(reflector.annotations(reflector.getStaticSymbol(file, 'MyComponent'))[0]
.providers[0]
.useFactory)
.toBe(reflector.getStaticSymbol(file, 'myLambda'));
});
});
});
const DEFAULT_TEST_DATA: {[key: string]: any} = {

View File

@ -456,7 +456,12 @@ export class MockStaticSymbolResolverHost implements StaticSymbolResolverHost {
filePath, this.data[filePath], ts.ScriptTarget.ES5, /* setParentNodes */ true);
const diagnostics: ts.Diagnostic[] = (<any>sf).parseDiagnostics;
if (diagnostics && diagnostics.length) {
const errors = diagnostics.map(d => `(${d.start}-${d.start+d.length}): ${d.messageText}`)
const errors =
diagnostics
.map(d => {
const {line, character} = ts.getLineAndCharacterOfPosition(d.file, d.start);
return `(${line}:${character}): ${d.messageText}`;
})
.join('\n');
throw Error(`Error encountered during parse of file ${filePath}\n${errors}`);
}

View File

@ -513,8 +513,9 @@ const minCoreIndex = `
export * from './src/codegen_private_exports';
`;
export function setup(options: {compileAngular: boolean} = {
compileAngular: true
export function setup(options: {compileAngular: boolean, compileAnimations: boolean} = {
compileAngular: true,
compileAnimations: true,
}) {
let angularFiles = new Map<string, string>();
@ -526,6 +527,13 @@ export function setup(options: {compileAngular: boolean} = {
emittingProgram.emit();
emittingHost.writtenAngularFiles(angularFiles);
}
if (options.compileAnimations) {
const emittingHost =
new EmittingCompilerHost(['@angular/animations/index.ts'], {emitMetadata: true});
const emittingProgram = ts.createProgram(emittingHost.scripts, settings, emittingHost);
emittingProgram.emit();
emittingHost.writtenAngularFiles(angularFiles);
}
});
return angularFiles;

View File

@ -451,6 +451,10 @@ export class MetadataCollector {
if (typeof varValue == 'string' || typeof varValue == 'number' ||
typeof varValue == 'boolean') {
locals.define(nameNode.text, varValue);
if (exported) {
locals.defineReference(
nameNode.text, {__symbolic: 'reference', name: nameNode.text});
}
} else if (!exported) {
if (varValue && !isMetadataError(varValue)) {
locals.define(nameNode.text, recordEntry(varValue, node));

View File

@ -227,7 +227,7 @@ export class Evaluator {
* Produce a JSON serialiable object representing `node`. The foldable values in the expression
* tree are folded. For example, a node representing `1 + 2` is folded into `3`.
*/
public evaluateNode(node: ts.Node): MetadataValue {
public evaluateNode(node: ts.Node, preferReference?: boolean): MetadataValue {
const t = this;
let error: MetadataError|undefined;
@ -240,8 +240,8 @@ export class Evaluator {
return !t.options.verboseInvalidExpression && isMetadataError(value);
}
const resolveName = (name: string): MetadataValue => {
const reference = this.symbols.resolve(name);
const resolveName = (name: string, preferReference?: boolean): MetadataValue => {
const reference = this.symbols.resolve(name, preferReference);
if (reference === undefined) {
// Encode as a global reference. StaticReflector will check the reference.
return recordEntry({__symbolic: 'reference', name}, node);
@ -268,8 +268,8 @@ export class Evaluator {
return true;
}
const propertyValue = isPropertyAssignment(assignment) ?
this.evaluateNode(assignment.initializer) :
resolveName(propertyName);
this.evaluateNode(assignment.initializer, /* preferReference */ true) :
resolveName(propertyName, /* preferReference */ true);
if (isFoldableError(propertyValue)) {
error = propertyValue;
return true; // Stop the forEachChild.
@ -286,7 +286,7 @@ export class Evaluator {
case ts.SyntaxKind.ArrayLiteralExpression:
let arr: MetadataValue[] = [];
ts.forEachChild(node, child => {
const value = this.evaluateNode(child);
const value = this.evaluateNode(child, /* preferReference */ true);
// Check for error
if (isFoldableError(value)) {
@ -403,7 +403,7 @@ export class Evaluator {
case ts.SyntaxKind.Identifier:
const identifier = <ts.Identifier>node;
const name = identifier.text;
return resolveName(name);
return resolveName(name, preferReference);
case ts.SyntaxKind.TypeReference:
const typeReferenceNode = <ts.TypeReferenceNode>node;
const typeNameNode = typeReferenceNode.typeName;

View File

@ -8,16 +8,22 @@
import * as ts from 'typescript';
import {MetadataValue} from './schema';
import {MetadataSymbolicReferenceExpression, MetadataValue} from './schema';
export class Symbols {
private _symbols: Map<string, MetadataValue>;
private references = new Map<string, MetadataSymbolicReferenceExpression>();
constructor(private sourceFile: ts.SourceFile) {}
resolve(name: string): MetadataValue|undefined { return this.symbols.get(name); }
resolve(name: string, preferReference?: boolean): MetadataValue|undefined {
return (preferReference && this.references.get(name)) || this.symbols.get(name);
}
define(name: string, value: MetadataValue) { this.symbols.set(name, value); }
defineReference(name: string, value: MetadataSymbolicReferenceExpression) {
this.references.set(name, value);
}
has(name: string): boolean { return this.symbols.has(name); }

View File

@ -635,8 +635,7 @@ describe('Collector', () => {
describe('with interpolations', () => {
function e(expr: string, prefix?: string) {
const source = createSource(`${prefix || ''} export let value = ${expr};`);
const metadata = collector.getMetadata(source);
const metadata = collectSource(`${prefix || ''} export let value = ${expr};`);
return expect(metadata.metadata['value']);
}
@ -704,15 +703,12 @@ describe('Collector', () => {
});
it('should ignore |null or |undefined in type expressions', () => {
const source = ts.createSourceFile(
'somefile.ts', `
const metadata = collectSource(`
import {Foo} from './foo';
export class SomeClass {
constructor (a: Foo, b: Foo | null, c: Foo | undefined, d: Foo | undefined | null, e: Foo | undefined | null | Foo) {}
}
`,
ts.ScriptTarget.Latest, true);
const metadata = collector.getMetadata(source);
`);
expect((metadata.metadata['SomeClass'] as ClassMetadata).members).toEqual({
__ctor__: [{
__symbolic: 'constructor',
@ -832,19 +828,18 @@ describe('Collector', () => {
describe('regerssion', () => {
it('should be able to collect a short-hand property value', () => {
const source = createSource(`
const metadata = collectSource(`
const children = { f1: 1 };
export const r = [
{path: ':locale', children}
];
`);
const metadata = collector.getMetadata(source);
expect(metadata.metadata).toEqual({r: [{path: ':locale', children: {f1: 1}}]});
});
// #17518
it('should skip a default function', () => {
const source = createSource(`
const metadata = collectSource(`
export default function () {
const mainRoutes = [
@ -856,12 +851,11 @@ describe('Collector', () => {
return mainRoutes;
}`);
const metadata = collector.getMetadata(source);
expect(metadata).toBeUndefined();
});
it('should skip a named default export', () => {
const source = createSource(`
const metadata = collectSource(`
function mainRoutes() {
const mainRoutes = [
@ -876,7 +870,6 @@ describe('Collector', () => {
exports = foo;
`);
const metadata = collector.getMetadata(source);
expect(metadata).toBeUndefined();
});
@ -903,11 +896,59 @@ describe('Collector', () => {
});
});
describe('references', () => {
beforeEach(() => { collector = new MetadataCollector({quotedNames: true}); });
it('should record a reference to an exported field of a useValue', () => {
const metadata = collectSource(`
export var someValue = 1;
export const v = {
useValue: someValue
};
`);
expect(metadata.metadata['someValue']).toEqual(1);
expect(metadata.metadata['v']).toEqual({
useValue: {__symbolic: 'reference', name: 'someValue'}
});
});
it('should leave external references in place in an object literal', () => {
const metadata = collectSource(`
export const myLambda = () => [1, 2, 3];
const indirect = [{a: 1, b: 3: c: myLambda}];
export const v = {
v: {i: indirect}
}
`);
expect(metadata.metadata['v']).toEqual({
v: {i: [{a: 1, b: 3, c: {__symbolic: 'reference', name: 'myLambda'}}]}
});
});
it('should leave an external reference in place in an array literal', () => {
const metadata = collectSource(`
export const myLambda = () => [1, 2, 3];
const indirect = [1, 3, myLambda}];
export const v = {
v: {i: indirect}
}
`);
expect(metadata.metadata['v']).toEqual({
v: {i: [1, 3, {__symbolic: 'reference', name: 'myLambda'}]}
});
});
});
function override(fileName: string, content: string) {
host.overrideFile(fileName, content);
host.addFile(fileName);
program = service.getProgram();
}
function collectSource(content: string): ModuleMetadata {
const sourceFile = createSource(content);
return collector.getMetadata(sourceFile);
}
});
// TODO: Do not use \` in a template literal as it confuses clang-format