From 32eafef6a715d25a939f757f9c0b149b4a4113a4 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 11 Feb 2020 16:33:20 +0100 Subject: [PATCH] fix(core): undecorated-classes-with-decorated-fields migration does not decorate derived classes (#35339) The `undecorated-classes-with-decorated-fields` migration has been introduced with 904a2018e0d3394ad91ffb6472f8271af7845295, but misses logic for decorating derived classes of undecorated classes which use Angular features. Example scenario: ```ts export abstract class MyBaseClass { @Input() someInput = true; } export abstract class BaseClassTwo extends MyBaseClass {} @Component(...) export class MyButton extends BaseClassTwo {} ``` Both abstract classes would need to be migrated. Previously, the migration only added `@Directive()` to `MyBaseClass`, but with this change, it also decorates `BaseClassTwo`. This is necessary because the Angular Compiler requires `BaseClassTwo` to have a directive definition when it flattens the directive metadata for `MyButton` in order to perform type checking. Technically, not decorating `BaseClassTwo` does not break at runtime. We basically want to enforce consistent use of `@Directive` to simplify the mental model. [See the migration guide](https://angular.io/guide/migration-undecorated-classes#migrating-classes-that-use-field-decorators). Fixes #34376. PR Close #35339 --- .../schematics/migrations/google3/BUILD.bazel | 1 + ...decoratedClassesWithDecoratedFieldsRule.ts | 57 +++++----- .../BUILD.bazel | 1 + .../google3/BUILD.bazel | 13 +++ .../google3/tslint_update_recorder.ts | 49 +++++++++ .../index.ts | 78 ++++++++------ .../transform.ts | 100 ++++++++++++++++++ .../update_recorder.ts | 19 ++++ .../utils.ts | 71 ------------- ...ated_classes_with_decorated_fields_spec.ts | 54 +++++++++- ...es_with_decorated_fields_migration_spec.ts | 53 ++++++++++ 11 files changed, 358 insertions(+), 138 deletions(-) create mode 100644 packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/BUILD.bazel create mode 100644 packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/tslint_update_recorder.ts create mode 100644 packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts create mode 100644 packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/update_recorder.ts delete mode 100644 packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/utils.ts diff --git a/packages/core/schematics/migrations/google3/BUILD.bazel b/packages/core/schematics/migrations/google3/BUILD.bazel index d078463aef..1a68dcf1d0 100644 --- a/packages/core/schematics/migrations/google3/BUILD.bazel +++ b/packages/core/schematics/migrations/google3/BUILD.bazel @@ -13,6 +13,7 @@ ts_library( "//packages/core/schematics/migrations/static-queries", "//packages/core/schematics/migrations/template-var-assignment", "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields", + "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3", "//packages/core/schematics/utils", "//packages/core/schematics/utils/tslint", "@npm//tslint", diff --git a/packages/core/schematics/migrations/google3/undecoratedClassesWithDecoratedFieldsRule.ts b/packages/core/schematics/migrations/google3/undecoratedClassesWithDecoratedFieldsRule.ts index b260f63b99..9a1150a9aa 100644 --- a/packages/core/schematics/migrations/google3/undecoratedClassesWithDecoratedFieldsRule.ts +++ b/packages/core/schematics/migrations/google3/undecoratedClassesWithDecoratedFieldsRule.ts @@ -6,12 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {Replacement, RuleFailure, Rules} from 'tslint'; +import {RuleFailure, Rules} from 'tslint'; import * as ts from 'typescript'; - -import {FALLBACK_DECORATOR, addImport, getNamedImports, getUndecoratedClassesWithDecoratedFields, hasNamedImport} from '../undecorated-classes-with-decorated-fields/utils'; - - +import {TslintUpdateRecorder} from '../undecorated-classes-with-decorated-fields/google3/tslint_update_recorder'; +import {UndecoratedClassesWithDecoratedFieldsTransform} from '../undecorated-classes-with-decorated-fields/transform'; /** * TSLint rule that adds an Angular decorator to classes that have Angular field decorators. @@ -20,37 +18,32 @@ import {FALLBACK_DECORATOR, addImport, getNamedImports, getUndecoratedClassesWit export class Rule extends Rules.TypedRule { applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[] { const typeChecker = program.getTypeChecker(); - const printer = ts.createPrinter(); - const classes = getUndecoratedClassesWithDecoratedFields(sourceFile, typeChecker); + const ruleName = this.ruleName; + const sourceFiles = program.getSourceFiles().filter( + s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s)); + const updateRecorders = new Map(); + const transform = + new UndecoratedClassesWithDecoratedFieldsTransform(typeChecker, getUpdateRecorder); - return classes.map((current, index) => { - const {classDeclaration: declaration, importDeclaration} = current; - const name = declaration.name; + // Migrate all source files in the project. + transform.migrate(sourceFiles); - // Set the class identifier node (if available) as the failing node so IDEs don't highlight - // the entire class with red. This is similar to how errors are shown for classes in other - // cases like an interface not being implemented correctly. - const start = (name || declaration).getStart(); - const end = (name || declaration).getEnd(); - const fixes = [Replacement.appendText(declaration.getStart(), `@${FALLBACK_DECORATOR}()\n`)]; + // Record the changes collected in the import manager. + transform.recordChanges(); - // If it's the first class that we're processing in this file, add `Directive` to the imports. - if (index === 0 && !hasNamedImport(importDeclaration, FALLBACK_DECORATOR)) { - const namedImports = getNamedImports(importDeclaration); + if (updateRecorders.has(sourceFile)) { + return updateRecorders.get(sourceFile) !.failures; + } + return []; - if (namedImports) { - fixes.push(new Replacement( - namedImports.getStart(), namedImports.getWidth(), - printer.printNode( - ts.EmitHint.Unspecified, addImport(namedImports, FALLBACK_DECORATOR), - sourceFile))); - } + /** Gets the update recorder for the specified source file. */ + function getUpdateRecorder(sourceFile: ts.SourceFile): TslintUpdateRecorder { + if (updateRecorders.has(sourceFile)) { + return updateRecorders.get(sourceFile) !; } - - return new RuleFailure( - sourceFile, start, end, - 'Classes with decorated fields must have an Angular decorator as well.', - 'undecorated-classes-with-decorated-fields', fixes); - }); + const recorder = new TslintUpdateRecorder(ruleName, sourceFile); + updateRecorders.set(sourceFile, recorder); + return recorder; + } } } diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/BUILD.bazel b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/BUILD.bazel index 3bfb111904..9831bbf557 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/BUILD.bazel +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/BUILD.bazel @@ -7,6 +7,7 @@ ts_library( visibility = [ "//packages/core/schematics:__pkg__", "//packages/core/schematics/migrations/google3:__pkg__", + "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3:__pkg__", "//packages/core/schematics/test:__pkg__", ], deps = [ diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/BUILD.bazel b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/BUILD.bazel new file mode 100644 index 0000000000..8950f56652 --- /dev/null +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/BUILD.bazel @@ -0,0 +1,13 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "google3", + srcs = glob(["**/*.ts"]), + tsconfig = "//packages/core/schematics:tsconfig.json", + visibility = ["//packages/core/schematics/migrations/google3:__pkg__"], + deps = [ + "//packages/core/schematics/migrations/undecorated-classes-with-decorated-fields", + "@npm//tslint", + "@npm//typescript", + ], +) diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/tslint_update_recorder.ts b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/tslint_update_recorder.ts new file mode 100644 index 0000000000..5dc825721b --- /dev/null +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/google3/tslint_update_recorder.ts @@ -0,0 +1,49 @@ +/** + * @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 {Replacement, RuleFailure} from 'tslint'; +import * as ts from 'typescript'; + +import {UpdateRecorder} from '../update_recorder'; + +export class TslintUpdateRecorder implements UpdateRecorder { + failures: RuleFailure[] = []; + + constructor(private ruleName: string, private sourceFile: ts.SourceFile) {} + + /** Adds the specified decorator to the given class declaration. */ + addClassDecorator(node: ts.ClassDeclaration, decoratorText: string) { + // Adding a decorator should be the last replacement. Replacements/rule failures + // are handled in reverse and in case a decorator and import are inserted at + // the start of the file, the class decorator should come after the import. + this.failures.unshift(new RuleFailure( + this.sourceFile, node.getStart(), 0, `Class needs to be decorated with ` + + `"${decoratorText}" because it uses Angular features.`, + this.ruleName, Replacement.appendText(node.getStart(), `${decoratorText}\n`))); + } + + /** Adds the specified import to the source file at the given position */ + addNewImport(start: number, importText: string) { + this.failures.push(new RuleFailure( + this.sourceFile, start, 0, `Source file needs to have import: "${importText}"`, + this.ruleName, Replacement.appendText(start, importText))); + } + + /** Updates existing named imports to the given new named imports. */ + updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string): void { + const fix = [ + Replacement.deleteText(namedBindings.getStart(), namedBindings.getWidth()), + Replacement.appendText(namedBindings.getStart(), newNamedBindings), + ]; + this.failures.push(new RuleFailure( + this.sourceFile, namedBindings.getStart(), namedBindings.getEnd(), + `Import needs to be updated to import symbols: "${newNamedBindings}"`, this.ruleName, fix)); + } + + commitUpdate() {} +} diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/index.ts b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/index.ts index 87dafe4f60..a911f4c79b 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/index.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/index.ts @@ -6,21 +6,21 @@ * found in the LICENSE file at https://angular.io/license */ -import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics'; +import {Rule, SchematicsException, Tree,} from '@angular-devkit/schematics'; import {dirname, relative} from 'path'; import * as ts from 'typescript'; import {getProjectTsConfigPaths} from '../../utils/project_tsconfig_paths'; import {createMigrationCompilerHost} from '../../utils/typescript/compiler_host'; import {parseTsconfigFile} from '../../utils/typescript/parse_tsconfig'; -import {FALLBACK_DECORATOR, addImport, getNamedImports, getUndecoratedClassesWithDecoratedFields, hasNamedImport} from './utils'; - +import {UpdateRecorder} from './update_recorder'; +import {UndecoratedClassesWithDecoratedFieldsTransform} from './transform'; /** * Migration that adds an Angular decorator to classes that have Angular field decorators. * https://hackmd.io/vuQfavzfRG6KUCtU7oK_EA */ export default function(): Rule { - return (tree: Tree, context: SchematicContext) => { + return (tree: Tree) => { const {buildPaths, testPaths} = getProjectTsConfigPaths(tree); const basePath = process.cwd(); const allPaths = [...buildPaths, ...testPaths]; @@ -41,39 +41,49 @@ function runUndecoratedClassesMigration(tree: Tree, tsconfigPath: string, basePa const host = createMigrationCompilerHost(tree, parsed.options, basePath); const program = ts.createProgram(parsed.fileNames, parsed.options, host); const typeChecker = program.getTypeChecker(); - const printer = ts.createPrinter(); const sourceFiles = program.getSourceFiles().filter( file => !file.isDeclarationFile && !program.isSourceFileFromExternalLibrary(file)); + const updateRecorders = new Map(); + const transform = + new UndecoratedClassesWithDecoratedFieldsTransform(typeChecker, getUpdateRecorder); - sourceFiles.forEach(sourceFile => { - const classes = getUndecoratedClassesWithDecoratedFields(sourceFile, typeChecker); + // Migrate all source files in the project. + transform.migrate(sourceFiles); - if (classes.length === 0) { - return; + // Record the changes collected in the import manager. + transform.recordChanges(); + + // Walk through each update recorder and commit the update. We need to commit the + // updates in batches per source file as there can be only one recorder per source + // file in order to avoid shifted character offsets. + updateRecorders.forEach(recorder => recorder.commitUpdate()); + + /** Gets the update recorder for the specified source file. */ + function getUpdateRecorder(sourceFile: ts.SourceFile): UpdateRecorder { + if (updateRecorders.has(sourceFile)) { + return updateRecorders.get(sourceFile) !; } - - const update = tree.beginUpdate(relative(basePath, sourceFile.fileName)); - - classes.forEach((current, index) => { - // If it's the first class that we're processing in this file, add `Directive` to the imports. - if (index === 0 && !hasNamedImport(current.importDeclaration, FALLBACK_DECORATOR)) { - const namedImports = getNamedImports(current.importDeclaration); - - if (namedImports) { - update.remove(namedImports.getStart(), namedImports.getWidth()); - update.insertRight( - namedImports.getStart(), - printer.printNode( - ts.EmitHint.Unspecified, addImport(namedImports, FALLBACK_DECORATOR), - sourceFile)); - } - } - - // We don't need to go through the AST to insert the decorator, because the change - // is pretty basic. Also this has a better chance of preserving the user's formatting. - update.insertLeft(current.classDeclaration.getStart(), `@${FALLBACK_DECORATOR}()\n`); - }); - - tree.commitUpdate(update); - }); + const treeRecorder = tree.beginUpdate(relative(basePath, sourceFile.fileName)); + const recorder: UpdateRecorder = { + addClassDecorator(node: ts.ClassDeclaration, text: string) { + // New imports should be inserted at the left while decorators should be inserted + // at the right in order to ensure that imports are inserted before the decorator + // if the start position of import and decorator is the source file start. + treeRecorder.insertRight(node.getStart(), `${text}\n`); + }, + addNewImport(start: number, importText: string) { + // New imports should be inserted at the left while decorators should be inserted + // at the right in order to ensure that imports are inserted before the decorator + // if the start position of import and decorator is the source file start. + treeRecorder.insertLeft(start, importText); + }, + updateExistingImport(namedBindings: ts.NamedImports, newNamedBindings: string) { + treeRecorder.remove(namedBindings.getStart(), namedBindings.getWidth()); + treeRecorder.insertRight(namedBindings.getStart(), newNamedBindings); + }, + commitUpdate() { tree.commitUpdate(treeRecorder); } + }; + updateRecorders.set(sourceFile, recorder); + return recorder; + } } diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts new file mode 100644 index 0000000000..990323a76c --- /dev/null +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts @@ -0,0 +1,100 @@ +/** + * @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 {ImportManager} from '../../utils/import_manager'; +import {getAngularDecorators} from '../../utils/ng_decorators'; +import {findBaseClassDeclarations} from '../../utils/typescript/find_base_classes'; + +import {UpdateRecorder} from './update_recorder'; + +export class UndecoratedClassesWithDecoratedFieldsTransform { + private printer = ts.createPrinter(); + private importManager = new ImportManager(this.getUpdateRecorder, this.printer); + + constructor( + private typeChecker: ts.TypeChecker, + private getUpdateRecorder: (sf: ts.SourceFile) => UpdateRecorder) {} + + /** + * Migrates the specified source files. The transform adds the abstract `@Directive` + * decorator to classes that have Angular field decorators but are not decorated. + * https://hackmd.io/vuQfavzfRG6KUCtU7oK_EA + */ + migrate(sourceFiles: ts.SourceFile[]) { + this._findUndecoratedDirectives(sourceFiles).forEach(node => { + const sourceFile = node.getSourceFile(); + const recorder = this.getUpdateRecorder(sourceFile); + const directiveExpr = + this.importManager.addImportToSourceFile(sourceFile, 'Directive', '@angular/core'); + const decoratorExpr = ts.createDecorator(ts.createCall(directiveExpr, undefined, undefined)); + recorder.addClassDecorator( + node, this.printer.printNode(ts.EmitHint.Unspecified, decoratorExpr, sourceFile)); + }); + } + + /** Records all changes that were made in the import manager. */ + recordChanges() { this.importManager.recordChanges(); } + + /** Finds undecorated directives in the specified source files. */ + private _findUndecoratedDirectives(sourceFiles: ts.SourceFile[]) { + const typeChecker = this.typeChecker; + const undecoratedDirectives = new Set(); + const undecoratedClasses = new Set(); + const decoratedDirectives = new WeakSet(); + + const visitNode = (node: ts.Node) => { + node.forEachChild(visitNode); + if (!ts.isClassDeclaration(node)) { + return; + } + const ngDecorators = node.decorators && getAngularDecorators(typeChecker, node.decorators); + const isDirectiveOrComponent = ngDecorators !== undefined && + ngDecorators.some(({name}) => name === 'Directive' || name === 'Component'); + if (isDirectiveOrComponent) { + decoratedDirectives.add(node); + } else { + if (this._hasAngularDecoratedClassMember(node)) { + undecoratedDirectives.add(node); + } else { + undecoratedClasses.add(node); + } + } + }; + + sourceFiles.forEach(sourceFile => sourceFile.forEachChild(visitNode)); + + // We collected all class declarations that use Angular features but are not decorated. For + // such undecorated directives, the derived classes also need to be migrated. To achieve this, + // we walk through all undecorated classes and mark those which extend from an undecorated + // directive as undecorated directive too. + undecoratedClasses.forEach(node => { + for (const {node: baseClass} of findBaseClassDeclarations(node, this.typeChecker)) { + // If the undecorated class inherits from a decorated directive, skip the current class. + // We do this because undecorated classes which inherit from directives/components are + // handled as part of the the `undecorated-classes-with-di` migration which copies + // inherited metadata. + if (decoratedDirectives.has(baseClass)) { + break; + } else if (undecoratedDirectives.has(baseClass)) { + undecoratedDirectives.add(node); + undecoratedClasses.delete(node); + break; + } + } + }); + + return undecoratedDirectives; + } + + private _hasAngularDecoratedClassMember(node: ts.ClassDeclaration): boolean { + return node.members.some( + m => m.decorators && getAngularDecorators(this.typeChecker, m.decorators).length !== 0); + } +} diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/update_recorder.ts b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/update_recorder.ts new file mode 100644 index 0000000000..7568256c35 --- /dev/null +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/update_recorder.ts @@ -0,0 +1,19 @@ +/** + * @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 {ImportManagerUpdateRecorder} from '../../utils/import_manager'; + +/** + * Update recorder interface that is used to transform source files + * in a non-colliding way. + */ +export interface UpdateRecorder extends ImportManagerUpdateRecorder { + addClassDecorator(node: ts.ClassDeclaration, text: string): void; + commitUpdate(): void; +} diff --git a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/utils.ts b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/utils.ts deleted file mode 100644 index 746f8676d6..0000000000 --- a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/utils.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** - * @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 {getAngularDecorators} from '../../utils/ng_decorators'; - -/** Name of the decorator that should be added to undecorated classes. */ -export const FALLBACK_DECORATOR = 'Directive'; - -/** Finds all of the undecorated classes that have decorated fields within a file. */ -export function getUndecoratedClassesWithDecoratedFields( - sourceFile: ts.SourceFile, typeChecker: ts.TypeChecker) { - const classes: UndecoratedClassWithDecoratedFields[] = []; - - sourceFile.forEachChild(function walk(node: ts.Node) { - if (ts.isClassDeclaration(node) && - (!node.decorators || !getAngularDecorators(typeChecker, node.decorators).length)) { - for (const member of node.members) { - const angularDecorators = - member.decorators && getAngularDecorators(typeChecker, member.decorators); - - if (angularDecorators && angularDecorators.length) { - classes.push( - {classDeclaration: node, importDeclaration: angularDecorators[0].importNode}); - return; - } - } - } - - node.forEachChild(walk); - }); - - return classes; -} - -/** Checks whether an import declaration has an import with a certain name. */ -export function hasNamedImport(declaration: ts.ImportDeclaration, symbolName: string): boolean { - const namedImports = getNamedImports(declaration); - - if (namedImports) { - return namedImports.elements.some(element => { - const {name, propertyName} = element; - return propertyName ? propertyName.text === symbolName : name.text === symbolName; - }); - } - - return false; -} - -/** Extracts the NamedImports node from an import declaration. */ -export function getNamedImports(declaration: ts.ImportDeclaration): ts.NamedImports|null { - const namedBindings = declaration.importClause && declaration.importClause.namedBindings; - return (namedBindings && ts.isNamedImports(namedBindings)) ? namedBindings : null; -} - -/** Adds a new import to a NamedImports node. */ -export function addImport(declaration: ts.NamedImports, symbolName: string) { - return ts.updateNamedImports(declaration, [ - ...declaration.elements, ts.createImportSpecifier(undefined, ts.createIdentifier(symbolName)) - ]); -} - -interface UndecoratedClassWithDecoratedFields { - classDeclaration: ts.ClassDeclaration; - importDeclaration: ts.ImportDeclaration; -} diff --git a/packages/core/schematics/test/google3/undecorated_classes_with_decorated_fields_spec.ts b/packages/core/schematics/test/google3/undecorated_classes_with_decorated_fields_spec.ts index 68718ca8e1..c78cdfcad0 100644 --- a/packages/core/schematics/test/google3/undecorated_classes_with_decorated_fields_spec.ts +++ b/packages/core/schematics/test/google3/undecorated_classes_with_decorated_fields_spec.ts @@ -64,7 +64,7 @@ describe('Google3 undecorated classes with decorated fields TSLint rule', () => expect(failures.length).toBe(1); expect(failures[0]) - .toBe('Classes with decorated fields must have an Angular decorator as well.'); + .toBe('Class needs to be decorated with "@Directive()" because it uses Angular features.'); }); it(`should add an import for Directive if there isn't one already`, () => { @@ -97,6 +97,27 @@ describe('Google3 undecorated classes with decorated fields TSLint rule', () => expect(getFile('/index.ts')).toContain(`import { Directive, Input } from '@angular/core';`); }); + it('should not generate conflicting imports there is a different `Directive` symbol', async() => { + writeFile('/index.ts', ` + import { HostBinding } from '@angular/core'; + + export class Directive { + // Simulates a scenario where a library defines a class named "Directive". + // We don't want to generate a conflicting import. + } + + export class MyLibrarySharedBaseClass { + @HostBinding('class.active') isActive: boolean; + } + `); + + runTSLint(true); + const fileContent = getFile('/index.ts'); + expect(fileContent) + .toContain(`import { HostBinding, Directive as Directive_1 } from '@angular/core';`); + expect(fileContent).toMatch(/@Directive_1\(\)\s+export class MyLibrarySharedBaseClass/); + }); + it('should add @Directive to undecorated classes that have @Input', () => { writeFile('/index.ts', ` import { Input } from '@angular/core'; @@ -229,4 +250,35 @@ describe('Google3 undecorated classes with decorated fields TSLint rule', () => expect(getFile('/index.ts')).toContain(`@Directive()\nexport class Base {`); }); + it('should add @Directive to undecorated derived classes of a migrated class', async() => { + writeFile('/index.ts', ` + import { Input, Directive, NgModule } from '@angular/core'; + + export class Base { + @Input() isActive: boolean; + } + + export class DerivedA extends Base {} + export class DerivedB extends DerivedA {} + export class DerivedC extends DerivedB {} + + @Directive({selector: 'my-comp'}) + export class MyComp extends DerivedC {} + + export class MyCompWrapped extends MyComp {} + + @NgModule({declarations: [MyComp, MyCompWrapped]}) + export class AppModule {} + `); + + runTSLint(true); + const fileContent = getFile('/index.ts'); + expect(fileContent).toContain(`import { Input, Directive, NgModule } from '@angular/core';`); + expect(fileContent).toMatch(/@Directive\(\)\s+export class Base/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedA/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedB/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedC/); + expect(fileContent).toMatch(/}\s+@Directive\(\{selector: 'my-comp'}\)\s+export class MyComp/); + expect(fileContent).toMatch(/}\s+export class MyCompWrapped/); + }); }); diff --git a/packages/core/schematics/test/undecorated_classes_with_decorated_fields_migration_spec.ts b/packages/core/schematics/test/undecorated_classes_with_decorated_fields_migration_spec.ts index ed04c675d1..c141898803 100644 --- a/packages/core/schematics/test/undecorated_classes_with_decorated_fields_migration_spec.ts +++ b/packages/core/schematics/test/undecorated_classes_with_decorated_fields_migration_spec.ts @@ -74,6 +74,27 @@ describe('Undecorated classes with decorated fields migration', () => { .toContain(`import { Directive, Input } from '@angular/core';`); }); + it('should not generate conflicting imports there is a different `Directive` symbol', async() => { + writeFile('/index.ts', ` + import { HostBinding } from '@angular/core'; + + export class Directive { + // Simulates a scenario where a library defines a class named "Directive". + // We don't want to generate a conflicting import. + } + + export class MyLibrarySharedBaseClass { + @HostBinding('class.active') isActive: boolean; + } + `); + + await runMigration(); + const fileContent = tree.readContent('/index.ts'); + expect(fileContent) + .toContain(`import { HostBinding, Directive as Directive_1 } from '@angular/core';`); + expect(fileContent).toMatch(/@Directive_1\(\)\s+export class MyLibrarySharedBaseClass/); + }); + it('should add @Directive to undecorated classes that have @Input', async() => { writeFile('/index.ts', ` import { Input } from '@angular/core'; @@ -206,6 +227,38 @@ describe('Undecorated classes with decorated fields migration', () => { expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`); }); + it('should add @Directive to undecorated derived classes of a migrated class', async() => { + writeFile('/index.ts', ` + import { Input, Directive, NgModule } from '@angular/core'; + + export class Base { + @Input() isActive: boolean; + } + + export class DerivedA extends Base {} + export class DerivedB extends DerivedA {} + export class DerivedC extends DerivedB {} + + @Directive({selector: 'my-comp'}) + export class MyComp extends DerivedC {} + + export class MyCompWrapped extends MyComp {} + + @NgModule({declarations: [MyComp, MyCompWrapped]}) + export class AppModule {} + `); + + await runMigration(); + const fileContent = tree.readContent('/index.ts'); + expect(fileContent).toContain(`import { Input, Directive, NgModule } from '@angular/core';`); + expect(fileContent).toMatch(/@Directive\(\)\s+export class Base/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedA/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedB/); + expect(fileContent).toMatch(/@Directive\(\)\s+export class DerivedC/); + expect(fileContent).toMatch(/}\s+@Directive\(\{selector: 'my-comp'}\)\s+export class MyComp/); + expect(fileContent).toMatch(/}\s+export class MyCompWrapped/); + }); + function writeFile(filePath: string, contents: string) { host.sync.write(normalize(filePath), virtualFs.stringToFileBuffer(contents)); }