feat(ivy): ngcc - implement `UndecoratedParentMigration` (#31544)

Implementing the "undecorated parent" migration described in
https://hackmd.io/sfb3Ju2MTmKHSUiX_dLWGg#Design

PR Close #31544

packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts

@@ -0,0 +1,95 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import * as ts from 'typescript';
+import {ErrorCode, makeDiagnostic} from '../../../src/ngtsc/diagnostics';
+import {ClassDeclaration} from '../../../src/ngtsc/reflection';
+import {isRelativePath} from '../utils';
+import {Migration, MigrationHost} from './migration';
+import {createDirectiveDecorator, hasConstructor, hasDirectiveDecorator, isClassDeclaration} from './utils';
+
+/**
+ * Ensure that the parents of directives and components that have no constructor are also decorated
+ * as a `Directive`.
+ *
+ * Example:
+ *
+ * ```
+ * export class BasePlain {
+ *   constructor(private vcr: ViewContainerRef) {}
+ * }
+ *
+ * @Directive({selector: '[blah]'})
+ * export class DerivedDir extends BasePlain {}
+ * ```
+ *
+ * When compiling `DerivedDir` which extends the undecorated `BasePlain` class, the compiler needs
+ * to generate an `ngDirectiveDef` for `DerivedDir`. In particular, it needs to generate a factory
+ * function that creates instances of `DerivedDir`.
+ *
+ * As `DerivedDir` has no constructor, the factory function for `DerivedDir` must delegate to the
+ * factory function for `BasePlain`. But for this to work, `BasePlain` must have a factory function,
+ * itself.
+ *
+ * This migration adds a `Directive` decorator to such undecorated parent classes, to ensure that
+ * the compiler will create the necessary factory function.
+ *
+ * The resulting code looks like:
+ *
+ * ```
+ * @Directive()
+ * export class BasePlain {
+ *   constructor(private vcr: ViewContainerRef) {}
+ * }
+ *
+ * @Directive({selector: '[blah]'})
+ * export class DerivedDir extends BasePlain {}
+ * ```
+ */
+export class UndecoratedParentMigration implements Migration {
+  apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null {
+    // Only interested in `clazz` if it is a `Component` or a `Directive`,
+    // and it has no constructor of its own.
+    if (!hasDirectiveDecorator(host, clazz) || hasConstructor(host, clazz)) {
+      return null;
+    }
+
+    // Only interested in `clazz` if it inherits from a base class.
+    const baseClassExpr = host.reflectionHost.getBaseClassExpression(clazz);
+    if (baseClassExpr === null) {
+      return null;
+    }
+
+    if (!ts.isIdentifier(baseClassExpr)) {
+      return makeDiagnostic(
+          ErrorCode.NGCC_MIGRATION_EXTERNAL_BASE_CLASS, baseClassExpr,
+          `${clazz.name.text} class has a dynamic base class ${baseClassExpr.getText()}, so it is not possible to migrate.`);
+      return null;
+    }
+
+    const baseClazz = host.reflectionHost.getDeclarationOfIdentifier(baseClassExpr) !.node;
+    if (!isClassDeclaration(baseClazz)) {
+      return null;
+    }
+
+    // Only interested in this base class if it doesn't have a `Directive` or `Component` decorator.
+    if (hasDirectiveDecorator(host, baseClazz)) {
+      return null;
+    }
+
+    const importInfo = host.reflectionHost.getImportOfIdentifier(baseClassExpr);
+    if (importInfo !== null && !isRelativePath(importInfo.from)) {
+      return makeDiagnostic(
+          ErrorCode.NGCC_MIGRATION_EXTERNAL_BASE_CLASS, baseClassExpr,
+          'The base class was imported from an external entry-point so we cannot add a directive to it.');
+    }
+
+    host.injectSyntheticDecorator(baseClazz, createDirectiveDecorator(baseClazz));
+
+    return null;
+  }
+}

packages/compiler-cli/ngcc/src/migrations/utils.ts

@@ -0,0 +1,63 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import * as ts from 'typescript';
+import {Reference} from '../../../src/ngtsc/imports';
+import {ClassDeclaration, Decorator, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
+import {MigrationHost} from './migration';
+
+export function isClassDeclaration(clazz: ts.Declaration): clazz is ClassDeclaration {
+  return isNamedClassDeclaration(clazz) || isNamedFunctionDeclaration(clazz) ||
+      isNamedVariableDeclaration(clazz);
+}
+
+/**
+ * Returns true if the `clazz` is decorated as a `Directive` or `Component`.
+ */
+export function hasDirectiveDecorator(host: MigrationHost, clazz: ClassDeclaration): boolean {
+  return host.metadata.getDirectiveMetadata(new Reference(clazz)) !== null;
+}
+
+/**
+ * Returns true if the `clazz` has its own constructor function.
+ */
+export function hasConstructor(host: MigrationHost, clazz: ClassDeclaration): boolean {
+  return host.reflectionHost.getConstructorParameters(clazz) !== null;
+}
+
+/**
+ * Create an empty `Directive` decorator that will be associated with the `clazz`.
+ */
+export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator {
+  const selectorArg = ts.createObjectLiteral([
+    // TODO: At the moment ngtsc does not accept a directive with no selector
+    ts.createPropertyAssignment('selector', ts.createStringLiteral('NGCC_DUMMY')),
+  ]);
+  const decoratorType = ts.createIdentifier('Directive');
+  const decoratorNode = ts.createObjectLiteral([
+    ts.createPropertyAssignment('type', decoratorType),
+    ts.createPropertyAssignment('args', ts.createArrayLiteral([selectorArg])),
+  ]);
+
+  setParentPointers(clazz.getSourceFile(), decoratorNode);
+
+  return {
+    name: 'Directive',
+    identifier: decoratorType,
+    import: {name: 'Directive', from: '@angular/core'},
+    node: decoratorNode,
+    args: [selectorArg],
+  };
+}
+
+/**
+ * Ensure that a tree of AST nodes have their parents wired up.
+ */
+export function setParentPointers(parent: ts.Node, child: ts.Node): void {
+  child.parent = parent;
+  ts.forEachChild(child, grandchild => setParentPointers(child, grandchild));
+}

packages/compiler-cli/ngcc/test/migrations/undecorated_parent_migration_spec.ts

@@ -0,0 +1,175 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import * as ts from 'typescript';
+import {AbsoluteFsPath, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system';
+import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
+import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
+import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
+import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
+import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
+import {UndecoratedParentMigration} from '../../src/migrations/undecorated_parent_migration';
+import {MockLogger} from '../helpers/mock_logger';
+import {getRootFiles, makeTestEntryPointBundle} from '../helpers/utils';
+
+runInEachFileSystem(() => {
+  describe('UndecoratedParentMigration', () => {
+    let _: typeof absoluteFrom;
+    let INDEX_FILENAME: AbsoluteFsPath;
+    beforeEach(() => {
+      _ = absoluteFrom;
+      INDEX_FILENAME = _('/node_modules//test-package/index.js');
+    });
+
+    it('should ignore undecorated classes', () => {
+      const {program, analysis} = setUpAndAnalyzeProgram([{
+        name: INDEX_FILENAME,
+        contents: `
+        export class DerivedClass extends BaseClass {}
+        export class BaseClass {}
+      `
+      }]);
+      const file = analysis.get(program.getSourceFile(INDEX_FILENAME) !);
+      expect(file).toBeUndefined();
+    });
+
+    it('should ignore an undecorated base class if the derived class has a constructor', () => {
+      const {program, analysis} = setUpAndAnalyzeProgram([{
+        name: INDEX_FILENAME,
+        contents: `
+        import {Directive, ViewContainerRef} from '@angular/core';
+        export class DerivedClass extends BaseClass {
+          constructor(private vcr: ViewContainerRef) {}
+        }
+        DerivedClass.decorators = [
+          { type: Directive, args: [{ selector: '[dir]' }] }
+        ];
+        DerivedClass.ctorParameters = () => [
+          { type: ViewContainerRef, }
+        ];
+        export class BaseClass {}
+      `
+      }]);
+      const file = analysis.get(program.getSourceFile(INDEX_FILENAME) !) !;
+      expect(file.compiledClasses.find(c => c.name === 'DerivedClass')).toBeDefined();
+      expect(file.compiledClasses.find(c => c.name === 'BaseClass')).toBeUndefined();
+    });
+
+    it('should add a decorator to an undecorated base class if the derived class is a Directive with no constructor',
+       () => {
+         const {program, analysis} = setUpAndAnalyzeProgram([{
+           name: INDEX_FILENAME,
+           contents: `
+        import {Directive, ViewContainerRef} from '@angular/core';
+        export class DerivedClass extends BaseClass {
+        }
+        DerivedClass.decorators = [
+          { type: Directive, args: [{ selector: '[dir]' }] }
+        ];
+        export class BaseClass {
+          constructor(private vcr: ViewContainerRef) {}
+        }
+        BaseClass.ctorParameters = () => [
+          { type: ViewContainerRef, }
+        ];
+      `
+         }]);
+         const file = analysis.get(program.getSourceFile(INDEX_FILENAME) !) !;
+         expect(file.compiledClasses.find(c => c.name === 'DerivedClass')).toBeDefined();
+         const baseClass = file.compiledClasses.find(c => c.name === 'BaseClass') !;
+         expect(baseClass.decorators !.length).toEqual(1);
+         const decorator = baseClass.decorators ![0];
+         expect(decorator.name).toEqual('Directive');
+         expect(decorator.import).toEqual({from: '@angular/core', name: 'Directive'});
+         expect(decorator.args !.length).toEqual(1);
+       });
+
+    it('should handle the base class being in a different file (same package) as the derived class',
+       () => {
+         const BASE_FILENAME = _('/node_modules/test-package/base.js');
+         const {program, analysis} = setUpAndAnalyzeProgram([
+           {
+             name: INDEX_FILENAME,
+             contents: `
+       import {Directive, ViewContainerRef} from '@angular/core';
+       import {BaseClass} from './base';
+       export class DerivedClass extends BaseClass {
+       }
+       DerivedClass.decorators = [
+         { type: Directive, args: [{ selector: '[dir]' }] }
+       ];
+     `
+           },
+           {
+             name: BASE_FILENAME,
+             contents: `
+          export class BaseClass {
+            constructor(private vcr: ViewContainerRef) {}
+          }
+          BaseClass.ctorParameters = () => [
+            { type: ViewContainerRef, }
+          ];
+      `
+           }
+         ]);
+         const file = analysis.get(program.getSourceFile(BASE_FILENAME) !) !;
+         const baseClass = file.compiledClasses.find(c => c.name === 'BaseClass') !;
+         expect(baseClass.decorators !.length).toEqual(1);
+         const decorator = baseClass.decorators ![0];
+         expect(decorator.name).toEqual('Directive');
+         expect(decorator.import).toEqual({from: '@angular/core', name: 'Directive'});
+         expect(decorator.args !.length).toEqual(1);
+       });
+
+    it('should error if the base class being is a different package from the derived class', () => {
+      const BASE_FILENAME = _('/node_modules/other-package/index.js');
+      const {errors} = setUpAndAnalyzeProgram([
+        {
+          name: INDEX_FILENAME,
+          contents: `
+       import {Directive, ViewContainerRef} from '@angular/core';
+       import {BaseClass} from 'other-package';
+       export class DerivedClass extends BaseClass {
+       }
+       DerivedClass.decorators = [
+         { type: Directive, args: [{ selector: '[dir]' }] }
+       ];
+     `
+        },
+        {
+          name: BASE_FILENAME,
+          contents: `
+          export class BaseClass {
+            constructor(private vcr: ViewContainerRef) {}
+          }
+          BaseClass.ctorParameters = () => [
+            { type: ViewContainerRef, }
+          ];
+      `
+        }
+      ]);
+      expect(errors.length).toEqual(1);
+    });
+  });
+
+  function setUpAndAnalyzeProgram(testFiles: TestFile[]) {
+    loadTestFiles(testFiles);
+    loadFakeCore(getFileSystem());
+    const errors: ts.Diagnostic[] = [];
+    const rootFiles = getRootFiles(testFiles);
+    const bundle = makeTestEntryPointBundle('test-package', 'es2015', 'esm2015', false, rootFiles);
+    const program = bundle.src.program;
+
+    const reflectionHost =
+        new Esm2015ReflectionHost(new MockLogger(), false, program.getTypeChecker());
+    const referencesRegistry = new NgccReferencesRegistry(reflectionHost);
+    const analyzer = new DecorationAnalyzer(
+        getFileSystem(), bundle, reflectionHost, referencesRegistry, error => errors.push(error));
+    analyzer.migrations = [new UndecoratedParentMigration()];
+    return {program, analysis: analyzer.analyzeProgram(), errors};
+  }
+});

packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts

@@ -61,6 +61,17 @@ export enum ErrorCode {
    * Raised when ngcc tries to inject a synthetic decorator over one that already exists.
    */
   NGCC_MIGRATION_DECORATOR_INJECTION_ERROR = 7001,
+
+  /**
+   * Raised when ngcc tries to decorate a base class that was imported from outside the package.
+   */
+  NGCC_MIGRATION_EXTERNAL_BASE_CLASS = 7002,
+
+  /**
+   * Raised when ngcc tries to migrate a class that is extended from a dynamic base class
+   * expression.
+   */
+  NGCC_MIGRATION_DYNAMIC_BASE_CLASS = 7003,
 }
 
 export function ngErrorCode(code: ErrorCode): number {