diff --git a/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts b/packages/compiler-cli/ngcc/src/migrations/undecorated_parent_migration.ts new file mode 100644 index 0000000000..a49ac6f042 --- /dev/null +++ b/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; + } +} diff --git a/packages/compiler-cli/ngcc/src/migrations/utils.ts b/packages/compiler-cli/ngcc/src/migrations/utils.ts new file mode 100644 index 0000000000..4113d43876 --- /dev/null +++ b/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)); +} diff --git a/packages/compiler-cli/ngcc/test/migrations/undecorated_parent_migration_spec.ts b/packages/compiler-cli/ngcc/test/migrations/undecorated_parent_migration_spec.ts new file mode 100644 index 0000000000..4702bfa67f --- /dev/null +++ b/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}; + } +}); diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts index 9446a94ee3..dad1acb5ad 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/code.ts +++ b/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 {