diff --git a/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields.ts b/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields.ts index dd81dd2187..ade71e3dfa 100644 --- a/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields.ts +++ b/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields.ts @@ -4,8 +4,10 @@ import { ElementRef, HostBinding, HostListener, + Injectable, Input, - NgModule + NgModule, + Pipe } from '@angular/core'; export class NonAngularBaseClass { @@ -76,3 +78,17 @@ export class UndecoratedPipeBase { export class WithDirectiveLifecycleHook { ngOnInit() {} } + +// This class is already decorated and should not be migrated. i.e. no TODO +// or Angular decorator should be added. `@Injectable` is sufficient. +@Injectable() +export class MyService { + ngOnDestroy() {} +} + +// This class is already decorated and should not be migrated. i.e. no TODO +// or Angular decorator should be added. `@Injectable` is sufficient. +@Pipe({name: 'my-pipe'}) +export class MyPipe { + ngOnDestroy() {} +} diff --git a/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields_expected.ts b/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields_expected.ts index 664188b826..cb6b1e3974 100644 --- a/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields_expected.ts +++ b/integration/ng_update_migrations/src/app/migration-tests/undecorated-classes-with-fields_expected.ts @@ -4,8 +4,10 @@ import { ElementRef, HostBinding, HostListener, + Injectable, Input, - NgModule + NgModule, + Pipe } from '@angular/core'; export class NonAngularBaseClass { @@ -87,3 +89,17 @@ export class UndecoratedPipeBase { export class WithDirectiveLifecycleHook { ngOnInit() {} } + +// This class is already decorated and should not be migrated. i.e. no TODO +// or Angular decorator should be added. `@Injectable` is sufficient. +@Injectable() +export class MyService { + ngOnDestroy() {} +} + +// This class is already decorated and should not be migrated. i.e. no TODO +// or Angular decorator should be added. `@Injectable` is sufficient. +@Pipe({name: 'my-pipe'}) +export class MyPipe { + ngOnDestroy() {} +} 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 index c1f9bcb81e..ddfe450b6b 100644 --- a/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts +++ b/packages/core/schematics/migrations/undecorated-classes-with-decorated-fields/transform.ts @@ -43,20 +43,27 @@ const DIRECTIVE_LIFECYCLE_HOOKS = new Set([ const AMBIGUOUS_LIFECYCLE_HOOKS = new Set(['ngOnDestroy']); /** Describes how a given class is used in the context of Angular. */ -enum ClassKind { +enum InferredKind { DIRECTIVE, AMBIGUOUS, UNKNOWN, } +/** Describes possible types of Angular declarations. */ +enum DeclarationType { + DIRECTIVE, + COMPONENT, + ABSTRACT_DIRECTIVE, + PIPE, + INJECTABLE, +} + /** Analyzed class declaration. */ interface AnalyzedClass { - /** Whether the class is decorated with @Directive or @Component. */ - isDirectiveOrComponent: boolean; - /** Whether the class is an abstract directive. */ - isAbstractDirective: boolean; - /** Kind of the given class in terms of Angular. */ - kind: ClassKind; + /** Type of declaration that is determined through an applied decorator. */ + decoratedType: DeclarationType|null; + /** Inferred class kind in terms of Angular. */ + inferredKind: InferredKind; } interface AnalysisFailure { @@ -64,6 +71,9 @@ interface AnalysisFailure { message: string; } +/** TODO message that is added to ambiguous classes using Angular features. */ +const AMBIGUOUS_CLASS_TODO = 'Add Angular decorator.'; + export class UndecoratedClassesWithDecoratedFieldsTransform { private printer = ts.createPrinter(); private importManager = new ImportManager(this.getUpdateRecorder, this.printer); @@ -81,10 +91,10 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { * indicating that a given class uses Angular features. https://hackmd.io/vuQfavzfRG6KUCtU7oK_EA */ migrate(sourceFiles: ts.SourceFile[]): AnalysisFailure[] { - const {result, ambiguous} = this._findUndecoratedAbstractDirectives(sourceFiles); + const {detectedAbstractDirectives, ambiguousClasses} = + this._findUndecoratedAbstractDirectives(sourceFiles); - - result.forEach(node => { + detectedAbstractDirectives.forEach(node => { const sourceFile = node.getSourceFile(); const recorder = this.getUpdateRecorder(sourceFile); const directiveExpr = @@ -98,12 +108,19 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { // determine whether the class is used as directive, service or pipe. The migration // could potentially determine the type by checking NgModule definitions or inheritance // of other known declarations, but this is out of scope and a TODO/failure is sufficient. - return Array.from(ambiguous).reduce((failures, node) => { + return Array.from(ambiguousClasses).reduce((failures, node) => { + // If the class has been reported as ambiguous before, skip adding a TODO and + // printing an error. A class could be visited multiple times when it's part + // of multiple build targets in the CLI project. + if (this._hasBeenReportedAsAmbiguous(node)) { + return failures; + } + const sourceFile = node.getSourceFile(); const recorder = this.getUpdateRecorder(sourceFile); // Add a TODO to the class that uses Angular features but is not decorated. - recorder.addClassTodo(node, `Add Angular decorator.`); + recorder.addClassTodo(node, AMBIGUOUS_CLASS_TODO); // Add an error for the class that will be printed in the `ng update` output. return failures.concat({ @@ -125,59 +142,83 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { * directives. Those are ambiguous and could be either Directive, Pipe or service. */ private _findUndecoratedAbstractDirectives(sourceFiles: ts.SourceFile[]) { - const result = new Set(); + const ambiguousClasses = new Set(); + const declarations = new WeakMap(); + const detectedAbstractDirectives = new Set(); const undecoratedClasses = new Set(); - const nonAbstractDirectives = new WeakSet(); - const abstractDirectives = new WeakSet(); - const ambiguous = new Set(); const visitNode = (node: ts.Node) => { node.forEachChild(visitNode); if (!ts.isClassDeclaration(node)) { return; } - const {isDirectiveOrComponent, isAbstractDirective, kind} = - this._analyzeClassDeclaration(node); - if (isDirectiveOrComponent) { - if (isAbstractDirective) { - abstractDirectives.add(node); - } else { - nonAbstractDirectives.add(node); - } - } else if (kind === ClassKind.DIRECTIVE) { - abstractDirectives.add(node); - result.add(node); + const {inferredKind, decoratedType} = this._analyzeClassDeclaration(node); + + if (decoratedType !== null) { + declarations.set(node, decoratedType); + return; + } + + if (inferredKind === InferredKind.DIRECTIVE) { + detectedAbstractDirectives.add(node); + } else if (inferredKind === InferredKind.AMBIGUOUS) { + ambiguousClasses.add(node); } else { - if (kind === ClassKind.AMBIGUOUS) { - ambiguous.add(node); - } undecoratedClasses.add(node); } }; sourceFiles.forEach(sourceFile => sourceFile.forEachChild(visitNode)); - // We collected all undecorated class declarations which inherit from abstract directives. - // For such abstract directives, the derived classes also need to be migrated. - undecoratedClasses.forEach(node => { - for (const {node: baseClass} of findBaseClassDeclarations(node, this.typeChecker)) { - // If the undecorated class inherits from a non-abstract directive, skip the current - // class. We do this because undecorated classes which inherit metadata from non-abstract - // directives are handled in the `undecorated-classes-with-di` migration that copies - // inherited metadata into an explicit decorator. - if (nonAbstractDirectives.has(baseClass)) { - break; - } else if (abstractDirectives.has(baseClass)) { - result.add(node); - // In case the undecorated class previously could not be detected as directive, - // remove it from the ambiguous set as we now know that it's a guaranteed directive. - ambiguous.delete(node); + /** + * Checks the inheritance of the given set of classes. It removes classes from the + * detected abstract directives set when they inherit from a non-abstract Angular + * declaration. e.g. an abstract directive can never extend from a component. + * + * If a class inherits from an abstract directive though, we will migrate them too + * as derived classes also need to be decorated. This has been done for a simpler mental + * model and reduced complexity in the Angular compiler. See migration plan document. + */ + const checkInheritanceOfClasses = (classes: Set) => { + classes.forEach(node => { + for (const {node: baseClass} of findBaseClassDeclarations(node, this.typeChecker)) { + if (!declarations.has(baseClass)) { + continue; + } + // If the undecorated class inherits from an abstract directive, always migrate it. + // Derived undecorated classes of abstract directives are always also considered + // abstract directives and need to be decorated too. This is necessary as otherwise + // the inheritance chain cannot be resolved by the Angular compiler. e.g. when it + // flattens directive metadata for type checking. In the other case, we never want + // to migrate a class if it extends from a non-abstract Angular declaration. That + // is an unsupported pattern as of v9 and was previously handled with the + // `undecorated-classes-with-di` migration (which copied the inherited decorator). + if (declarations.get(baseClass) === DeclarationType.ABSTRACT_DIRECTIVE) { + detectedAbstractDirectives.add(node); + } else { + detectedAbstractDirectives.delete(node); + } + ambiguousClasses.delete(node); break; } - } - }); + }); + }; - return {result, ambiguous}; + // Check inheritance of any detected abstract directive. We want to remove + // classes that are not eligible abstract directives due to inheritance. i.e. + // if a class extends from a component, it cannot be a derived abstract directive. + checkInheritanceOfClasses(detectedAbstractDirectives); + // Update the class declarations to reflect the detected abstract directives. This is + // then used later when we check for undecorated classes that inherit from an abstract + // directive and need to be decorated. + detectedAbstractDirectives.forEach( + n => declarations.set(n, DeclarationType.ABSTRACT_DIRECTIVE)); + // Check ambiguous and undecorated classes if they inherit from an abstract directive. + // If they do, we want to migrate them too. See function definition for more details. + checkInheritanceOfClasses(ambiguousClasses); + checkInheritanceOfClasses(undecoratedClasses); + + return {detectedAbstractDirectives, ambiguousClasses}; } /** @@ -186,19 +227,30 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { */ private _analyzeClassDeclaration(node: ts.ClassDeclaration): AnalyzedClass { const ngDecorators = node.decorators && getAngularDecorators(this.typeChecker, node.decorators); - const kind = this._determineClassKind(node); + const inferredKind = this._determineClassKind(node); if (ngDecorators === undefined || ngDecorators.length === 0) { - return {isDirectiveOrComponent: false, isAbstractDirective: false, kind}; + return {decoratedType: null, inferredKind}; } const directiveDecorator = ngDecorators.find(({name}) => name === 'Directive'); const componentDecorator = ngDecorators.find(({name}) => name === 'Component'); + const pipeDecorator = ngDecorators.find(({name}) => name === 'Pipe'); + const injectableDecorator = ngDecorators.find(({name}) => name === 'Injectable'); const isAbstractDirective = directiveDecorator !== undefined && this._isAbstractDirective(directiveDecorator); - return { - isDirectiveOrComponent: !!directiveDecorator || !!componentDecorator, - isAbstractDirective, - kind, - }; + + let decoratedType: DeclarationType|null = null; + if (isAbstractDirective) { + decoratedType = DeclarationType.ABSTRACT_DIRECTIVE; + } else if (componentDecorator !== undefined) { + decoratedType = DeclarationType.COMPONENT; + } else if (directiveDecorator !== undefined) { + decoratedType = DeclarationType.DIRECTIVE; + } else if (pipeDecorator !== undefined) { + decoratedType = DeclarationType.PIPE; + } else if (injectableDecorator !== undefined) { + decoratedType = DeclarationType.INJECTABLE; + } + return {decoratedType, inferredKind}; } /** @@ -228,8 +280,8 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { * e.g. lifecycle hooks or decorated members like `@Input` or `@Output` are * considered Angular features.. */ - private _determineClassKind(node: ts.ClassDeclaration): ClassKind { - let usage = ClassKind.UNKNOWN; + private _determineClassKind(node: ts.ClassDeclaration): InferredKind { + let usage = InferredKind.UNKNOWN; for (const member of node.members) { const propertyName = member.name !== undefined ? getPropertyNameText(member.name) : null; @@ -237,7 +289,7 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { // If the class declares any of the known directive lifecycle hooks, we can // immediately exit the loop as the class is guaranteed to be a directive. if (propertyName !== null && DIRECTIVE_LIFECYCLE_HOOKS.has(propertyName)) { - return ClassKind.DIRECTIVE; + return InferredKind.DIRECTIVE; } const ngDecorators = member.decorators !== undefined ? @@ -245,7 +297,7 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { []; for (const {name} of ngDecorators) { if (DIRECTIVE_FIELD_DECORATORS.has(name)) { - return ClassKind.DIRECTIVE; + return InferredKind.DIRECTIVE; } } @@ -253,10 +305,27 @@ export class UndecoratedClassesWithDecoratedFieldsTransform { // the given class is a directive, update the kind and continue looking for other // members that would unveil a more specific kind (i.e. being a directive). if (propertyName !== null && AMBIGUOUS_LIFECYCLE_HOOKS.has(propertyName)) { - usage = ClassKind.AMBIGUOUS; + usage = InferredKind.AMBIGUOUS; } } return usage; } + + /** + * Checks whether a given class has been reported as ambiguous in previous + * migration run. e.g. when build targets are migrated first, and then test + * targets that have an overlap with build source files, the same class + * could be detected as ambiguous. + */ + private _hasBeenReportedAsAmbiguous(node: ts.ClassDeclaration): boolean { + const sourceFile = node.getSourceFile(); + const leadingComments = ts.getLeadingCommentRanges(sourceFile.text, node.pos); + if (leadingComments === undefined) { + return false; + } + return leadingComments.some( + ({kind, pos, end}) => kind === ts.SyntaxKind.SingleLineCommentTrivia && + sourceFile.text.substring(pos, end).includes(`TODO: ${AMBIGUOUS_CLASS_TODO}`)); + } } 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 1a7c00dfce..04afcf0f30 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 @@ -136,24 +136,36 @@ describe('Google3 undecorated classes with decorated fields TSLint rule', () => it('should not change decorated classes', () => { writeFile('/index.ts', ` - import { Input, Component, Output, EventEmitter } from '@angular/core'; + import { Input, Component, Directive, Pipe, Injectable } from '@angular/core'; @Component({}) - export class Base { + export class MyComp { + @Input() isActive: boolean; + } + + @Directive({selector: 'dir'}) + export class MyDir { @Input() isActive: boolean; } - export class Child extends Base { - @Output() clicked = new EventEmitter(); + @Injectable() + export class MyService { + ngOnDestroy() {} + } + + @Pipe({name: 'my-pipe'}) + export class MyPipe { + ngOnDestroy() {} } `); runTSLint(true); const content = getFile('/index.ts'); - expect(content).toContain( - `import { Input, Component, Output, EventEmitter, Directive } from '@angular/core';`); - expect(content).toContain(`@Component({})\n export class Base {`); - expect(content).toContain(`@Directive()\nexport class Child extends Base {`); + expect(content).toMatch(/@Component\({}\)\s+export class MyComp {/); + expect(content).toMatch(/@Directive\({selector: 'dir'}\)\s+export class MyDir {/); + expect(content).toMatch(/@Injectable\(\)\s+export class MyService {/); + expect(content).toMatch(/@Pipe\({name: 'my-pipe'}\)\s+export class MyPipe {/); + expect(content).not.toContain('TODO'); }); it('should add @Directive to undecorated classes that have @Output', () => { diff --git a/packages/core/schematics/test/helpers.ts b/packages/core/schematics/test/helpers.ts index 4194a8318f..f2bcfe2d6f 100644 --- a/packages/core/schematics/test/helpers.ts +++ b/packages/core/schematics/test/helpers.ts @@ -9,6 +9,7 @@ /** * Template string function that can be used to dedent the resulting * string literal. The smallest common indentation will be omitted. + * Additionally, whitespace in empty lines is removed. */ export function dedent(strings: TemplateStringsArray, ...values: any[]) { let joinedString = ''; @@ -24,5 +25,7 @@ export function dedent(strings: TemplateStringsArray, ...values: any[]) { const minLineIndent = Math.min(...matches.map(el => el.length)); const omitMinIndentRegex = new RegExp(`^[ \\t]{${minLineIndent}}`, 'gm'); - return minLineIndent > 0 ? joinedString.replace(omitMinIndentRegex, '') : joinedString; + const omitEmptyLineWhitespaceRegex = /^[ \t]+$/gm; + const result = minLineIndent > 0 ? joinedString.replace(omitMinIndentRegex, '') : joinedString; + return result.replace(omitEmptyLineWhitespaceRegex, ''); } 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 e8a2d98525..9887d4bb1a 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 @@ -11,6 +11,7 @@ import {TempScopedNodeJsSyncHost} from '@angular-devkit/core/node/testing'; import {HostTree} from '@angular-devkit/schematics'; import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing'; import * as shx from 'shelljs'; +import {dedent} from './helpers'; describe('Undecorated classes with decorated fields migration', () => { let runner: SchematicTestRunner; @@ -117,26 +118,253 @@ describe('Undecorated classes with decorated fields migration', () => { expect(tree.readContent('/index.ts')).toContain(`@Directive()\nexport class Base {`); }); - it('should not change decorated classes', async () => { - writeFile('/index.ts', ` - import { Input, Component, Output, EventEmitter } from '@angular/core'; + it('should not migrate classes decorated with @Component', async () => { + writeFile('/index.ts', dedent` + import {Input, Component} from '@angular/core'; - @Component({}) + @Component({selector: 'hello', template: 'hello'}) export class Base { @Input() isActive: boolean; } - - export class Child extends Base { - @Output() clicked = new EventEmitter(); + + @Component({selector: 'hello', template: 'hello'}) + export class Derived extends Base { + ngOnDestroy() {} } `); await runMigration(); - const content = tree.readContent('/index.ts'); - expect(content).toContain( - `import { Input, Component, Output, EventEmitter, Directive } from '@angular/core';`); - expect(content).toContain(`@Component({})\n export class Base {`); - expect(content).toContain(`@Directive()\nexport class Child extends Base {`); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Input, Component} from '@angular/core'; + + @Component({selector: 'hello', template: 'hello'}) + export class Base { + @Input() isActive: boolean; + } + + @Component({selector: 'hello', template: 'hello'}) + export class Derived extends Base { + ngOnDestroy() {} + } + `); + }); + + it('should not migrate classes decorated with @Directive', async () => { + writeFile('/index.ts', dedent` + import {Input, Directive} from '@angular/core'; + + @Directive() + export class Base { + @Input() isActive: boolean; + } + + @Directive({selector: 'other'}) + export class Other extends Base { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Input, Directive} from '@angular/core'; + + @Directive() + export class Base { + @Input() isActive: boolean; + } + + @Directive({selector: 'other'}) + export class Other extends Base { + ngOnDestroy() {} + } + `); + }); + + it('should not migrate when class inherits from component', async () => { + writeFile('/index.ts', dedent` + import {Input, Component} from '@angular/core'; + + @Component({selector: 'my-comp', template: 'my-comp'}) + export class MyComp {} + + export class WithDisabled extends MyComp { + @Input() disabled: boolean; + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Input, Component} from '@angular/core'; + + @Component({selector: 'my-comp', template: 'my-comp'}) + export class MyComp {} + + export class WithDisabled extends MyComp { + @Input() disabled: boolean; + } + `); + }); + + it('should not migrate when class inherits from pipe', async () => { + writeFile('/index.ts', dedent` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'my-pipe'}) + export class MyPipe {} + + export class PipeDerived extends MyPipe { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'my-pipe'}) + export class MyPipe {} + + export class PipeDerived extends MyPipe { + ngOnDestroy() {} + } + `); + }); + + it('should not migrate when class inherits from injectable', async () => { + writeFile('/index.ts', dedent` + import {Injectable} from '@angular/core'; + + @Injectable() + export class MyService {} + + export class ServiceDerived extends MyService { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Injectable} from '@angular/core'; + + @Injectable() + export class MyService {} + + export class ServiceDerived extends MyService { + ngOnDestroy() {} + } + `); + }); + + it('should not migrate when class inherits from directive', async () => { + writeFile('/index.ts', dedent` + import {Directive} from '@angular/core'; + + @Directive({selector: 'hello'}) + export class MyDir {} + + export class DirDerived extends MyDir { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Directive} from '@angular/core'; + + @Directive({selector: 'hello'}) + export class MyDir {} + + export class DirDerived extends MyDir { + ngOnDestroy() {} + } + `); + }); + + it('should not add multiple TODOs for ambiguous classes', async () => { + writeFile('/angular.json', JSON.stringify({ + projects: { + test: { + architect: { + build: {options: {tsConfig: './tsconfig.json'}}, + test: {options: {tsConfig: './tsconfig.json'}}, + } + } + } + })); + writeFile('/index.ts', dedent` + export class MyService { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(tree.readContent('/index.ts')).toBe(dedent` + // TODO: Add Angular decorator. + export class MyService { + ngOnDestroy() {} + } + `); + }); + + it('should not report pipe using `ngOnDestroy` as ambiguous', async () => { + writeFile('/index.ts', dedent` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'my-pipe'}) + export class MyPipe { + ngOnDestroy() {} + transform() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Pipe} from '@angular/core'; + + @Pipe({name: 'my-pipe'}) + export class MyPipe { + ngOnDestroy() {} + transform() {} + } + `); + }); + + it('should not report injectable using `ngOnDestroy` as ambiguous', async () => { + writeFile('/index.ts', dedent` + import {Injectable} from '@angular/core'; + + @Injectable({providedIn: 'root'}) + export class MyService { + ngOnDestroy() {} + } + `); + + await runMigration(); + + expect(warnings.length).toBe(0); + expect(tree.readContent('/index.ts')).toBe(dedent` + import {Injectable} from '@angular/core'; + + @Injectable({providedIn: 'root'}) + export class MyService { + ngOnDestroy() {} + } + `); }); it('should add @Directive to undecorated classes that have @Output', async () => { @@ -298,6 +526,8 @@ describe('Undecorated classes with decorated fields migration', () => { await runMigration(); const fileContent = tree.readContent('/index.ts'); + + expect(warnings.length).toBe(0); 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/); @@ -305,6 +535,7 @@ describe('Undecorated classes with decorated fields migration', () => { 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/); + expect(fileContent).not.toContain('TODO: Add Angular decorator'); }); it('should add @Directive to derived undecorated classes of abstract directives', async () => {