feat(ngcc): migrate services that are missing `@Injectable()` (#33362)

A class that is provided as Angular service is required to have an
`@Injectable()` decorator so that the compiler generates its injectable
definition for the runtime. Applications are automatically migrated
using the "missing-injectable" schematic, however libraries built for
older version of Angular may not yet satisfy this requirement.

This commit ports the "missing-injectable" schematic to a migration that
is ran when ngcc is processing a library. This ensures that any service
that is provided from an NgModule or Directive/Component will have an
`@Injectable()` decorator.

PR Close #33362
This commit is contained in:
JoostK 2019-10-20 20:40:48 +02:00 committed by Andrew Kushnir
parent 0de2dbfec1
commit 31b9492951
8 changed files with 1082 additions and 137 deletions

View File

@ -119,7 +119,8 @@ export class DecorationAnalyzer {
.map(sourceFile => this.analyzeFile(sourceFile))
.filter(isDefined);
const migrationHost = new DefaultMigrationHost(
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers, analyzedFiles);
this.reflectionHost, this.fullMetaReader, this.evaluator, this.handlers,
this.bundle.entryPoint.path, analyzedFiles);
analyzedFiles.forEach(analyzedFile => this.migrateFile(migrationHost, analyzedFile));
analyzedFiles.forEach(analyzedFile => this.resolveFile(analyzedFile));
const compiledFiles = analyzedFiles.map(analyzedFile => this.compileFile(analyzedFile));

View File

@ -6,15 +6,18 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
import {AbsoluteFsPath} from '../../../src/ngtsc/file_system';
import {MetadataReader} from '../../../src/ngtsc/metadata';
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {DecoratorHandler} from '../../../src/ngtsc/transform';
import {NgccReflectionHost} from '../host/ngcc_host';
import {MigrationHost} from '../migrations/migration';
import {AnalyzedClass, AnalyzedFile} from './types';
import {analyzeDecorators} from './util';
import {analyzeDecorators, isWithinPackage} from './util';
/**
* The standard implementation of `MigrationHost`, which is created by the
@ -24,7 +27,7 @@ export class DefaultMigrationHost implements MigrationHost {
constructor(
readonly reflectionHost: NgccReflectionHost, readonly metadata: MetadataReader,
readonly evaluator: PartialEvaluator, private handlers: DecoratorHandler<any, any>[],
private analyzedFiles: AnalyzedFile[]) {}
private entryPointPath: AbsoluteFsPath, private analyzedFiles: AnalyzedFile[]) {}
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void {
const classSymbol = this.reflectionHost.getClassSymbol(clazz) !;
@ -41,6 +44,25 @@ export class DefaultMigrationHost implements MigrationHost {
mergeAnalyzedClasses(oldAnalyzedClass, newAnalyzedClass);
}
}
getAllDecorators(clazz: ClassDeclaration): Decorator[]|null {
const sourceFile = clazz.getSourceFile();
const analyzedFile = this.analyzedFiles.find(file => file.sourceFile === sourceFile);
if (analyzedFile === undefined) {
return null;
}
const analyzedClass = analyzedFile.analyzedClasses.find(c => c.declaration === clazz);
if (analyzedClass === undefined) {
return null;
}
return analyzedClass.decorators;
}
isInScope(clazz: ClassDeclaration): boolean {
return isWithinPackage(this.entryPointPath, clazz.getSourceFile());
}
}
function getOrCreateAnalyzedFile(

View File

@ -42,4 +42,20 @@ export interface MigrationHost {
* @param decorator the decorator to inject.
*/
injectSyntheticDecorator(clazz: ClassDeclaration, decorator: Decorator): void;
/**
* Retrieves all decorators that are associated with the class, including synthetic decorators
* that have been injected before.
* @param clazz the class for which all decorators are retrieved.
* @returns the list of the decorators, or null if the class was not decorated.
*/
getAllDecorators(clazz: ClassDeclaration): Decorator[]|null;
/**
* Determines whether the provided class in within scope of the entry-point that is currently
* being compiled.
* @param clazz the class for which to determine whether it is within the current entry-point.
* @returns true if the file is part of the compiled entry-point, false otherwise.
*/
isInScope(clazz: ClassDeclaration): boolean;
}

