fix(ivy): correctly detect classes in ngcc Esm5ReflectionHost (#25406)

PR Close #25406

packages/compiler-cli/src/ngcc/src/host/esm2015_host.ts

@@ -188,12 +188,11 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
   }
 
   /**
-   * Find a symbol for a declaration that we think is a class.
+   * Find a symbol for a node that we think is a class.
-   * @param declaration The declaration whose symbol we are finding
+   * @param node The node whose symbol we are finding.
-   * @returns the symbol for the declaration or `undefined` if it is not
+   * @returns The symbol for the node or `undefined` if it is not a "class" or has no symbol.
-   * a "class" or has no symbol.
    */
-  getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined {
+  getClassSymbol(declaration: ts.Node): ts.Symbol|undefined {
     return ts.isClassDeclaration(declaration) ?
         declaration.name && this.checker.getSymbolAtLocation(declaration.name) :
         undefined;
@@ -423,4 +422,4 @@ function isNamedDeclaration(node: ts.Declaration): node is ts.NamedDeclaration {
 function isClassMemberType(node: ts.Declaration): node is ts.ClassElement|
     ts.PropertyAccessExpression|ts.BinaryExpression {
   return ts.isClassElement(node) || isPropertyAccess(node) || ts.isBinaryExpression(node);
-}
+}

packages/compiler-cli/src/ngcc/src/host/esm5_host.ts

@@ -33,25 +33,64 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
   constructor(checker: ts.TypeChecker) { super(checker); }
 
   /**
-   * Check whether the given declaration node actually represents a class.
+   * Check whether the given node actually represents a class.
    */
-  isClass(node: ts.Declaration): boolean { return !!this.getClassSymbol(node); }
+  isClass(node: ts.Node): boolean { return super.isClass(node) || !!this.getClassSymbol(node); }
 
   /**
-   * In ESM5 the implementation of a class is a function expression that is hidden inside an IIFE.
+   * Find a symbol for a node that we think is a class.
+   *
+   * In ES5, the implementation of a class is a function expression that is hidden inside an IIFE.
    * So we need to dig around inside to get hold of the "class" symbol.
-   * @param declaration the top level declaration that represents an exported class.
+   *
+   * `node` might be one of:
+   * - A class declaration (from a declaration file).
+   * - The declaration of the outer variable, which is assigned the result of the IIFE.
+   * - The function declaration inside the IIFE, which is eventually returned and assigned to the
+   *   outer variable.
+   *
+   * @param node The top level declaration that represents an exported class or the function
+   *     expression inside the IIFE.
+   * @returns The symbol for the node or `undefined` if it is not a "class" or has no symbol.
    */
-  getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined {
+  getClassSymbol(node: ts.Node): ts.Symbol|undefined {
-    if (ts.isVariableDeclaration(declaration)) {
+    const symbol = super.getClassSymbol(node);
-      const iifeBody = getIifeBody(declaration);
+    if (symbol) return symbol;
-      if (iifeBody) {
+
-        const innerClassIdentifier = getReturnIdentifier(iifeBody);
+    if (ts.isVariableDeclaration(node)) {
-        if (innerClassIdentifier) {
+      const iifeBody = getIifeBody(node);
-          return this.checker.getSymbolAtLocation(innerClassIdentifier);
+      if (!iifeBody) return undefined;
-        }
+
-      }
+      const innerClassIdentifier = getReturnIdentifier(iifeBody);
+      if (!innerClassIdentifier) return undefined;
+
+      return this.checker.getSymbolAtLocation(innerClassIdentifier);
+    } else if (ts.isFunctionDeclaration(node)) {
+      // It might be the function expression inside the IIFE. We need to go 5 levels up...
+
+      // 1. IIFE body.
+      let outerNode = node.parent;
+      if (!outerNode || !ts.isBlock(outerNode)) return undefined;
+
+      // 2. IIFE function expression.
+      outerNode = outerNode.parent;
+      if (!outerNode || !ts.isFunctionExpression(outerNode)) return undefined;
+
+      // 3. IIFE call expression.
+      outerNode = outerNode.parent;
+      if (!outerNode || !ts.isCallExpression(outerNode)) return undefined;
+
+      // 4. Parenthesis around IIFE.
+      outerNode = outerNode.parent;
+      if (!outerNode || !ts.isParenthesizedExpression(outerNode)) return undefined;
+
+      // 5. Outer variable declaration.
+      outerNode = outerNode.parent;
+      if (!outerNode || !ts.isVariableDeclaration(outerNode)) return undefined;
+
+      return this.getClassSymbol(outerNode);
     }
+
     return undefined;
   }
 

packages/compiler-cli/src/ngcc/src/host/ngcc_host.ts

@@ -12,5 +12,5 @@ import {ReflectionHost} from '../../../ngtsc/host';
  * A reflection host that has extra methods for looking at non-Typescript package formats
  */
 export interface NgccReflectionHost extends ReflectionHost {
-  getClassSymbol(declaration: ts.Declaration): ts.Symbol|undefined;
+  getClassSymbol(node: ts.Node): ts.Symbol|undefined;
 }

packages/compiler-cli/src/ngcc/test/host/esm5_host_spec.ts

@@ -8,6 +8,7 @@
 
 import * as ts from 'typescript';
 import {ClassMemberKind, Import} from '../../../ngtsc/host';
+import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
 import {Esm5ReflectionHost} from '../../src/host/esm5_host';
 import {getDeclaration, makeProgram} from '../helpers/utils';
 
@@ -50,7 +51,8 @@ const SIMPLE_CLASS_FILE = {
   name: '/simple_class.js',
   contents: `
     var EmptyClass = (function() {
-      function EmptyClass() {}
+      function EmptyClass() {
+      }
       return EmptyClass;
     }());
     var NoDecoratorConstructorClass = (function() {
@@ -1122,20 +1124,104 @@ describe('Esm5ReflectionHost', () => {
     });
   });
 
-  describe('isClass()', () => {
+  describe('getClassSymbol()', () => {
-    it('should return true if a given node is an ES5 class declaration', () => {
+    let superGetClassSymbolSpy: jasmine.Spy;
+
+    beforeEach(() => {
+      superGetClassSymbolSpy = spyOn(Esm2015ReflectionHost.prototype, 'getClassSymbol');
+    });
+
+    it('should return the class symbol returned by the superclass (if any)', () => {
+      const mockNode = {} as ts.Node;
+      const mockSymbol = {} as ts.Symbol;
+      superGetClassSymbolSpy.and.returnValue(mockSymbol);
+
+      const host = new Esm5ReflectionHost({} as any);
+
+      expect(host.getClassSymbol(mockNode)).toBe(mockSymbol);
+      expect(superGetClassSymbolSpy).toHaveBeenCalledWith(mockNode);
+    });
+
+    it('should return the class symbol for an ES5 class (outer variable declaration)', () => {
       const program = makeProgram(SIMPLE_CLASS_FILE);
       const host = new Esm5ReflectionHost(program.getTypeChecker());
       const node =
           getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration);
-      expect(host.isClass(node)).toBe(true);
+      expect(host.getClassSymbol(node)).toBeDefined();
     });
 
-    it('should return false if a given node is not an ES5 class declaration', () => {
+    it('should return the class symbol for an ES5 class (inner function declaration)', () => {
+      const program = makeProgram(SIMPLE_CLASS_FILE);
+      const host = new Esm5ReflectionHost(program.getTypeChecker());
+      const outerNode =
+          getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration);
+      const innerNode =
+          (((outerNode.initializer as ts.ParenthesizedExpression).expression as ts.CallExpression)
+               .expression as ts.FunctionExpression)
+              .body.statements.find(ts.isFunctionDeclaration) !;
+
+      expect(host.getClassSymbol(innerNode)).toBeDefined();
+    });
+
+    it('should return the same class symbol for outer and inner declarations', () => {
+      const program = makeProgram(SIMPLE_CLASS_FILE);
+      const host = new Esm5ReflectionHost(program.getTypeChecker());
+      const outerNode =
+          getDeclaration(program, SIMPLE_CLASS_FILE.name, 'EmptyClass', ts.isVariableDeclaration);
+      const innerNode =
+          (((outerNode.initializer as ts.ParenthesizedExpression).expression as ts.CallExpression)
+               .expression as ts.FunctionExpression)
+              .body.statements.find(ts.isFunctionDeclaration) !;
+
+      expect(host.getClassSymbol(innerNode)).toBe(host.getClassSymbol(outerNode));
+    });
+
+    it('should return undefined if node is not an ES5 class', () => {
       const program = makeProgram(FOO_FUNCTION_FILE);
       const host = new Esm5ReflectionHost(program.getTypeChecker());
       const node = getDeclaration(program, FOO_FUNCTION_FILE.name, 'foo', ts.isFunctionDeclaration);
-      expect(host.isClass(node)).toBe(false);
+      expect(host.getClassSymbol(node)).toBeUndefined();
+    });
+  });
+
+  describe('isClass()', () => {
+    let host: Esm5ReflectionHost;
+    let mockNode: ts.Node;
+    let superIsClassSpy: jasmine.Spy;
+    let getClassSymbolSpy: jasmine.Spy;
+
+    beforeEach(() => {
+      host = new Esm5ReflectionHost(null as any);
+      mockNode = {} as any;
+
+      superIsClassSpy = spyOn(Esm2015ReflectionHost.prototype, 'isClass');
+      getClassSymbolSpy = spyOn(Esm5ReflectionHost.prototype, 'getClassSymbol');
+    });
+
+    it('should return true if superclass returns true', () => {
+      superIsClassSpy.and.returnValue(true);
+
+      expect(host.isClass(mockNode)).toBe(true);
+      expect(superIsClassSpy).toHaveBeenCalledWith(mockNode);
+      expect(getClassSymbolSpy).not.toHaveBeenCalled();
+    });
+
+    it('should return true if it can find a symbol for the class', () => {
+      superIsClassSpy.and.returnValue(false);
+      getClassSymbolSpy.and.returnValue(true);
+
+      expect(host.isClass(mockNode)).toBe(true);
+      expect(superIsClassSpy).toHaveBeenCalledWith(mockNode);
+      expect(getClassSymbolSpy).toHaveBeenCalledWith(mockNode);
+    });
+
+    it('should return false if it cannot find a symbol for the class', () => {
+      superIsClassSpy.and.returnValue(false);
+      getClassSymbolSpy.and.returnValue(false);
+
+      expect(host.isClass(mockNode)).toBe(false);
+      expect(superIsClassSpy).toHaveBeenCalledWith(mockNode);
+      expect(getClassSymbolSpy).toHaveBeenCalledWith(mockNode);
     });
   });
 });

packages/compiler-cli/src/ngtsc/host/src/reflection.ts

@@ -406,9 +406,9 @@ export interface ReflectionHost {
   getExportsOfModule(module: ts.Node): Map<string, Declaration>|null;
 
   /**
-   * Check whether the given declaration node actually represents a class.
+   * Check whether the given node actually represents a class.
    */
-  isClass(node: ts.Declaration): boolean;
+  isClass(node: ts.Node): boolean;
 
   hasBaseClass(node: ts.Declaration): boolean;
 }

packages/compiler-cli/src/ngtsc/metadata/src/reflector.ts

@@ -127,7 +127,7 @@ export class TypeScriptReflectionHost implements ReflectionHost {
     return map;
   }
 
-  isClass(node: ts.Declaration): boolean {
+  isClass(node: ts.Node): boolean {
     // In TypeScript code, classes are ts.ClassDeclarations.
     return ts.isClassDeclaration(node);
   }