refactor(core): clean up support for ES2015 constructor delegation (#30368)

This commit moves the delegated constructor detection to a helper
function and also adds more test coverage.

The original code for this came from https://github.com/angular/angular/pull/24156
thanks to @ts2do.

Closes #24156
Closes #27267

// FW-1310

PR Close #30368

packages/core/src/reflection/reflection_capabilities.ts

@@ -26,6 +26,19 @@ export const INHERITED_CLASS_WITH_CTOR =
 export const INHERITED_CLASS_WITH_DELEGATE_CTOR =
     /^class\s+[A-Za-z\d$_]*\s*extends\s+[^{]+{[\s\S]*constructor\s*\(\)\s*{\s+super\(\.\.\.arguments\)/;
 
+/**
+ * Determine whether a stringified type is a class which delegates its constructor
+ * to its parent.
+ *
+ * This is not trivial since compiled code can actually contain a constructor function
+ * even if the original source code did not. For instance, when the child class contains
+ * an initialized instance property.
+ */
+export function isDelegateCtor(typeStr: string): boolean {
+  return DELEGATE_CTOR.test(typeStr) || INHERITED_CLASS_WITH_DELEGATE_CTOR.test(typeStr) ||
+      (INHERITED_CLASS.test(typeStr) && !INHERITED_CLASS_WITH_CTOR.test(typeStr));
+}
+
 export class ReflectionCapabilities implements PlatformReflectionCapabilities {
   private _reflect: any;
 
@@ -72,8 +85,7 @@ export class ReflectionCapabilities implements PlatformReflectionCapabilities {
     // This also helps to work around for https://github.com/Microsoft/TypeScript/issues/12439
     // that sets 'design:paramtypes' to []
     // if a class inherits from another class but has no ctor declared itself.
-    if (DELEGATE_CTOR.exec(typeStr) || INHERITED_CLASS_WITH_DELEGATE_CTOR.exec(typeStr) ||
+    if (isDelegateCtor(typeStr)) {
-        (INHERITED_CLASS.exec(typeStr) && !INHERITED_CLASS_WITH_CTOR.exec(typeStr))) {
       return null;
     }
 

packages/core/test/reflection/reflector_spec.ts

@@ -7,7 +7,7 @@
  */
 
 import {Reflector} from '@angular/core/src/reflection/reflection';
-import {DELEGATE_CTOR, INHERITED_CLASS, INHERITED_CLASS_WITH_CTOR, ReflectionCapabilities} from '@angular/core/src/reflection/reflection_capabilities';
+import {ReflectionCapabilities, isDelegateCtor} from '@angular/core/src/reflection/reflection_capabilities';
 import {makeDecorator, makeParamDecorator, makePropDecorator} from '@angular/core/src/util/decorators';
 import {global} from '@angular/core/src/util/global';
 
@@ -165,8 +165,10 @@ class TestObj {
       });
     });
 
-    describe('ctor inheritance detection', () => {
+    describe('isDelegateCtor', () => {
-      it('should use the right regex', () => {
+      it('should support ES5 compiled classes', () => {
+        // These classes will be compiled to ES5 code so their stringified form
+        // below will contain ES5 constructor functions rather than native classes.
         class Parent {}
 
         class ChildNoCtor extends Parent {}
@@ -177,9 +179,9 @@ class TestObj {
           private x = 10;
         }
 
-        expect(DELEGATE_CTOR.exec(ChildNoCtor.toString())).toBeTruthy();
+        expect(isDelegateCtor(ChildNoCtor.toString())).toBe(true);
-        expect(DELEGATE_CTOR.exec(ChildNoCtorPrivateProps.toString())).toBeTruthy();
+        expect(isDelegateCtor(ChildNoCtorPrivateProps.toString())).toBe(true);
-        expect(DELEGATE_CTOR.exec(ChildWithCtor.toString())).toBeFalsy();
+        expect(isDelegateCtor(ChildWithCtor.toString())).toBe(false);
       });
 
       it('should not throw when no prototype on type', () => {
@@ -190,6 +192,9 @@ class TestObj {
       });
 
       it('should support native class', () => {
+        // These classes are defined as strings unlike the tests above because otherwise
+        // the compiler (of these tests) will convert them to ES5 constructor function
+        // style classes.
         const ChildNoCtor = `class ChildNoCtor extends Parent {}\n`;
         const ChildWithCtor = `class ChildWithCtor extends Parent {\n` +
             `  constructor() { super(); }` +
@@ -199,24 +204,23 @@ class TestObj {
             `  constructor() { super(); }` +
             `}\n`;
         const ChildNoCtorPrivateProps = `class ChildNoCtorPrivateProps extends Parent {\n` +
-            `  private x = 10;\n` +
+            `  constructor() {\n` +
+            // Note that the instance property causes a pass-through constructor to be synthesized
+            `    super(...arguments);\n` +
+            `    this.x = 10;\n` +
+            `  }\n` +
             `}\n`;
 
-        const checkNoOwnMetadata = (str: string) =>
+        expect(isDelegateCtor(ChildNoCtor)).toBe(true);
-            INHERITED_CLASS.exec(str) && !INHERITED_CLASS_WITH_CTOR.exec(str);
+        expect(isDelegateCtor(ChildNoCtorPrivateProps)).toBe(true);
-
+        expect(isDelegateCtor(ChildWithCtor)).toBe(false);
-        expect(checkNoOwnMetadata(ChildNoCtor)).toBeTruthy();
+        expect(isDelegateCtor(ChildNoCtorComplexBase)).toBe(true);
-        expect(checkNoOwnMetadata(ChildNoCtorPrivateProps)).toBeTruthy();
+        expect(isDelegateCtor(ChildWithCtorComplexBase)).toBe(false);
-        expect(checkNoOwnMetadata(ChildWithCtor)).toBeFalsy();
-        expect(checkNoOwnMetadata(ChildNoCtorComplexBase)).toBeTruthy();
-        expect(checkNoOwnMetadata(ChildWithCtorComplexBase)).toBeFalsy();
       });
 
       it('should properly handle all class forms', () => {
-        const ctor = (str: string) => expect(INHERITED_CLASS.exec(str)).toBeTruthy() &&
+        const ctor = (str: string) => expect(isDelegateCtor(str)).toBe(false);
-            expect(INHERITED_CLASS_WITH_CTOR.exec(str)).toBeTruthy();
+        const noCtor = (str: string) => expect(isDelegateCtor(str)).toBe(true);
-        const noCtor = (str: string) => expect(INHERITED_CLASS.exec(str)).toBeTruthy() &&
-            expect(INHERITED_CLASS_WITH_CTOR.exec(str)).toBeFalsy();
 
         ctor(`class Bar extends Foo {constructor(){}}`);
         ctor(`class Bar extends Foo { constructor ( ) {} }`);