From 34c8692d7116c2bec19e443d30dc155f1ba05de0 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 20 Jul 2018 12:52:26 -0700 Subject: [PATCH 1/2] feat(ivy): use the ReflectionHost to resolve parameters and initializers ngtsc's static resolver can evaluate function calls where parameters have default values. In TypeScript code these default values live on the function definition, but in ES5 code the default values are represented by statements in the function body. A new ReflectionHost method getDefinitionOfFunction() abstracts over this difference, and allows the static reflector to more accurately evaluate ES5 code. --- .../src/ngtsc/host/src/reflection.ts | 77 ++++++++++++++++++- .../src/ngtsc/metadata/src/reflector.ts | 17 +++- .../src/ngtsc/metadata/src/resolver.ts | 12 +-- .../src/ngtsc/metadata/test/reflector_spec.ts | 4 +- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts index bfea18a9ac0d..e88b15612ab3 100644 --- a/packages/compiler-cli/src/ngtsc/host/src/reflection.ts +++ b/packages/compiler-cli/src/ngtsc/host/src/reflection.ts @@ -147,9 +147,9 @@ export interface ClassMember { } /** - * A parameter to a function or constructor. + * A parameter to a constructor. */ -export interface Parameter { +export interface CtorParameter { /** * Name of the parameter, if available. * @@ -180,6 +180,54 @@ export interface Parameter { decorators: Decorator[]|null; } +/** + * Definition of a function or method, including its body if present and any parameters. + * + * In TypeScript code this metadata will be a simple reflection of the declarations in the node + * itself. In ES5 code this can be more complicated, as the default values for parameters may + * be extracted from certain body statements. + */ +export interface FunctionDefinition { + /** + * A reference to the node which declares the function. + */ + node: T; + + /** + * Statements of the function body, if a body is present, or null if no body is present. + * + * This list may have been filtered to exclude statements which perform parameter default value + * initialization. + */ + body: ts.Statement[]|null; + + /** + * Metadata regarding the function's parameters, including possible default value expressions. + */ + parameters: Parameter[]; +} + +/** + * A parameter to a function or method. + */ +export interface Parameter { + /** + * Name of the parameter, if available. + */ + name: string|null; + + /** + * Declaration which created this parameter. + */ + node: ts.ParameterDeclaration; + + /** + * Expression which represents the default value of the parameter, if any. + */ + initializer: ts.Expression|null; +} + /** * The source of an imported symbol, including the original symbol name and the module from which it * was imported. @@ -273,7 +321,30 @@ export interface ReflectionHost { * a constructor exists. If the constructor exists and has 0 parameters, this array will be empty. * If the class has no constructor, this method returns `null`. */ - getConstructorParameters(declaration: ts.Declaration): Parameter[]|null; + getConstructorParameters(declaration: ts.Declaration): CtorParameter[]|null; + + /** + * Reflect over a function and return metadata about its parameters and body. + * + * Functions in TypeScript and ES5 code have different AST representations, in particular around + * default values for parameters. A TypeScript function has its default value as the initializer + * on the parameter declaration, whereas an ES5 function has its default value set in a statement + * of the form: + * + * if (param === void 0) { param = 3; } + * + * This method abstracts over these details, and interprets the function declaration and body to + * extract parameter default values and the "real" body. + * + * A current limitation is that this metadata has no representation for shorthand assignment of + * parameter objects in the function signature. + * + * @param fn a TypeScript `ts.Declaration` node representing the function over which to reflect. + * + * @returns a `FunctionDefinition` giving metadata about the function definition. + */ + getDefinitionOfFunction(fn: T): FunctionDefinition; /** * Determine if an identifier was imported from another module and return `Import` metadata diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts index 5d604f8ec075..c3ce537fa5e1 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Declaration, Decorator, Import, Parameter, ReflectionHost} from '../../host'; +import {ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, FunctionDefinition, Import, ReflectionHost} from '../../host'; /** * reflector.ts implements static reflection of declarations using the TypeScript `ts.TypeChecker`. @@ -31,7 +31,7 @@ export class TypeScriptReflectionHost implements ReflectionHost { .filter((member): member is ClassMember => member !== null); } - getConstructorParameters(declaration: ts.Declaration): Parameter[]|null { + getConstructorParameters(declaration: ts.Declaration): CtorParameter[]|null { const clazz = castDeclarationToClassOrDie(declaration); // First, find the constructor. @@ -141,6 +141,19 @@ export class TypeScriptReflectionHost implements ReflectionHost { return this.getDeclarationOfSymbol(symbol); } + getDefinitionOfFunction(node: T): FunctionDefinition { + return { + node, + body: node.body !== undefined ? Array.from(node.body.statements) : null, + parameters: node.parameters.map(param => { + const name = parameterName(param.name); + const initializer = param.initializer || null; + return {name, node: param, initializer}; + }), + }; + } + /** * Resolve a `ts.Symbol` to its declaration, keeping track of the `viaModule` along the way. * diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts index acc8ff3f17ff..37875e50b2ac 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/resolver.ts @@ -547,11 +547,11 @@ class StaticInterpreter { `calling something that is not a function declaration? ${ts.SyntaxKind[lhs.node.kind]} (${node.getText()})`); } - const fn = lhs.node; + const fn = this.host.getDefinitionOfFunction(lhs.node); // If the function is foreign (declared through a d.ts file), attempt to resolve it with the // foreignFunctionResolver, if one is specified. - if (fn.body === undefined) { + if (fn.body === null) { let expr: ts.Expression|null = null; if (context.foreignFunctionResolver) { expr = context.foreignFunctionResolver(lhs, node.arguments); @@ -572,10 +572,10 @@ class StaticInterpreter { } const body = fn.body; - if (body.statements.length !== 1 || !ts.isReturnStatement(body.statements[0])) { + if (body.length !== 1 || !ts.isReturnStatement(body[0])) { throw new Error('Function body must have a single return statement only.'); } - const ret = body.statements[0] as ts.ReturnStatement; + const ret = body[0] as ts.ReturnStatement; const newScope: Scope = new Map(); fn.parameters.forEach((param, index) => { @@ -584,10 +584,10 @@ class StaticInterpreter { const arg = node.arguments[index]; value = this.visitExpression(arg, context); } - if (value === undefined && param.initializer !== undefined) { + if (value === undefined && param.initializer !== null) { value = this.visitExpression(param.initializer, context); } - newScope.set(param, value); + newScope.set(param.node, value); }); return ret.expression !== undefined ? diff --git a/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts b/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts index 47e05f0642e9..71ff87090a58 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/test/reflector_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Parameter} from '../../host'; +import {CtorParameter} from '../../host'; import {getDeclaration, makeProgram} from '../../testing/in_memory_typescript'; import {TypeScriptReflectionHost} from '../src/reflector'; @@ -165,7 +165,7 @@ describe('reflector', () => { }); function expectParameter( - param: Parameter, name: string, type?: string, decorator?: string, + param: CtorParameter, name: string, type?: string, decorator?: string, decoratorFrom?: string): void { expect(param.name !).toEqual(name); if (type === undefined) { From e0e09adb98ea8b808171267f0b23dbe33580edad Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 25 Jul 2018 07:57:35 +0100 Subject: [PATCH 2/2] feat(ivy): implement `getDefinitionOfFunction` on ES2015 and ES5 reflection hosts --- .../src/ngcc/src/host/esm2015_host.ts | 7 +- .../src/ngcc/src/host/esm5_host.ts | 74 ++++++++++++- .../src/ngcc/test/host/esm2015_host_spec.ts | 100 +++++++++++++++++- .../src/ngcc/test/host/esm5_host_spec.ts | 85 +++++++++++++++ 4 files changed, 258 insertions(+), 8 deletions(-) diff --git a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts index a2dff5d7df03..97605c5538df 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -8,9 +8,10 @@ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Decorator, Parameter} from '../../../ngtsc/host'; +import {ClassMember, ClassMemberKind, CtorParameter, Decorator} from '../../../ngtsc/host'; import {TypeScriptReflectionHost, reflectObjectLiteral} from '../../../ngtsc/metadata'; import {getNameText} from '../utils'; + import {NgccReflectionHost} from './ngcc_host'; export const DECORATORS = 'decorators' as ts.__String; @@ -162,7 +163,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * * @throws if `declaration` does not resolve to a class declaration. */ - getConstructorParameters(clazz: ts.Declaration): Parameter[]|null { + getConstructorParameters(clazz: ts.Declaration): CtorParameter[]|null { const classSymbol = this.getClassSymbol(clazz); if (!classSymbol) { throw new Error( @@ -170,7 +171,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } const parameterNodes = this.getConstructorParameterDeclarations(classSymbol); if (parameterNodes) { - const parameters: Parameter[] = []; + const parameters: CtorParameter[] = []; const decoratorInfo = this.getConstructorDecorators(classSymbol); parameterNodes.forEach((node, index) => { const info = decoratorInfo[index]; diff --git a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts index b79b13d0de20..7e66beb21b08 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -7,8 +7,9 @@ */ import * as ts from 'typescript'; -import {ClassMember, ClassMemberKind, Decorator} from '../../../ngtsc/host'; +import {ClassMember, ClassMemberKind, Decorator, FunctionDefinition, Parameter} from '../../../ngtsc/host'; import {reflectObjectLiteral} from '../../../ngtsc/metadata'; +import {getNameText} from '../utils'; import {CONSTRUCTOR_PARAMS, Esm2015ReflectionHost, getPropertyValueFromSymbol} from './esm2015_host'; /** @@ -54,6 +55,27 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { return undefined; } + /** + * Parse a function declaration to find the relevant metadata about it. + * In ESM5 we need to do special work with optional arguments to the function, since they get + * their own initializer statement that needs to be parsed and then not included in the "body" + * statements of the function. + * @param node the function declaration to parse. + */ + getDefinitionOfFunction(node: T): FunctionDefinition { + const parameters = + node.parameters.map(p => ({name: getNameText(p.name), node: p, initializer: null})); + + let statements: ts.Statement[]|null = null; + if (node.body) { + const firstNonInitializer = + node.body.statements.findIndex(s => !reflectParamInitializer(s, parameters)); + statements = node.body.statements.slice(firstNonInitializer); + } + return {node, body: statements || null, parameters}; + } + /** * Find the declarations of the constructor parameters of a class identified by its symbol. * In ESM5 there is no "class" so the constructor that we want is actually the declaration @@ -134,4 +156,52 @@ function getReturnStatement(declaration: ts.Expression | undefined): ts.ReturnSt function reflectArrayElement(element: ts.Expression) { return ts.isObjectLiteralExpression(element) ? reflectObjectLiteral(element) : null; -} \ No newline at end of file +} + +/** + * Parse the statement to extract the ESM5 parameter initializer if there is one. + * If one is found, add it to the appropriate parameter in the `parameters` collection. + * + * The form we are looking for is: + * + * ``` + * if (arg === void 0) { arg = initializer; } + * ``` + * + * @param statement A statement that may be initializing an optional parameter + * @param parameters The collection of parameters that were found in the function definition + * @returns true if the statement was a parameter initializer + */ +function reflectParamInitializer(statement: ts.Statement, parameters: Parameter[]) { + if (ts.isIfStatement(statement) && isUndefinedComparison(statement.expression) && + ts.isBlock(statement.thenStatement) && statement.thenStatement.statements.length === 1) { + const ifStatementComparison = statement.expression; // (arg === void 0) + const thenStatement = statement.thenStatement.statements[0]; // arg = initializer; + if (isAssignment(thenStatement)) { + const comparisonName = ifStatementComparison.left.text; + const assignmentName = thenStatement.expression.left.text; + if (comparisonName === assignmentName) { + const parameter = parameters.find(p => p.name === comparisonName); + if (parameter) { + parameter.initializer = thenStatement.expression.right; + return true; + } + } + } + } + return false; +} + +function isUndefinedComparison(expression: ts.Expression): expression is ts.Expression& + {left: ts.Identifier, right: ts.Expression} { + return ts.isBinaryExpression(expression) && + expression.operatorToken.kind === ts.SyntaxKind.EqualsEqualsEqualsToken && + ts.isVoidExpression(expression.right) && ts.isIdentifier(expression.left); +} + +function isAssignment(statement: ts.Statement): statement is ts.ExpressionStatement& + {expression: {left: ts.Identifier, right: ts.Expression}} { + return ts.isExpressionStatement(statement) && ts.isBinaryExpression(statement.expression) && + statement.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken && + ts.isIdentifier(statement.expression.left); +} diff --git a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts index b30866ade3fe..f62e0c67148e 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm2015_host_spec.ts @@ -353,6 +353,38 @@ const EXPORTS_FILES = [ }, ]; +const FUNCTION_BODY_FILE = { + name: '/function_body.js', + contents: ` + function foo(x) { + return x; + } + function bar(x, y = 42) { + return x + y; + } + function baz(x) { + let y; + if (y === void 0) { y = 42; } + return x; + } + let y; + function qux(x) { + if (x === void 0) { y = 42; } + return y; + } + function moo() { + let x; + if (x === void 0) { x = 42; } + return x; + } + let x; + function juu() { + if (x === void 0) { x = 42; } + return x; + } + ` +}; + describe('Esm2015ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -702,7 +734,7 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getConstructorParameters', () => { + describe('getConstructorParameters()', () => { it('should find the decorated constructor parameters', () => { const program = makeProgram(SOME_DIRECTIVE_FILE); const host = new Esm2015ReflectionHost(program.getTypeChecker()); @@ -898,7 +930,69 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getImportOfIdentifier', () => { + describe('getDefinitionOfFunction()', () => { + it('should return an object describing the function declaration passed as an argument', () => { + const program = makeProgram(FUNCTION_BODY_FILE); + const host = new Esm2015ReflectionHost(program.getTypeChecker()); + + const fooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'foo', ts.isFunctionDeclaration) !; + const fooDef = host.getDefinitionOfFunction(fooNode); + expect(fooDef.node).toBe(fooNode); + expect(fooDef.body !.length).toEqual(1); + expect(fooDef.body ![0].getText()).toEqual(`return x;`); + expect(fooDef.parameters.length).toEqual(1); + expect(fooDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + + const barNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'bar', ts.isFunctionDeclaration) !; + const barDef = host.getDefinitionOfFunction(barNode); + expect(barDef.node).toBe(barNode); + expect(barDef.body !.length).toEqual(1); + expect(ts.isReturnStatement(barDef.body ![0])).toBeTruthy(); + expect(barDef.body ![0].getText()).toEqual(`return x + y;`); + expect(barDef.parameters.length).toEqual(2); + expect(barDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + expect(barDef.parameters[1].name).toEqual('y'); + expect(barDef.parameters[1].initializer !.getText()).toEqual('42'); + + const bazNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'baz', ts.isFunctionDeclaration) !; + const bazDef = host.getDefinitionOfFunction(bazNode); + expect(bazDef.node).toBe(bazNode); + expect(bazDef.body !.length).toEqual(3); + expect(bazDef.parameters.length).toEqual(1); + expect(bazDef.parameters[0].name).toEqual('x'); + expect(bazDef.parameters[0].initializer).toBe(null); + + const quxNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'qux', ts.isFunctionDeclaration) !; + const quxDef = host.getDefinitionOfFunction(quxNode); + expect(quxDef.node).toBe(quxNode); + expect(quxDef.body !.length).toEqual(2); + expect(quxDef.parameters.length).toEqual(1); + expect(quxDef.parameters[0].name).toEqual('x'); + expect(quxDef.parameters[0].initializer).toBe(null); + + const mooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'moo', ts.isFunctionDeclaration) !; + const mooDef = host.getDefinitionOfFunction(mooNode); + expect(mooDef.node).toBe(mooNode); + expect(mooDef.body !.length).toEqual(3); + expect(mooDef.parameters).toEqual([]); + + const juuNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'juu', ts.isFunctionDeclaration) !; + const juuDef = host.getDefinitionOfFunction(juuNode); + expect(juuDef.node).toBe(juuNode); + expect(juuDef.body !.length).toEqual(2); + expect(juuDef.parameters).toEqual([]); + }); + }); + + describe('getImportOfIdentifier()', () => { it('should find the import of an identifier', () => { const program = makeProgram(...IMPORTS_FILES); const host = new Esm2015ReflectionHost(program.getTypeChecker()); @@ -930,7 +1024,7 @@ describe('Esm2015ReflectionHost', () => { }); }); - describe('getDeclarationOfIdentifier', () => { + describe('getDeclarationOfIdentifier()', () => { it('should return the declaration of a locally defined identifier', () => { const program = makeProgram(SOME_DIRECTIVE_FILE); const host = new Esm2015ReflectionHost(program.getTypeChecker()); diff --git a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts index 92b9ab0719b6..ba22c26588c2 100644 --- a/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts @@ -406,6 +406,43 @@ const EXPORTS_FILES = [ }, ]; +const FUNCTION_BODY_FILE = { + name: '/function_body.js', + contents: ` + function foo(x) { + return x; + } + function bar(x, y) { + if (y === void 0) { y = 42; } + return x + y; + } + function complex() { + var x = 42; + return 42; + } + function baz(x) { + var y; + if (x === void 0) { y = 42; } + return y; + } + var y; + function qux(x) { + if (x === void 0) { y = 42; } + return y; + } + function moo() { + var x; + if (x === void 0) { x = 42; } + return x; + } + var x; + function juu() { + if (x === void 0) { x = 42; } + return x; + } + ` +}; + describe('Esm5ReflectionHost', () => { describe('getDecoratorsOfDeclaration()', () => { @@ -930,6 +967,54 @@ describe('Esm5ReflectionHost', () => { }); }); + describe('getDefinitionOfFunction()', () => { + it('should return an object describing the function declaration passed as an argument', () => { + const program = makeProgram(FUNCTION_BODY_FILE); + const host = new Esm5ReflectionHost(program.getTypeChecker()); + + const fooNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'foo', ts.isFunctionDeclaration) !; + const fooDef = host.getDefinitionOfFunction(fooNode); + expect(fooDef.node).toBe(fooNode); + expect(fooDef.body !.length).toEqual(1); + expect(fooDef.body ![0].getText()).toEqual(`return x;`); + expect(fooDef.parameters.length).toEqual(1); + expect(fooDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + + const barNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'bar', ts.isFunctionDeclaration) !; + const barDef = host.getDefinitionOfFunction(barNode); + expect(barDef.node).toBe(barNode); + expect(barDef.body !.length).toEqual(1); + expect(ts.isReturnStatement(barDef.body ![0])).toBeTruthy(); + expect(barDef.body ![0].getText()).toEqual(`return x + y;`); + expect(barDef.parameters.length).toEqual(2); + expect(barDef.parameters[0].name).toEqual('x'); + expect(fooDef.parameters[0].initializer).toBe(null); + expect(barDef.parameters[1].name).toEqual('y'); + expect(barDef.parameters[1].initializer !.getText()).toEqual('42'); + + const bazNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'baz', ts.isFunctionDeclaration) !; + const bazDef = host.getDefinitionOfFunction(bazNode); + expect(bazDef.node).toBe(bazNode); + expect(bazDef.body !.length).toEqual(3); + expect(bazDef.parameters.length).toEqual(1); + expect(bazDef.parameters[0].name).toEqual('x'); + expect(bazDef.parameters[0].initializer).toBe(null); + + const quxNode = + getDeclaration(program, FUNCTION_BODY_FILE.name, 'qux', ts.isFunctionDeclaration) !; + const quxDef = host.getDefinitionOfFunction(quxNode); + expect(quxDef.node).toBe(quxNode); + expect(quxDef.body !.length).toEqual(2); + expect(quxDef.parameters.length).toEqual(1); + expect(quxDef.parameters[0].name).toEqual('x'); + expect(quxDef.parameters[0].initializer).toBe(null); + }); + }); + describe('getImportOfIdentifier', () => { it('should find the import of an identifier', () => { const program = makeProgram(...IMPORTS_FILES);