From 98f8b0f32833a445290dbf131974c8419b054009 Mon Sep 17 00:00:00 2001 From: JoostK Date: Tue, 5 Mar 2019 23:29:28 +0100 Subject: [PATCH] fix(ivy): ngcc - properly handle aliases class expressions (#29119) In ES2015, classes could have been emitted as a variable declaration initialized with a class expression. In certain situations, an intermediary variable suffixed with `_1` is present such that the variable declaration's initializer becomes a binary expression with its rhs being the class expression, and its lhs being the identifier of the intermediate variable. This structure was not recognized, resulting in such classes not being considered as a class in `Esm2015ReflectionHost`. As a consequence, the analysis of functions/methods that return a `ModuleWithProviders` object did not take the methods of such classes into account. Another edge-case with such intermediate variable was that static properties would not be considered as class members. A testcase was added to prevent regressions. Fixes #29078 PR Close #29119 --- .../module_with_providers_analyzer.ts | 20 +- .../ngcc/src/host/esm2015_host.ts | 269 ++++++++++++++++-- .../compiler-cli/ngcc/src/host/esm5_host.ts | 11 +- .../compiler-cli/ngcc/src/host/ngcc_host.ts | 7 +- .../ngcc/src/rendering/renderer.ts | 2 +- packages/compiler-cli/ngcc/src/utils.ts | 2 +- .../host/esm2015_host_import_helper_spec.ts | 17 ++ .../ngcc/test/host/esm2015_host_spec.ts | 128 ++++++++- .../test/host/esm5_host_import_helper_spec.ts | 2 +- .../ngcc/test/host/esm5_host_spec.ts | 41 ++- .../src/ngtsc/reflection/src/host.ts | 4 +- .../src/ngtsc/util/src/typescript.ts | 2 +- 12 files changed, 431 insertions(+), 74 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts index fd24d5c727..11894f9d13 100644 --- a/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/module_with_providers_analyzer.ts @@ -9,9 +9,9 @@ import * as ts from 'typescript'; import {ReferencesRegistry} from '../../../src/ngtsc/annotations'; import {Reference} from '../../../src/ngtsc/imports'; -import {Declaration} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, Declaration} from '../../../src/ngtsc/reflection'; import {ModuleWithProvidersFunction, NgccReflectionHost} from '../host/ngcc_host'; -import {isDefined} from '../utils'; +import {hasNameIdentifier, isDefined} from '../utils'; export interface ModuleWithProvidersInfo { /** @@ -23,7 +23,7 @@ export interface ModuleWithProvidersInfo { /** * The NgModule class declaration (in the .d.ts file) to add as a type parameter. */ - ngModule: Declaration; + ngModule: Declaration; } export type ModuleWithProvidersAnalyses = Map; @@ -44,11 +44,7 @@ export class ModuleWithProvidersAnalyzer { null; if (!typeParam || isAnyKeyword(typeParam)) { // Either we do not have a parameterized type or the type is `any`. - let ngModule = this.host.getDeclarationOfIdentifier(fn.ngModule); - if (!ngModule) { - throw new Error( - `Cannot find a declaration for NgModule ${fn.ngModule.text} referenced in ${fn.declaration.getText()}`); - } + let ngModule = fn.ngModule; // For internal (non-library) module references, redirect the module's value declaration // to its type declaration. if (ngModule.viaModule === null) { @@ -57,14 +53,12 @@ export class ModuleWithProvidersAnalyzer { throw new Error( `No typings declaration can be found for the referenced NgModule class in ${fn.declaration.getText()}.`); } - if (!ts.isClassDeclaration(dtsNgModule)) { + if (!ts.isClassDeclaration(dtsNgModule) || !hasNameIdentifier(dtsNgModule)) { throw new Error( - `The referenced NgModule in ${fn.declaration.getText()} is not a class declaration in the typings program; instead we get ${dtsNgModule.getText()}`); + `The referenced NgModule in ${fn.declaration.getText()} is not a named class declaration in the typings program; instead we get ${dtsNgModule.getText()}`); } // Record the usage of the internal module as it needs to become an exported symbol - const reference = new Reference(ngModule.node); - reference.addIdentifier(fn.ngModule); - this.referencesRegistry.add(ngModule.node, reference); + this.referencesRegistry.add(ngModule.node, new Reference(ngModule.node)); ngModule = {node: dtsNgModule, viaModule: null}; } diff --git a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts index 1ee3f154aa..3758af6851 100644 --- a/packages/compiler-cli/ngcc/src/host/esm2015_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm2015_host.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Decorator, Import, TypeScriptReflectionHost, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Declaration, Decorator, Import, TypeScriptReflectionHost, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; import {Logger} from '../logging/logger'; import {BundleProgram} from '../packages/bundle_program'; import {findAll, getNameText, hasNameIdentifier, isDefined} from '../utils'; @@ -50,6 +50,29 @@ export const CONSTRUCTOR_PARAMS = 'ctorParameters' as ts.__String; */ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements NgccReflectionHost { protected dtsDeclarationMap: Map|null; + + /** + * The set of source files that have already been preprocessed. + */ + protected preprocessedSourceFiles = new Set(); + + /** + * In ES2015, class declarations may have been down-leveled into variable declarations, + * initialized using a class expression. In certain scenarios, an additional variable + * is introduced that represents the class so that results in code such as: + * + * ``` + * let MyClass_1; let MyClass = MyClass_1 = class MyClass {}; + * ``` + * + * This map tracks those aliased variables to their original identifier, i.e. the key + * corresponds with the declaration of `MyClass_1` and its value becomes the `MyClass` identifier + * of the variable declaration. + * + * This map is populated during the preprocessing of each source file. + */ + protected aliasedClassDeclarations = new Map(); + constructor( protected logger: Logger, protected isCore: boolean, checker: ts.TypeChecker, dts?: BundleProgram|null) { @@ -62,19 +85,20 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N * Classes should have a `name` identifier, because they may need to be referenced in other parts * of the program. * + * In ES2015, a class may be declared using a variable declaration of the following structure: + * + * ``` + * var MyClass = MyClass_1 = class MyClass {}; + * ``` + * + * Here, the intermediate `MyClass_1` assignment is optional. In the above example, the + * `class MyClass {}` node is returned as declaration of `MyClass`. + * * @param node the node that represents the class whose declaration we are finding. * @returns the declaration of the class or `undefined` if it is not a "class". */ getClassDeclaration(node: ts.Node): ClassDeclaration|undefined { - if (ts.isVariableDeclaration(node) && node.initializer) { - node = node.initializer; - } - - if (!ts.isClassDeclaration(node) && !ts.isClassExpression(node)) { - return undefined; - } - - return hasNameIdentifier(node) ? node : undefined; + return getInnerClassDeclaration(node) || undefined; } /** @@ -155,6 +179,60 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N return null; } + hasBaseClass(clazz: ClassDeclaration): boolean { + const superHasBaseClass = super.hasBaseClass(clazz); + if (superHasBaseClass) { + return superHasBaseClass; + } + + const innerClassDeclaration = getInnerClassDeclaration(clazz); + if (innerClassDeclaration === null) { + return false; + } + + return innerClassDeclaration.heritageClauses !== undefined && + innerClassDeclaration.heritageClauses.some( + clause => clause.token === ts.SyntaxKind.ExtendsKeyword); + } + + /** + * Check whether the given node actually represents a class. + */ + isClass(node: ts.Node): node is ClassDeclaration { + return super.isClass(node) || !!this.getClassDeclaration(node); + } + + /** + * Trace an identifier to its declaration, if possible. + * + * This method attempts to resolve the declaration of the given identifier, tracing back through + * imports and re-exports until the original declaration statement is found. A `Declaration` + * object is returned if the original declaration is found, or `null` is returned otherwise. + * + * In ES2015, we need to account for identifiers that refer to aliased class declarations such as + * `MyClass_1`. Since such declarations are only available within the module itself, we need to + * find the original class declaration, e.g. `MyClass`, that is associated with the aliased one. + * + * @param id a TypeScript `ts.Identifier` to trace back to a declaration. + * + * @returns metadata about the `Declaration` if the original declaration is found, or `null` + * otherwise. + */ + getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null { + const superDeclaration = super.getDeclarationOfIdentifier(id); + + // The identifier may have been of an additional class assignment such as `MyClass_1` that was + // present as alias for `MyClass`. If so, resolve such aliases to their original declaration. + if (superDeclaration !== null) { + const aliasedIdentifier = this.resolveAliasedClassIdentifier(superDeclaration.node); + if (aliasedIdentifier !== null) { + return this.getDeclarationOfIdentifier(aliasedIdentifier); + } + } + + return superDeclaration; + } + /** * Search the given module for variable declarations in which the initializer * is an identifier marked with the `PRE_R3_MARKER`. @@ -341,6 +419,74 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N ///////////// Protected Helpers ///////////// + /** + * Finds the identifier of the actual class declaration for a potentially aliased declaration of a + * class. + * + * If the given declaration is for an alias of a class, this function will determine an identifier + * to the original declaration that represents this class. + * + * @param declaration The declaration to resolve. + * @returns The original identifier that the given class declaration resolves to, or `undefined` + * if the declaration does not represent an aliased class. + */ + protected resolveAliasedClassIdentifier(declaration: ts.Declaration): ts.Identifier|null { + this.ensurePreprocessed(declaration.getSourceFile()); + return this.aliasedClassDeclarations.get(declaration) || null; + } + + /** + * Ensures that the source file that `node` is part of has been preprocessed. + * + * During preprocessing, all statements in the source file will be visited such that certain + * processing steps can be done up-front and cached for subsequent usages. + * + * @param sourceFile The source file that needs to have gone through preprocessing. + */ + protected ensurePreprocessed(sourceFile: ts.SourceFile): void { + if (!this.preprocessedSourceFiles.has(sourceFile)) { + this.preprocessedSourceFiles.add(sourceFile); + + for (const statement of sourceFile.statements) { + this.preprocessStatement(statement); + } + } + } + + /** + * Analyzes the given statement to see if it corresponds with a variable declaration like + * `let MyClass = MyClass_1 = class MyClass {};`. If so, the declaration of `MyClass_1` + * is associated with the `MyClass` identifier. + * + * @param statement The statement that needs to be preprocessed. + */ + protected preprocessStatement(statement: ts.Statement): void { + if (!ts.isVariableStatement(statement)) { + return; + } + + const declarations = statement.declarationList.declarations; + if (declarations.length !== 1) { + return; + } + + const declaration = declarations[0]; + const initializer = declaration.initializer; + if (!ts.isIdentifier(declaration.name) || !initializer || !isAssignment(initializer) || + !ts.isIdentifier(initializer.left) || !ts.isClassExpression(initializer.right)) { + return; + } + + const aliasedIdentifier = initializer.left; + + const aliasedDeclaration = this.getDeclarationOfIdentifier(aliasedIdentifier); + if (aliasedDeclaration === null) { + throw new Error( + `Unable to locate declaration of ${aliasedIdentifier.text} in "${statement.getText()}"`); + } + this.aliasedClassDeclarations.set(aliasedDeclaration.node, declaration.name); + } + protected getDecoratorsOfSymbol(symbol: ClassSymbol): Decorator[]|null { const decoratorsProperty = this.getStaticProperty(symbol, DECORATORS); if (decoratorsProperty) { @@ -492,8 +638,9 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N // } // MyClass.staticProperty = ...; // ``` - if (ts.isVariableDeclaration(symbol.valueDeclaration.parent)) { - const variableSymbol = this.checker.getSymbolAtLocation(symbol.valueDeclaration.parent.name); + const variableDeclaration = getVariableDeclarationOfDeclaration(symbol.valueDeclaration); + if (variableDeclaration !== undefined) { + const variableSymbol = this.checker.getSymbolAtLocation(variableDeclaration.name); if (variableSymbol && variableSymbol.exports) { variableSymbol.exports.forEach((value, key) => { const decorators = decoratorsMap.get(key as string); @@ -703,7 +850,6 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N } - /** * Reflect over the given array node and extract decorator information from each element. * @@ -1166,12 +1312,13 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N protected parseForModuleWithProviders( name: string, node: ts.Node|null, implementation: ts.Node|null = node, container: ts.Declaration|null = null): ModuleWithProvidersFunction|null { - const declaration = implementation && - (ts.isFunctionDeclaration(implementation) || ts.isMethodDeclaration(implementation) || - ts.isFunctionExpression(implementation)) ? - implementation : - null; - const body = declaration ? this.getDefinitionOfFunction(declaration).body : null; + if (implementation === null || + (!ts.isFunctionDeclaration(implementation) && !ts.isMethodDeclaration(implementation) && + !ts.isFunctionExpression(implementation))) { + return null; + } + const declaration = implementation; + const body = this.getDefinitionOfFunction(declaration).body; const lastStatement = body && body[body.length - 1]; const returnExpression = lastStatement && ts.isReturnStatement(lastStatement) && lastStatement.expression || null; @@ -1180,10 +1327,27 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N prop => !!prop.name && ts.isIdentifier(prop.name) && prop.name.text === 'ngModule') || null; - const ngModule = ngModuleProperty && ts.isPropertyAssignment(ngModuleProperty) && + const ngModuleIdentifier = ngModuleProperty && ts.isPropertyAssignment(ngModuleProperty) && ts.isIdentifier(ngModuleProperty.initializer) && ngModuleProperty.initializer || null; - return ngModule && declaration && {name, ngModule, declaration, container}; + + // If no `ngModule` property was found in an object literal return value, return `null` to + // indicate that the provided node does not appear to be a `ModuleWithProviders` function. + if (ngModuleIdentifier === null) { + return null; + } + + const ngModuleDeclaration = this.getDeclarationOfIdentifier(ngModuleIdentifier); + if (!ngModuleDeclaration) { + throw new Error( + `Cannot find a declaration for NgModule ${ngModuleIdentifier.text} referenced in "${declaration!.getText()}"`); + } + if (!hasNameIdentifier(ngModuleDeclaration.node)) { + return null; + } + const ngModule = ngModuleDeclaration as Declaration; + + return {name, ngModule, declaration, container}; } } @@ -1209,10 +1373,8 @@ export function isAssignmentStatement(statement: ts.Statement): statement is Ass ts.isIdentifier(statement.expression.left); } -export function isAssignment(expression: ts.Expression): - expression is ts.AssignmentExpression { - return ts.isBinaryExpression(expression) && - expression.operatorToken.kind === ts.SyntaxKind.EqualsToken; +export function isAssignment(node: ts.Node): node is ts.AssignmentExpression { + return ts.isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.EqualsToken; } /** @@ -1264,6 +1426,38 @@ function getCalleeName(call: ts.CallExpression): string|null { ///////////// Internal Helpers ///////////// +/** + * In ES2015, a class may be declared using a variable declaration of the following structure: + * + * ``` + * var MyClass = MyClass_1 = class MyClass {}; + * ``` + * + * Here, the intermediate `MyClass_1` assignment is optional. In the above example, the + * `class MyClass {}` expression is returned as declaration of `MyClass`. Note that if `node` + * represents a regular class declaration, it will be returned as-is. + * + * @param node the node that represents the class whose declaration we are finding. + * @returns the declaration of the class or `null` if it is not a "class". + */ +function getInnerClassDeclaration(node: ts.Node): + ClassDeclaration|null { + // Recognize a variable declaration of the form `var MyClass = class MyClass {}` or + // `var MyClass = MyClass_1 = class MyClass {};` + if (ts.isVariableDeclaration(node) && node.initializer !== undefined) { + node = node.initializer; + while (isAssignment(node)) { + node = node.right; + } + } + + if (!ts.isClassDeclaration(node) && !ts.isClassExpression(node)) { + return null; + } + + return hasNameIdentifier(node) ? node : null; +} + function getDecoratorArgs(node: ts.ObjectLiteralExpression): ts.Expression[] { // The arguments of a decorator are held in the `args` property of its declaration object. const argsProperty = node.properties.filter(ts.isPropertyAssignment) @@ -1333,6 +1527,31 @@ function collectExportedDeclarations( } } +/** + * Attempt to resolve the variable declaration that the given declaration is assigned to. + * For example, for the following code: + * + * ``` + * var MyClass = MyClass_1 = class MyClass {}; + * ``` + * + * and the provided declaration being `class MyClass {}`, this will return the `var MyClass` + * declaration. + * + * @param declaration The declaration for which any variable declaration should be obtained. + * @returns the outer variable declaration if found, undefined otherwise. + */ +function getVariableDeclarationOfDeclaration(declaration: ts.Declaration): ts.VariableDeclaration| + undefined { + let node = declaration.parent; + + // Detect an intermediary variable assignment and skip over it. + if (isAssignment(node) && ts.isIdentifier(node.left)) { + node = node.parent; + } + + return ts.isVariableDeclaration(node) ? node : undefined; +} /** * A constructor function may have been "synthesized" by TypeScript during JavaScript emit, diff --git a/packages/compiler-cli/ngcc/src/host/esm5_host.ts b/packages/compiler-cli/ngcc/src/host/esm5_host.ts index 235f2d81b3..a2354a40db 100644 --- a/packages/compiler-cli/ngcc/src/host/esm5_host.ts +++ b/packages/compiler-cli/ngcc/src/host/esm5_host.ts @@ -9,6 +9,7 @@ import * as ts from 'typescript'; import {ClassDeclaration, ClassMember, ClassMemberKind, ClassSymbol, CtorParameter, Declaration, Decorator, FunctionDefinition, Parameter, isNamedVariableDeclaration, reflectObjectLiteral} from '../../../src/ngtsc/reflection'; +import {isFromDtsFile} from '../../../src/ngtsc/util/src/typescript'; import {getNameText, hasNameIdentifier} from '../utils'; import {Esm2015ReflectionHost, ParamInfo, getPropertyValueFromSymbol, isAssignmentStatement} from './esm2015_host'; @@ -33,11 +34,6 @@ import {Esm2015ReflectionHost, ParamInfo, getPropertyValueFromSymbol, isAssignme * */ export class Esm5ReflectionHost extends Esm2015ReflectionHost { - /** - * Check whether the given node actually represents a class. - */ - isClass(node: ts.Node): node is ClassDeclaration { return !!this.getClassDeclaration(node); } - /** * Determines whether the given declaration, which should be a "class", has a base "class". * @@ -180,7 +176,10 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost { * @throws if `declaration` does not resolve to a class declaration. */ getMembersOfClass(clazz: ClassDeclaration): ClassMember[] { - if (super.isClass(clazz)) return super.getMembersOfClass(clazz); + // Do not follow ES5's resolution logic when the node resides in a .d.ts file. + if (isFromDtsFile(clazz)) { + return super.getMembersOfClass(clazz); + } // The necessary info is on the inner function declaration (inside the ES5 class IIFE). const innerFunctionSymbol = this.getInnerFunctionSymbolFromClassDeclaration(clazz); diff --git a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts index 1ddfe6dd54..f1462db497 100644 --- a/packages/compiler-cli/ngcc/src/host/ngcc_host.ts +++ b/packages/compiler-cli/ngcc/src/host/ngcc_host.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {ClassSymbol, ReflectionHost} from '../../../src/ngtsc/reflection'; +import {ClassDeclaration, ClassSymbol, Declaration, ReflectionHost} from '../../../src/ngtsc/reflection'; import {DecoratedClass} from './decorated_class'; export const PRE_R3_MARKER = '__PRE_R3__'; @@ -37,9 +37,10 @@ export interface ModuleWithProvidersFunction { */ container: ts.Declaration|null; /** - * The identifier of the `ngModule` property on the `ModuleWithProviders` object. + * The declaration of the class that the `ngModule` property on the `ModuleWithProviders` object + * refers to. */ - ngModule: ts.Identifier; + ngModule: Declaration; } /** diff --git a/packages/compiler-cli/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/ngcc/src/rendering/renderer.ts index 89c2502023..b8fc9ae370 100644 --- a/packages/compiler-cli/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/renderer.ts @@ -204,7 +204,7 @@ export abstract class Renderer { outputText: MagicString, moduleWithProviders: ModuleWithProvidersInfo[], importManager: ImportManager): void { moduleWithProviders.forEach(info => { - const ngModuleName = (info.ngModule.node as ts.ClassDeclaration).name !.text; + const ngModuleName = info.ngModule.node.name.text; const declarationFile = info.declaration.getSourceFile().fileName; const ngModuleFile = info.ngModule.node.getSourceFile().fileName; const importPath = info.ngModule.viaModule || diff --git a/packages/compiler-cli/ngcc/src/utils.ts b/packages/compiler-cli/ngcc/src/utils.ts index ceef984d94..a1a8a4e2fd 100644 --- a/packages/compiler-cli/ngcc/src/utils.ts +++ b/packages/compiler-cli/ngcc/src/utils.ts @@ -44,7 +44,7 @@ export function findAll(node: ts.Node, test: (node: ts.Node) => node is ts.No /** * Does the given declaration have a name which is an identifier? * @param declaration The declaration to test. - * @returns true if the declaration has an identifer for a name. + * @returns true if the declaration has an identifier for a name. */ export function hasNameIdentifier(declaration: ts.Declaration): declaration is ts.Declaration& {name: ts.Identifier} { diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts index 3d71518167..da5834541a 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts @@ -85,6 +85,7 @@ const FILES = [ }; } }; + HttpClientXsrfModule.staticProperty = 'static'; HttpClientXsrfModule = HttpClientXsrfModule_1 = tslib_1.__decorate([ NgModule({ providers: [], @@ -224,6 +225,22 @@ describe('Fesm2015ReflectionHost [import helper style]', () => { expect(staticProperty.value !.getText()).toEqual(`'static'`); }); + it('should find static properties on a class that has an intermediate variable assignment', + () => { + const program = makeTestProgram(fileSystem.files[2]); + const host = + new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration( + program, '/ngmodule.js', 'HttpClientXsrfModule', isNamedVariableDeclaration); + + const members = host.getMembersOfClass(classNode); + const staticProperty = members.find(member => member.name === 'staticProperty') !; + expect(staticProperty.kind).toEqual(ClassMemberKind.Property); + expect(staticProperty.isStatic).toEqual(true); + expect(ts.isPropertyAccessExpression(staticProperty.implementation !)).toEqual(true); + expect(staticProperty.value !.getText()).toEqual(`'static'`); + }); + it('should use `getImportOfIdentifier()` to retrieve import info', () => { const spy = spyOn(Esm2015ReflectionHost.prototype, 'getImportOfIdentifier').and.returnValue({}); diff --git a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts index 416151d408..1e61fc583c 100644 --- a/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm2015_host_spec.ts @@ -83,6 +83,16 @@ const SIMPLE_CLASS_FILE = { `, }; +const CLASS_EXPRESSION_FILE = { + name: '/class_expression.js', + contents: ` + var AliasedClass_1; + let EmptyClass = class EmptyClass {}; + let AliasedClass = AliasedClass_1 = class AliasedClass {} + let usageOfAliasedClass = AliasedClass_1; + `, +}; + const FOO_FUNCTION_FILE = { name: '/foo_function.js', contents: ` @@ -551,7 +561,17 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ } ` }, - {name: '/src/module', contents: 'export class ExternalModule {}'}, + { + name: '/src/aliased_class.js', + contents: ` + var AliasedModule_1; + let AliasedModule = AliasedModule_1 = class AliasedModule { + static forRoot() { return { ngModule: AliasedModule_1 }; } + }; + export { AliasedModule }; + ` + }, + {name: '/src/module.js', contents: 'export class ExternalModule {}'}, ]; describe('Esm2015ReflectionHost', () => { @@ -1327,6 +1347,18 @@ describe('Esm2015ReflectionHost', () => { expect(actualDeclaration !.node).toBe(expectedDeclarationNode); expect(actualDeclaration !.viaModule).toBe('@angular/core'); }); + + it('should return the original declaration of an aliased class', () => { + const program = makeTestProgram(CLASS_EXPRESSION_FILE); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classDeclaration = getDeclaration( + program, CLASS_EXPRESSION_FILE.name, 'AliasedClass', ts.isVariableDeclaration); + const usageOfAliasedClass = getDeclaration( + program, CLASS_EXPRESSION_FILE.name, 'usageOfAliasedClass', ts.isVariableDeclaration); + const aliasedClassIdentifier = usageOfAliasedClass.initializer as ts.Identifier; + expect(aliasedClassIdentifier.text).toBe('AliasedClass_1'); + expect(host.getDeclarationOfIdentifier(aliasedClassIdentifier) !.node).toBe(classDeclaration); + }); }); describe('getExportsOfModule()', () => { @@ -1373,6 +1405,23 @@ describe('Esm2015ReflectionHost', () => { expect(host.isClass(node)).toBe(true); }); + it('should return true if a given node is a class expression assigned into a variable', () => { + const program = makeTestProgram(CLASS_EXPRESSION_FILE); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const node = getDeclaration( + program, CLASS_EXPRESSION_FILE.name, 'EmptyClass', ts.isVariableDeclaration); + expect(host.isClass(node)).toBe(true); + }); + + it('should return true if a given node is a class expression assigned into two variables', + () => { + const program = makeTestProgram(CLASS_EXPRESSION_FILE); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const node = getDeclaration( + program, CLASS_EXPRESSION_FILE.name, 'AliasedClass', ts.isVariableDeclaration); + expect(host.isClass(node)).toBe(true); + }); + it('should return false if a given node is a TS function declaration', () => { const program = makeTestProgram(FOO_FUNCTION_FILE); const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); @@ -1382,6 +1431,46 @@ describe('Esm2015ReflectionHost', () => { }); }); + describe('hasBaseClass()', () => { + it('should not consider a class without extends clause as having a base class', () => { + const file = { + name: '/base_class.js', + contents: `class TestClass {}`, + }; + const program = makeTestProgram(file); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', isNamedClassDeclaration); + expect(host.hasBaseClass(classNode)).toBe(false); + }); + + it('should consider a class with extends clause as having a base class', () => { + const file = { + name: '/base_class.js', + contents: ` + class BaseClass {} + class TestClass extends BaseClass {}`, + }; + const program = makeTestProgram(file); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', isNamedClassDeclaration); + expect(host.hasBaseClass(classNode)).toBe(true); + }); + + it('should consider an aliased class with extends clause as having a base class', () => { + const file = { + name: '/base_class.js', + contents: ` + let TestClass_1; + class BaseClass {} + let TestClass = TestClass_1 = class TestClass extends BaseClass {}`, + }; + const program = makeTestProgram(file); + const host = new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker()); + const classNode = getDeclaration(program, file.name, 'TestClass', isNamedVariableDeclaration); + expect(host.hasBaseClass(classNode)).toBe(true); + }); + }); + describe('getGenericArityOfClass()', () => { it('should properly count type parameters', () => { const program = makeTestProgram(ARITY_CLASSES[0]); @@ -1554,12 +1643,13 @@ describe('Esm2015ReflectionHost', () => { new Esm2015ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); const file = srcProgram.getSourceFile('/src/functions.js') !; const fns = host.getModuleWithProvidersFunctions(file); - expect(fns.map(info => [info.declaration.name !.getText(), info.ngModule.text])).toEqual([ - ['ngModuleIdentifier', 'InternalModule'], - ['ngModuleWithEmptyProviders', 'InternalModule'], - ['ngModuleWithProviders', 'InternalModule'], - ['externalNgModule', 'ExternalModule'], - ]); + expect(fns.map(fn => [fn.declaration.name !.getText(), fn.ngModule.node.name.text])) + .toEqual([ + ['ngModuleIdentifier', 'InternalModule'], + ['ngModuleWithEmptyProviders', 'InternalModule'], + ['ngModuleWithProviders', 'InternalModule'], + ['externalNgModule', 'ExternalModule'], + ]); }); it('should find every static method on exported classes that return an object that looks like a ModuleWithProviders object', @@ -1569,12 +1659,24 @@ describe('Esm2015ReflectionHost', () => { new Esm2015ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); const file = srcProgram.getSourceFile('/src/methods.js') !; const fn = host.getModuleWithProvidersFunctions(file); - expect(fn.map(fn => [fn.declaration.name !.getText(), fn.ngModule.text])).toEqual([ - ['ngModuleIdentifier', 'InternalModule'], - ['ngModuleWithEmptyProviders', 'InternalModule'], - ['ngModuleWithProviders', 'InternalModule'], - ['externalNgModule', 'ExternalModule'], - ]); + expect(fn.map(fn => [fn.declaration.name !.getText(), fn.ngModule.node.name.text])) + .toEqual([ + ['ngModuleIdentifier', 'InternalModule'], + ['ngModuleWithEmptyProviders', 'InternalModule'], + ['ngModuleWithProviders', 'InternalModule'], + ['externalNgModule', 'ExternalModule'], + ]); }); + + // https://github.com/angular/angular/issues/29078 + it('should resolve aliased module references to their original declaration', () => { + const srcProgram = makeTestProgram(...MODULE_WITH_PROVIDERS_PROGRAM); + const host = new Esm2015ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); + const file = srcProgram.getSourceFile('/src/aliased_class.js') !; + const fn = host.getModuleWithProvidersFunctions(file); + expect(fn.map(fn => [fn.declaration.name !.getText(), fn.ngModule.node.name.text])).toEqual([ + ['forRoot', 'AliasedModule'], + ]); + }); }); }); diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts index ca6c593123..bcaadcc8a9 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts @@ -450,4 +450,4 @@ function findIdentifier( function isNgModulePropertyAssignment(identifier: ts.Identifier): boolean { return ts.isPropertyAssignment(identifier.parent) && identifier.parent.name.getText() === 'ngModule'; -} \ No newline at end of file +} diff --git a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts index f94e131524..6f5430b93b 100644 --- a/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts +++ b/packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts @@ -700,7 +700,20 @@ const MODULE_WITH_PROVIDERS_PROGRAM = [ export {SomeService, InternalModule}; ` }, - {name: '/src/module', contents: 'export class ExternalModule {}'}, + { + name: '/src/aliased_class.js', + contents: ` + var AliasedModule = (function() { + function AliasedModule() {} + AliasedModule_1 = AliasedModule; + AliasedModule.forRoot = function() { return { ngModule: AliasedModule_1 }; }; + var AliasedModule_1; + return AliasedModule; + }()); + export { AliasedModule }; + ` + }, + {name: '/src/module.js', contents: 'export class ExternalModule {}'}, ]; describe('Esm5ReflectionHost', () => { @@ -1832,12 +1845,13 @@ describe('Esm5ReflectionHost', () => { const host = new Esm5ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); const file = srcProgram.getSourceFile('/src/functions.js') !; const fns = host.getModuleWithProvidersFunctions(file); - expect(fns.map(info => [info.declaration.name !.getText(), info.ngModule.text])).toEqual([ - ['ngModuleIdentifier', 'InternalModule'], - ['ngModuleWithEmptyProviders', 'InternalModule'], - ['ngModuleWithProviders', 'InternalModule'], - ['externalNgModule', 'ExternalModule'], - ]); + expect(fns.map(fn => [fn.declaration.name !.getText(), fn.ngModule.node.name.text])) + .toEqual([ + ['ngModuleIdentifier', 'InternalModule'], + ['ngModuleWithEmptyProviders', 'InternalModule'], + ['ngModuleWithProviders', 'InternalModule'], + ['externalNgModule', 'ExternalModule'], + ]); }); it('should find every static method on exported classes that return an object that looks like a ModuleWithProviders object', @@ -1846,7 +1860,7 @@ describe('Esm5ReflectionHost', () => { const host = new Esm5ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); const file = srcProgram.getSourceFile('/src/methods.js') !; const fn = host.getModuleWithProvidersFunctions(file); - expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.text])).toEqual([ + expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([ [ 'function() { return { ngModule: InternalModule }; }', 'InternalModule', @@ -1865,5 +1879,16 @@ describe('Esm5ReflectionHost', () => { ], ]); }); + + // https://github.com/angular/angular/issues/29078 + it('should resolve aliased module references to their original declaration', () => { + const srcProgram = makeTestProgram(...MODULE_WITH_PROVIDERS_PROGRAM); + const host = new Esm5ReflectionHost(new MockLogger(), false, srcProgram.getTypeChecker()); + const file = srcProgram.getSourceFile('/src/aliased_class.js') !; + const fn = host.getModuleWithProvidersFunctions(file); + expect(fn.map(fn => [fn.declaration.getText(), fn.ngModule.node.name.text])).toEqual([ + ['function() { return { ngModule: AliasedModule_1 }; }', 'AliasedModule'], + ]); + }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index aefa5edd99..4e87a1409e 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -317,11 +317,11 @@ export interface Import { * The declaration of a symbol, along with information about how it was imported into the * application. */ -export interface Declaration { +export interface Declaration { /** * TypeScript reference to the declaration itself. */ - node: ts.Declaration; + node: T; /** * The absolute module path from which the symbol was imported into the application, if the symbol diff --git a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts index a63ded5d28..d9df799747 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/typescript.ts @@ -25,7 +25,7 @@ export function isFromDtsFile(node: ts.Node): boolean { if (sf === undefined) { sf = ts.getOriginalNode(node).getSourceFile(); } - return sf !== undefined && D_TS.test(sf.fileName); + return sf !== undefined && sf.isDeclarationFile; } export function nodeNameForError(node: ts.Node & {name?: ts.Node}): string {