View File

@ -0,0 +1,192 @@
/**
* @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 {forwardRefResolver} from '../../../src/ngtsc/annotations';
import {Reference} from '../../../src/ngtsc/imports';
import {ResolvedValue, ResolvedValueMap} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {Migration, MigrationHost} from './migration';
import {createInjectableDecorator, isClassDeclaration} from './utils';
/**
* Ensures that classes that are provided as an Angular service in either `NgModule.providers` or
* `Directive.providers`/`Component.viewProviders` are decorated with one of the `@Injectable`,
* `@Directive`, `@Component` or `@Pipe` decorators, adding an `@Injectable()` decorator when none
* are present.
*
* At least one decorator is now mandatory, as otherwise the compiler would not compile an
* injectable definition for the service. This is unlike View Engine, where having just an unrelated
* decorator may have been sufficient for the service to become injectable.
*
* In essence, this migration operates on classes that are themselves an NgModule, Directive or
* Component. Their metadata is statically evaluated so that their "providers"/"viewProviders"
* properties can be analyzed. For any provider that refers to an undecorated class, the class will
* be migrated to have an `@Injectable()` decorator.
*
* This implementation mirrors the "missing-injectable" schematic.
*/
export class MissingInjectableMigration implements Migration {
apply(clazz: ClassDeclaration, host: MigrationHost): ts.Diagnostic|null {
const decorators = host.reflectionHost.getDecoratorsOfDeclaration(clazz);
if (decorators === null) {
return null;
}
for (const decorator of decorators) {
const name = getAngularCoreDecoratorName(decorator);
if (name === 'NgModule') {
migrateNgModuleProviders(decorator, host);
} else if (name === 'Directive') {
migrateDirectiveProviders(decorator, host, /* isComponent */ false);
} else if (name === 'Component') {
migrateDirectiveProviders(decorator, host, /* isComponent */ true);
}
}
return null;
}
}
/**
* Iterates through all `NgModule.providers` and adds the `@Injectable()` decorator to any provider
* that is not otherwise decorated.
*/
function migrateNgModuleProviders(decorator: Decorator, host: MigrationHost): void {
if (decorator.args === null || decorator.args.length !== 1) {
return;
}
const metadata = host.evaluator.evaluate(decorator.args[0], forwardRefResolver);
if (!(metadata instanceof Map)) {
return;
}
migrateProviders(metadata, 'providers', host);
// TODO(alxhub): we should probably also check for `ModuleWithProviders` here.
}
/**
* Iterates through all `Directive.providers` and if `isComponent` is set to true also
* `Component.viewProviders` and adds the `@Injectable()` decorator to any provider that is not
* otherwise decorated.
*/
function migrateDirectiveProviders(
decorator: Decorator, host: MigrationHost, isComponent: boolean): void {
if (decorator.args === null || decorator.args.length !== 1) {
return;
}
const metadata = host.evaluator.evaluate(decorator.args[0], forwardRefResolver);
if (!(metadata instanceof Map)) {
return;
}
migrateProviders(metadata, 'providers', host);
if (isComponent) {
migrateProviders(metadata, 'viewProviders', host);
}
}
/**
* Given an object with decorator metadata, iterates through the list of providers to add
* `@Injectable()` to any provider that is not otherwise decorated.
*/
function migrateProviders(metadata: ResolvedValueMap, field: string, host: MigrationHost): void {
if (!metadata.has(field)) {
return;
}
const providers = metadata.get(field) !;
if (!Array.isArray(providers)) {
return;
}
for (const provider of providers) {
migrateProvider(provider, host);
}
}
/**
* Analyzes a single provider entry and determines the class that is required to have an
* `@Injectable()` decorator.
*/
function migrateProvider(provider: ResolvedValue, host: MigrationHost): void {
if (provider instanceof Map) {
if (!provider.has('provide') || provider.has('useValue') || provider.has('useFactory') ||
provider.has('useExisting')) {
return;
}
if (provider.has('useClass')) {
// {provide: ..., useClass: SomeClass, deps: [...]} does not require a decorator on SomeClass,
// as the provider itself configures 'deps'. Only if 'deps' is missing will this require a
// factory to exist on SomeClass.
if (!provider.has('deps')) {
migrateProviderClass(provider.get('useClass') !, host);
}
} else {
migrateProviderClass(provider.get('provide') !, host);
}
} else if (Array.isArray(provider)) {
for (const v of provider) {
migrateProvider(v, host);
}
} else {
migrateProviderClass(provider, host);
}
}
/**
* Given a provider class, adds the `@Injectable()` decorator if no other relevant Angular decorator
* is present on the class.
*/
function migrateProviderClass(provider: ResolvedValue, host: MigrationHost): void {
// Providers that do not refer to a class cannot be migrated.
if (!(provider instanceof Reference)) {
return;
}
const clazz = provider.node as ts.Declaration;
if (isClassDeclaration(clazz) && host.isInScope(clazz) && needsInjectableDecorator(clazz, host)) {
host.injectSyntheticDecorator(clazz, createInjectableDecorator(clazz));
}
}
const NO_MIGRATE_DECORATORS = new Set(['Injectable', 'Directive', 'Component', 'Pipe']);
/**
* Determines if the given class needs to be decorated with `@Injectable()` based on whether it
* already has an Angular decorator applied.
*/
function needsInjectableDecorator(clazz: ClassDeclaration, host: MigrationHost): boolean {
const decorators = host.getAllDecorators(clazz);
if (decorators === null) {
return true;
}
for (const decorator of decorators) {
const name = getAngularCoreDecoratorName(decorator);
if (name !== null && NO_MIGRATE_DECORATORS.has(name)) {
return false;
}
}
return true;
}
/**
* Determines the original name of a decorator if it is from '@angular/core'. For other decorators,
* null is returned.
*/
export function getAngularCoreDecoratorName(decorator: Decorator): string|null {
if (decorator.import === null || decorator.import.from !== '@angular/core') {
return null;
}
return decorator.import.name;
}

