diff --git a/integration/ngcc/test.sh b/integration/ngcc/test.sh index efa8901f7a..0e8e17d49e 100755 --- a/integration/ngcc/test.sh +++ b/integration/ngcc/test.sh @@ -39,6 +39,12 @@ ivy-ngcc grep "static ngInjectorDef: ɵngcc0.InjectorDef;" node_modules/@angular/core/src/application_module.d.ts if [[ $? != 0 ]]; then exit 1; fi +# Did it generate a base factory call for synthesized constructors correctly? + grep "const ɵMatTable_BaseFactory = ɵngcc0.ɵgetInheritedFactory(MatTable);" node_modules/@angular/material/esm2015/table.js + if [[ $? != 0 ]]; then exit 1; fi + grep "const ɵMatTable_BaseFactory = ɵngcc0.ɵgetInheritedFactory(MatTable);" node_modules/@angular/material/esm5/table.es5.js + if [[ $? != 0 ]]; then exit 1; fi + # Can it be safely run again (as a noop)? ivy-ngcc 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 c3e57f23bf..e4241d62d9 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts @@ -800,10 +800,16 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N if (constructorSymbol) { // For some reason the constructor does not have a `valueDeclaration` ?!? const constructor = constructorSymbol.declarations && - constructorSymbol.declarations[0] as ts.ConstructorDeclaration; - if (constructor && constructor.parameters) { + constructorSymbol.declarations[0] as ts.ConstructorDeclaration | undefined; + if (!constructor) { + return []; + } + if (constructor.parameters.length > 0) { return Array.from(constructor.parameters); } + if (isSynthesizedConstructor(constructor)) { + return null; + } return []; } return null; @@ -1228,3 +1234,52 @@ function collectExportedDeclarations( }); } } + + +/** + * A constructor function may have been "synthesized" by TypeScript during JavaScript emit, + * in the case no user-defined constructor exists and e.g. property initializers are used. + * Those initializers need to be emitted into a constructor in JavaScript, so the TypeScript + * compiler generates a synthetic constructor. + * + * We need to identify such constructors as ngcc needs to be able to tell if a class did + * originally have a constructor in the TypeScript source. When a class has a superclass, + * a synthesized constructor must not be considered as a user-defined constructor as that + * prevents a base factory call from being created by ngtsc, resulting in a factory function + * that does not inject the dependencies of the superclass. Hence, we identify a default + * synthesized super call in the constructor body, according to the structure that TypeScript + * emits during JavaScript emit: + * https://github.com/Microsoft/TypeScript/blob/v3.2.2/src/compiler/transformers/ts.ts#L1068-L1082 + * + * @param constructor a constructor function to test + * @returns true if the constructor appears to have been synthesized + */ +function isSynthesizedConstructor(constructor: ts.ConstructorDeclaration): boolean { + if (!constructor.body) return false; + + const firstStatement = constructor.body.statements[0]; + if (!firstStatement || !ts.isExpressionStatement(firstStatement)) return false; + + return isSynthesizedSuperCall(firstStatement.expression); +} + +/** + * Tests whether the expression appears to have been synthesized by TypeScript, i.e. whether + * it is of the following form: + * + * ``` + * super(...arguments); + * ``` + * + * @param expression the expression that is to be tested + * @returns true if the expression appears to be a synthesized super call + */ +function isSynthesizedSuperCall(expression: ts.Expression): boolean { + if (!ts.isCallExpression(expression)) return false; + if (expression.expression.kind !== ts.SyntaxKind.SuperKeyword) return false; + if (expression.arguments.length !== 1) return false; + + const argument = expression.arguments[0]; + return ts.isSpreadElement(argument) && ts.isIdentifier(argument.expression) && + argument.expression.text === 'arguments'; +} 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 9482e7730e..db04510eaa 100644 --- a/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/src/ngcc/src/host/esm5_host.ts @@ -41,6 +41,24 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { return super.isClass(node) || !!this.getClassSymbol(node); } + /** + * Determines whether the given declaration has a base class. + * + * In ES5, we need to determine if the IIFE wrapper takes a `_super` parameter . + */ + hasBaseClass(node: ts.Declaration): boolean { + const classSymbol = this.getClassSymbol(node); + if (!classSymbol) return false; + + const iifeBody = classSymbol.valueDeclaration.parent; + if (!iifeBody || !ts.isBlock(iifeBody)) return false; + + const iife = iifeBody.parent; + if (!iife || !ts.isFunctionExpression(iife)) return false; + + return iife.parameters.length === 1 && isSuperIdentifier(iife.parameters[0].name); + } + /** * Find a symbol for a node that we think is a class. * @@ -137,11 +155,15 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { * @returns an array of `ts.ParameterDeclaration` objects representing each of the parameters in * the class's constructor or null if there is no constructor. */ - protected getConstructorParameterDeclarations(classSymbol: ts.Symbol): ts.ParameterDeclaration[] { + protected getConstructorParameterDeclarations(classSymbol: ts.Symbol): + ts.ParameterDeclaration[]|null { const constructor = classSymbol.valueDeclaration as ts.FunctionDeclaration; - if (constructor && constructor.parameters) { + if (constructor.parameters.length > 0) { return Array.from(constructor.parameters); } + if (isSynthesizedConstructor(constructor)) { + return null; + } return []; } @@ -253,6 +275,147 @@ function reflectArrayElement(element: ts.Expression) { return ts.isObjectLiteralExpression(element) ? reflectObjectLiteral(element) : null; } +/** + * A constructor function may have been "synthesized" by TypeScript during JavaScript emit, + * in the case no user-defined constructor exists and e.g. property initializers are used. + * Those initializers need to be emitted into a constructor in JavaScript, so the TypeScript + * compiler generates a synthetic constructor. + * + * We need to identify such constructors as ngcc needs to be able to tell if a class did + * originally have a constructor in the TypeScript source. For ES5, we can not tell an + * empty constructor apart from a synthesized constructor, but fortunately that does not + * matter for the code generated by ngtsc. + * + * When a class has a superclass however, a synthesized constructor must not be considered + * as a user-defined constructor as that prevents a base factory call from being created by + * ngtsc, resulting in a factory function that does not inject the dependencies of the + * superclass. Hence, we identify a default synthesized super call in the constructor body, + * according to the structure that TypeScript's ES2015 to ES5 transformer generates in + * https://github.com/Microsoft/TypeScript/blob/v3.2.2/src/compiler/transformers/es2015.ts#L1082-L1098 + * + * @param constructor a constructor function to test + * @returns true if the constructor appears to have been synthesized + */ +function isSynthesizedConstructor(constructor: ts.FunctionDeclaration): boolean { + if (!constructor.body) return false; + + const firstStatement = constructor.body.statements[0]; + if (!firstStatement) return false; + + return isSynthesizedSuperThisAssignment(firstStatement) || + isSynthesizedSuperReturnStatement(firstStatement); +} + +/** + * Identifies a synthesized super call of the form: + * + * ``` + * var _this = _super !== null && _super.apply(this, arguments) || this; + * ``` + * + * @param statement a statement that may be a synthesized super call + * @returns true if the statement looks like a synthesized super call + */ +function isSynthesizedSuperThisAssignment(statement: ts.Statement): boolean { + if (!ts.isVariableStatement(statement)) return false; + + const variableDeclarations = statement.declarationList.declarations; + if (variableDeclarations.length !== 1) return false; + + const variableDeclaration = variableDeclarations[0]; + if (!ts.isIdentifier(variableDeclaration.name) || + !variableDeclaration.name.text.startsWith('_this')) + return false; + + const initializer = variableDeclaration.initializer; + if (!initializer) return false; + + return isSynthesizedDefaultSuperCall(initializer); +} +/** + * Identifies a synthesized super call of the form: + * + * ``` + * return _super !== null && _super.apply(this, arguments) || this; + * ``` + * + * @param statement a statement that may be a synthesized super call + * @returns true if the statement looks like a synthesized super call + */ +function isSynthesizedSuperReturnStatement(statement: ts.Statement): boolean { + if (!ts.isReturnStatement(statement)) return false; + + const expression = statement.expression; + if (!expression) return false; + + return isSynthesizedDefaultSuperCall(expression); +} + +/** + * Tests whether the expression is of the form: + * + * ``` + * _super !== null && _super.apply(this, arguments) || this; + * ``` + * + * This structure is generated by TypeScript when transforming ES2015 to ES5, see + * https://github.com/Microsoft/TypeScript/blob/v3.2.2/src/compiler/transformers/es2015.ts#L1148-L1163 + * + * @param expression an expression that may represent a default super call + * @returns true if the expression corresponds with the above form + */ +function isSynthesizedDefaultSuperCall(expression: ts.Expression): boolean { + if (!isBinaryExpr(expression, ts.SyntaxKind.BarBarToken)) return false; + if (expression.right.kind !== ts.SyntaxKind.ThisKeyword) return false; + + const left = expression.left; + if (!isBinaryExpr(left, ts.SyntaxKind.AmpersandAmpersandToken)) return false; + + return isSuperNotNull(left.left) && isSuperApplyCall(left.right); +} + +function isSuperNotNull(expression: ts.Expression): boolean { + return isBinaryExpr(expression, ts.SyntaxKind.ExclamationEqualsEqualsToken) && + isSuperIdentifier(expression.left); +} + +/** + * Tests whether the expression is of the form + * + * ``` + * _super.apply(this, arguments) + * ``` + * + * @param expression an expression that may represent a default super call + * @returns true if the expression corresponds with the above form + */ +function isSuperApplyCall(expression: ts.Expression): boolean { + if (!ts.isCallExpression(expression) || expression.arguments.length !== 2) return false; + + const targetFn = expression.expression; + if (!ts.isPropertyAccessExpression(targetFn)) return false; + if (!isSuperIdentifier(targetFn.expression)) return false; + if (targetFn.name.text !== 'apply') return false; + + const thisArgument = expression.arguments[0]; + if (thisArgument.kind !== ts.SyntaxKind.ThisKeyword) return false; + + const argumentsArgument = expression.arguments[1]; + return ts.isIdentifier(argumentsArgument) && argumentsArgument.text === 'arguments'; +} + +function isBinaryExpr( + expression: ts.Expression, operator: ts.BinaryOperator): expression is ts.BinaryExpression { + return ts.isBinaryExpression(expression) && expression.operatorToken.kind === operator; +} + +function isSuperIdentifier(node: ts.Node): boolean { + // Verify that the identifier is prefixed with `_super`. We don't test for equivalence + // as TypeScript may have suffixed the name, e.g. `_super_1` to avoid name conflicts. + // Requiring only a prefix should be sufficiently accurate. + return ts.isIdentifier(node) && node.text.startsWith('_super'); +} + /** * 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. 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 a68191af74..9248f76586 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 @@ -980,6 +980,70 @@ describe('Fesm2015ReflectionHost', () => { })); }); + describe('synthesized constructors', () => { + function getConstructorParameters(constructor: string) { + const file = { + name: '/synthesized_constructors.js', + contents: ` + class BaseClass {} + class TestClass extends BaseClass { + ${constructor} + } + `, + }; + + const program = makeTestProgram(file); + const host = new Esm2015ReflectionHost(false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', ts.isClassDeclaration); + return host.getConstructorParameters(classNode); + } + + it('recognizes super call as first statement', () => { + const parameters = getConstructorParameters(` + constructor() { + super(...arguments); + this.synthesizedProperty = null; + }`); + + expect(parameters).toBeNull(); + }); + + it('does not consider super call without spread element as synthesized', () => { + const parameters = getConstructorParameters(` + constructor() { + super(arguments); + }`); + + expect(parameters !.length).toBe(0); + }); + + it('does not consider constructors with parameters as synthesized', () => { + const parameters = getConstructorParameters(` + constructor(arg) { + super(...arguments); + }`); + + expect(parameters !.length).toBe(1); + }); + + it('does not consider manual super calls as synthesized', () => { + const parameters = getConstructorParameters(` + constructor() { + super(); + }`); + + expect(parameters !.length).toBe(0); + }); + + it('does not consider empty constructors as synthesized', () => { + const parameters = getConstructorParameters(` + constructor() { + }`); + + expect(parameters !.length).toBe(0); + }); + }); + describe('(returned parameters `decorators`)', () => { it('should ignore param decorator elements that are not object literals', () => { const program = makeTestProgram(INVALID_CTOR_DECORATORS_FILE); 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 7889ae74d3..1501869fc9 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 @@ -946,6 +946,84 @@ describe('Esm5ReflectionHost', () => { }); }); + describe('synthesized constructors', () => { + function getConstructorParameters(constructor: string) { + const file = { + name: '/synthesized_constructors.js', + contents: ` + var TestClass = /** @class */ (function (_super) { + __extends(TestClass, _super); + ${constructor} + return TestClass; + }(null)); + `, + }; + + const program = makeTestProgram(file); + const host = new Esm5ReflectionHost(false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', ts.isVariableDeclaration); + return host.getConstructorParameters(classNode); + } + + it('recognizes _this assignment from super call', () => { + const parameters = getConstructorParameters(` + function TestClass() { + var _this = _super !== null && _super.apply(this, arguments) || this; + _this.synthesizedProperty = null; + return _this; + }`); + + expect(parameters).toBeNull(); + }); + + it('recognizes super call as return statement', () => { + const parameters = getConstructorParameters(` + function TestClass() { + return _super !== null && _super.apply(this, arguments) || this; + }`); + + expect(parameters).toBeNull(); + }); + + it('handles the case where a unique name was generated for _super or _this', () => { + const parameters = getConstructorParameters(` + function TestClass() { + var _this_1 = _super_1 !== null && _super_1.apply(this, arguments) || this; + _this_1._this = null; + _this_1._super = null; + return _this_1; + }`); + + expect(parameters).toBeNull(); + }); + + it('does not consider constructors with parameters as synthesized', () => { + const parameters = getConstructorParameters(` + function TestClass(arg) { + return _super !== null && _super.apply(this, arguments) || this; + }`); + + expect(parameters !.length).toBe(1); + }); + + it('does not consider manual super calls as synthesized', () => { + const parameters = getConstructorParameters(` + function TestClass() { + return _super.call(this) || this; + }`); + + expect(parameters !.length).toBe(0); + }); + + it('does not consider empty constructors as synthesized', () => { + const parameters = getConstructorParameters(` + function TestClass() { + }`); + + expect(parameters !.length).toBe(0); + }); + }); + describe('(returned parameters `decorators.args`)', () => { it('should be an empty array if param decorator has no `args` property', () => { const program = makeTestProgram(INVALID_CTOR_DECORATOR_ARGS_FILE); @@ -1252,6 +1330,51 @@ describe('Esm5ReflectionHost', () => { }); }); + describe('hasBaseClass()', () => { + function hasBaseClass(source: string) { + const file = { + name: '/synthesized_constructors.js', + contents: source, + }; + + const program = makeTestProgram(file); + const host = new Esm5ReflectionHost(false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', ts.isVariableDeclaration); + return host.hasBaseClass(classNode); + } + + it('should consider an IIFE with _super parameter as having a base class', () => { + const result = hasBaseClass(` + var TestClass = /** @class */ (function (_super) { + __extends(TestClass, _super); + function TestClass() {} + return TestClass; + }(null));`); + expect(result).toBe(true); + }); + + it('should consider an IIFE with a unique name generated for the _super parameter as having a base class', + () => { + const result = hasBaseClass(` + var TestClass = /** @class */ (function (_super_1) { + __extends(TestClass, _super_1); + function TestClass() {} + return TestClass; + }(null));`); + expect(result).toBe(true); + }); + + it('should not consider an IIFE without parameter as having a base class', () => { + const result = hasBaseClass(` + var TestClass = /** @class */ (function () { + __extends(TestClass, _super); + function TestClass() {} + return TestClass; + }(null));`); + expect(result).toBe(false); + }); + }); + describe('findDecoratedClasses()', () => { it('should return an array of all decorated classes in the given source file', () => { const program = makeTestProgram(...DECORATED_FILES); diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index c82cadd190..c8b5512694 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -425,6 +425,9 @@ export interface ReflectionHost { */ isClass(node: ts.Node): node is ts.NamedDeclaration; + /** + * Determines whether the given declaration has a base class. + */ hasBaseClass(node: ts.Declaration): boolean; /**