View File

@ -54,6 +54,27 @@ export function createDirectiveDecorator(clazz: ClassDeclaration): Decorator {
};
}
/**
* Create an empty `Injectable` decorator that will be associated with the `clazz`.
*/
export function createInjectableDecorator(clazz: ClassDeclaration): Decorator {
const decoratorType = ts.createIdentifier('Injectable');
const decoratorNode = ts.createObjectLiteral([
ts.createPropertyAssignment('type', decoratorType),
ts.createPropertyAssignment('args', ts.createArrayLiteral([])),
]);
setParentPointers(clazz.getSourceFile(), decoratorNode);
return {
name: 'Injectable',
identifier: decoratorType,
import: {name: 'Injectable', from: '@angular/core'},
node: decoratorNode,
args: [],
};
}
/**
* Ensure that a tree of AST nodes have their parents wired up.
*/

View File

@ -6,156 +6,256 @@
* found in the LICENSE file at https://angular.io/license
*/
import {ErrorCode} from '../../../src/ngtsc/diagnostics';
import {AbsoluteFsPath, absoluteFrom} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {ClassDeclaration, Decorator} from '../../../src/ngtsc/reflection';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../../src/ngtsc/transform';
import {DefaultMigrationHost} from '../../src/analysis/migration_host';
import {AnalyzedClass, AnalyzedFile} from '../../src/analysis/types';
import {NgccClassSymbol} from '../../src/host/ngcc_host';
describe('DefaultMigrationHost', () => {
describe('injectSyntheticDecorator()', () => {
const mockHost: any = {
getClassSymbol: (node: any): NgccClassSymbol | undefined => {
const symbol = { valueDeclaration: node, name: node.name.text } as any;
return {
name: node.name.text,
declaration: symbol,
implementation: symbol,
runInEachFileSystem(() => {
describe('DefaultMigrationHost', () => {
let _: typeof absoluteFrom;
let entryPointPath: AbsoluteFsPath;
let mockHost: any;
let mockMetadata: any = {};
let mockEvaluator: any = {};
let mockClazz: any;
let mockDecorator: any = {name: 'MockDecorator'};
beforeEach(() => {
_ = absoluteFrom;
entryPointPath = _('/node_modules/some-package/entry-point');
mockHost = {
getClassSymbol: (node: any): NgccClassSymbol | undefined => {
const symbol = { valueDeclaration: node, name: node.name.text } as any;
return {
name: node.name.text,
declaration: symbol,
implementation: symbol,
};
},
};
const mockSourceFile: any = {
fileName: _('/node_modules/some-package/entry-point/test-file.js'),
};
mockClazz = {
name: {text: 'MockClazz'},
getSourceFile: () => mockSourceFile,
};
});
describe('injectSyntheticDecorator()', () => {
it('should call `detect()` on each of the provided handlers', () => {
const log: string[] = [];
const handler1 = new TestHandler('handler1', log);
const handler2 = new TestHandler('handler2', log);
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler1, handler2], entryPointPath, []);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(log).toEqual([
`handler1:detect:MockClazz:MockDecorator`,
`handler2:detect:MockClazz:MockDecorator`,
]);
});
it('should call `analyze()` on each of the provided handlers whose `detect()` call returns a result',
() => {
const log: string[] = [];
const handler1 = new TestHandler('handler1', log);
const handler2 = new AlwaysDetectHandler('handler2', log);
const handler3 = new TestHandler('handler3', log);
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler1, handler2, handler3],
entryPointPath, []);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(log).toEqual([
`handler1:detect:MockClazz:MockDecorator`,
`handler2:detect:MockClazz:MockDecorator`,
`handler3:detect:MockClazz:MockDecorator`,
'handler2:analyze:MockClazz',
]);
});
it('should add a newly `AnalyzedFile` to the `analyzedFiles` object', () => {
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0].name).toEqual('MockClazz');
});
it('should add a newly `AnalyzedClass` to an existing `AnalyzedFile` object', () => {
const DUMMY_CLASS_1: any = {};
const DUMMY_CLASS_2: any = {};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [DUMMY_CLASS_1, DUMMY_CLASS_2],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(3);
expect(analyzedFiles[0].analyzedClasses[2].name).toEqual('MockClazz');
});
it('should add a new decorator into an already existing `AnalyzedClass`', () => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: null,
};
},
};
const mockMetadata: any = {};
const mockEvaluator: any = {};
const mockClazz: any = {
name: {text: 'MockClazz'},
getSourceFile: () => { fileName: 'test-file.js'; },
};
const mockDecorator: any = {name: 'MockDecorator'};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass);
expect(analyzedClass.decorators !.length).toEqual(1);
expect(analyzedClass.decorators ![0].name).toEqual('MockDecorator');
});
it('should call `detect()` on each of the provided handlers', () => {
const log: string[] = [];
const handler1 = new TestHandler('handler1', log);
const handler2 = new TestHandler('handler2', log);
const host =
new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler1, handler2], []);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(log).toEqual([
`handler1:detect:MockClazz:MockDecorator`,
`handler2:detect:MockClazz:MockDecorator`,
]);
it('should merge a new decorator into pre-existing decorators an already existing `AnalyzedClass`',
() => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: [{name: 'OtherDecorator'} as Decorator],
};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass);
expect(analyzedClass.decorators !.length).toEqual(2);
expect(analyzedClass.decorators ![1].name).toEqual('MockDecorator');
});
it('should throw an error if the injected decorator already exists', () => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: [{name: 'MockDecorator'} as Decorator],
};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
expect(() => host.injectSyntheticDecorator(mockClazz, mockDecorator))
.toThrow(jasmine.objectContaining(
{code: ErrorCode.NGCC_MIGRATION_DECORATOR_INJECTION_ERROR}));
});
});
it('should call `analyze()` on each of the provided handlers whose `detect()` call returns a result',
() => {
const log: string[] = [];
const handler1 = new TestHandler('handler1', log);
const handler2 = new AlwaysDetectHandler('handler2', log);
const handler3 = new TestHandler('handler3', log);
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler1, handler2, handler3], []);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(log).toEqual([
`handler1:detect:MockClazz:MockDecorator`,
`handler2:detect:MockClazz:MockDecorator`,
`handler3:detect:MockClazz:MockDecorator`,
'handler2:analyze:MockClazz',
]);
});
describe('getAllDecorators', () => {
it('should be null for unknown source files', () => {
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
it('should add a newly `AnalyzedFile` to the `analyzedFiles` object', () => {
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [];
const host =
new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0].name).toEqual('MockClazz');
const decorators = host.getAllDecorators(mockClazz);
expect(decorators).toBeNull();
});
it('should be null for unknown classes', () => {
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
const sourceFile: any = {};
const unrelatedClass: any = {
getSourceFile: () => sourceFile,
};
analyzedFiles.push({sourceFile, analyzedClasses: [unrelatedClass]});
const decorators = host.getAllDecorators(mockClazz);
expect(decorators).toBeNull();
});
it('should include injected decorators', () => {
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const existingDecorator = { name: 'ExistingDecorator' } as Decorator;
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: [existingDecorator],
};
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], entryPointPath, analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
const decorators = host.getAllDecorators(mockClazz) !;
expect(decorators.length).toBe(2);
expect(decorators[0]).toBe(existingDecorator);
expect(decorators[1]).toBe(mockDecorator);
});
});
it('should add a newly `AnalyzedClass` to an existing `AnalyzedFile` object', () => {
const DUMMY_CLASS_1: any = {};
const DUMMY_CLASS_2: any = {};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [DUMMY_CLASS_1, DUMMY_CLASS_2],
}];
const host =
new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(3);
expect(analyzedFiles[0].analyzedClasses[2].name).toEqual('MockClazz');
});
describe('isInScope', () => {
it('should be true for nodes within the entry-point', () => {
const analyzedFiles: AnalyzedFile[] = [];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles);
it('should add a new decorator into an already existing `AnalyzedClass`', () => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: null,
};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host =
new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass);
expect(analyzedClass.decorators !.length).toEqual(1);
expect(analyzedClass.decorators ![0].name).toEqual('MockDecorator');
});
const sourceFile: any = {
fileName: _('/node_modules/some-package/entry-point/relative.js'),
};
const clazz: any = {
getSourceFile: () => sourceFile,
};
expect(host.isInScope(clazz)).toBe(true);
});
it('should merge a new decorator into pre-existing decorators an already existing `AnalyzedClass`',
() => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: [{name: 'OtherDecorator'} as Decorator],
};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles);
host.injectSyntheticDecorator(mockClazz, mockDecorator);
expect(analyzedFiles.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses.length).toEqual(1);
expect(analyzedFiles[0].analyzedClasses[0]).toBe(analyzedClass);
expect(analyzedClass.decorators !.length).toEqual(2);
expect(analyzedClass.decorators ![1].name).toEqual('MockDecorator');
});
it('should be false for nodes outside the entry-point', () => {
const analyzedFiles: AnalyzedFile[] = [];
const host = new DefaultMigrationHost(
mockHost, mockMetadata, mockEvaluator, [], entryPointPath, analyzedFiles);
it('should throw an error if the injected decorator already exists', () => {
const analyzedClass: AnalyzedClass = {
name: 'MockClazz',
declaration: mockClazz,
matches: [],
decorators: [{name: 'MockDecorator'} as Decorator],
};
const log: string[] = [];
const handler = new AlwaysDetectHandler('handler', log);
const analyzedFiles: AnalyzedFile[] = [{
sourceFile: mockClazz.getSourceFile(),
analyzedClasses: [analyzedClass],
}];
const host =
new DefaultMigrationHost(mockHost, mockMetadata, mockEvaluator, [handler], analyzedFiles);
expect(() => host.injectSyntheticDecorator(mockClazz, mockDecorator))
.toThrow(
jasmine.objectContaining({code: ErrorCode.NGCC_MIGRATION_DECORATOR_INJECTION_ERROR}));
const sourceFile: any = {
fileName: _('/node_modules/some-package/other-entry/index.js'),
};
const clazz: any = {
getSourceFile: () => sourceFile,
};
expect(host.isInScope(clazz)).toBe(false);
});
});
});
});

View File

@ -0,0 +1,592 @@
/**
* @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 {DecorationAnalyses} from '../../src/analysis/types';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {MissingInjectableMigration, getAngularCoreDecoratorName} from '../../src/migrations/missing_injectable_migration';
import {MockLogger} from '../helpers/mock_logger';
import {getRootFiles, makeTestEntryPointBundle} from '../helpers/utils';
runInEachFileSystem(() => {
describe('MissingInjectableMigration', () => {
let _: typeof absoluteFrom;
let INDEX_FILENAME: AbsoluteFsPath;
beforeEach(() => {
_ = absoluteFrom;
INDEX_FILENAME = _('/node_modules/test-package/index.js');
});
describe('NgModule', () => runTests('NgModule', 'providers'));
describe('Directive', () => runTests('Directive', 'providers'));
describe('Component', () => {
runTests('Component', 'providers');
runTests('Component', 'viewProviders');
it('should migrate all providers defined in "viewProviders" and "providers" in the same ' +
'component',
() => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {Component} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
export class TestClass {}
TestClass.decorators = [
{ type: Component, args: [{
template: "",
providers: [ServiceA],
viewProviders: [ServiceB],
}]
}
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceB')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceC')).toBe(false);
});
});
function runTests(
type: 'NgModule' | 'Directive' | 'Component', propName: 'providers' | 'viewProviders') {
const args = type === 'Component' ? 'template: "", ' : '';
it(`should migrate type provider in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class OtherService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'OtherService')).toBe(false);
});
it(`should migrate object literal provider in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class OtherService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [{provide: MyService}]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'OtherService')).toBe(false);
});
it(`should migrate object literal provider with forwardRef in ${type}`, async() => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}, forwardRef} from '@angular/core';
export class MyService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [{provide: forwardRef(() => MyService) }]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
});
it(`should not migrate object literal provider with "useValue" in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [{provide: MyService, useValue: null }]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it(`should not migrate object literal provider with "useFactory" in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [{provide: MyService, useFactory: () => null }]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it(`should not migrate object literal provider with "useExisting" in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class MyToken {}
export class MyTokenAlias {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [
MyService,
{provide: MyToken, useExisting: MyService},
{provide: MyTokenAlias, useExisting: MyToken},
]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'MyToken')).toBe(false);
expect(hasInjectableDecorator(index, analysis, 'MyTokenAlias')).toBe(false);
});
it(`should migrate object literal provider with "useClass" in ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class MyService {}
export class MyToken {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [{provide: MyToken, useClass: MyService}]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'MyToken')).toBe(false);
});
it('should not migrate provider which is already decorated with @Injectable', () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {Injectable, ${type}} from '@angular/core';
export class MyService {}
MyService.decorators = [
{ type: Injectable }
];
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(getInjectableDecorators(index, analysis, 'MyService').length).toBe(1);
});
it('should not migrate provider which is already decorated with @Directive', () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {Directive, ${type}} from '@angular/core';
export class MyService {}
MyService.decorators = [
{ type: Directive }
];
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it('should not migrate provider which is already decorated with @Component', () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {Component, ${type}} from '@angular/core';
export class MyService {}
MyService.decorators = [
{ type: Component, args: [{template: ""}] }
];
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it('should not migrate provider which is already decorated with @Pipe', () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {Pipe, ${type}} from '@angular/core';
export class MyService {}
MyService.decorators = [
{ type: Pipe, args: [{name: "pipe"}] }
];
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it(`should migrate multiple providers in same ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [ServiceA, ServiceB]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceB')).toBe(true);
});
it(`should migrate multiple mixed providers in same ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
export class ServiceD {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [
ServiceA,
{provide: ServiceB},
{provide: SomeToken, useClass: ServiceC},
]
}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceB')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceC')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceD')).toBe(false);
});
it(`should migrate multiple nested providers in same ${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
export class ServiceD {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [
ServiceA,
[
{provide: ServiceB},
ServiceC,
],
]}]
}
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceB')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceC')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceD')).toBe(false);
});
it('should migrate providers referenced indirectly', () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class ServiceC {}
const PROVIDERS = [ServiceA, ServiceB];
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: PROVIDERS}] }
];
`
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceB')).toBe(true);
expect(hasInjectableDecorator(index, analysis, 'ServiceC')).toBe(false);
});
it(`should migrate provider once if referenced in multiple ${type} definitions`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class ServiceB {}
export class TestClassA {}
TestClassA.decorators = [
{ type: ${type}, args: [{${args}${propName}: [ServiceA]}] }
];
export class TestClassB {}
TestClassB.decorators = [
{ type: ${type}, args: [{${args}${propName}: [ServiceA, ServiceB]}] }
];
`
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(getInjectableDecorators(index, analysis, 'ServiceA').length).toBe(1);
expect(getInjectableDecorators(index, analysis, 'ServiceB').length).toBe(1);
});
type !== 'Component' && it(`should support @${type} without metadata argument`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
export class ServiceA {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'ServiceA')).toBe(false);
});
it(`should migrate services in a different file`, () => {
const SERVICE_FILENAME = _('/node_modules/test-package/service.js');
const {program, analysis} = setUpAndAnalyzeProgram([
{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
import {MyService} from './service';
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
},
{
name: SERVICE_FILENAME,
contents: `
export declare class MyService {}
`,
}
]);
const index = program.getSourceFile(SERVICE_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
});
it(`should not migrate services in a different package`, () => {
const SERVICE_FILENAME = _('/node_modules/external/index.d.ts');
const {program, analysis} = setUpAndAnalyzeProgram([
{
name: INDEX_FILENAME,
contents: `
import {${type}} from '@angular/core';
import {MyService} from 'external';
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
},
{
name: SERVICE_FILENAME,
contents: `
export declare class MyService {}
`,
}
]);
const index = program.getSourceFile(SERVICE_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
it(`should deal with renamed imports for @${type}`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type} as Renamed} from '@angular/core';
export class MyService {}
export class TestClass {}
TestClass.decorators = [
{ type: Renamed, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(true);
});
it(`should deal with decorators named @${type} not from '@angular/core'`, () => {
const {program, analysis} = setUpAndAnalyzeProgram([{
name: INDEX_FILENAME,
contents: `
import {${type}} from 'other';
export class MyService {}
export class TestClass {}
TestClass.decorators = [
{ type: ${type}, args: [{${args}${propName}: [MyService]}] }
];
`,
}]);
const index = program.getSourceFile(INDEX_FILENAME) !;
expect(hasInjectableDecorator(index, analysis, 'MyService')).toBe(false);
});
}
function setUpAndAnalyzeProgram(testFiles: TestFile[]) {
loadTestFiles(testFiles);
loadFakeCore(getFileSystem());
const errors: ts.Diagnostic[] = [];
const rootFiles = getRootFiles(testFiles);
const bundle = makeTestEntryPointBundle('test-package', '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 MissingInjectableMigration()];
return {program, analysis: analyzer.analyzeProgram(), errors};
}
function getInjectableDecorators(
sourceFile: ts.SourceFile, analysis: DecorationAnalyses, className: string) {
const file = analysis.get(sourceFile);
if (file === undefined) {
return [];
}
const clazz = file.compiledClasses.find(c => c.name === className);
if (clazz === undefined || clazz.decorators === null) {
return [];
}
return clazz.decorators.filter(
decorator => getAngularCoreDecoratorName(decorator) === 'Injectable');
}
function hasInjectableDecorator(
sourceFile: ts.SourceFile, analysis: DecorationAnalyses, className: string) {
return getInjectableDecorators(sourceFile, analysis, className).length > 0;
}
});
});

View File

@ -16,3 +16,4 @@ export {InjectableDecoratorHandler} from './src/injectable';
export {NgModuleDecoratorHandler} from './src/ng_module';
export {PipeDecoratorHandler} from './src/pipe';
export {NoopReferencesRegistry, ReferencesRegistry} from './src/references_registry';
export {forwardRefResolver} from './src/util';