feat(ivy): throw compilation error when providing undecorated classes (#34460)

Adds a compilation error if the consumer tries to pass in an undecorated class into the `providers` of an `NgModule`, or the `providers`/`viewProviders` arrays of a `Directive`/`Component`.

PR Close #34460
This commit is contained in:
crisbeto 2019-12-11 17:59:05 +01:00 committed by Kara Erickson
parent 6057c7a373
commit dcc8ff4ce7
20 changed files with 613 additions and 60 deletions

View File

@ -7,12 +7,13 @@
*/
import {ConstantPool} from '@angular/compiler';
import * as ts from 'typescript';
import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics';
import {FileSystem, LogicalFileSystem, absoluteFrom, dirname, resolve} from '../../../src/ngtsc/file_system';
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, PrivateExportAliasingHost, Reexport, ReferenceEmitter} from '../../../src/ngtsc/imports';
import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from '../../../src/ngtsc/metadata';
import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../../src/ngtsc/metadata';
import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
import {ClassDeclaration} from '../../../src/ngtsc/reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../src/ngtsc/scope';
@ -30,6 +31,7 @@ import {AnalyzedClass, AnalyzedFile, CompiledClass, CompiledFile, DecorationAnal
import {NOOP_DEPENDENCY_TRACKER, analyzeDecorators, isWithinPackage} from './util';
/**
* Simple class that resolves and loads files directly from the filesystem.
*/
@ -85,6 +87,7 @@ export class DecorationAnalyzer {
new PartialEvaluator(this.reflectionHost, this.typeChecker, /* dependencyTracker */ null);
importGraph = new ImportGraph(this.moduleResolver);
cycleAnalyzer = new CycleAnalyzer(this.importGraph);
injectableRegistry = new InjectableClassRegistry(this.reflectionHost);
handlers: DecoratorHandler<unknown, unknown, unknown>[] = [
new ComponentDecoratorHandler(
this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader,
@ -92,28 +95,28 @@ export class DecorationAnalyzer {
/* defaultPreserveWhitespaces */ false,
/* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat,
this.moduleResolver, this.cycleAnalyzer, this.refEmitter, NOOP_DEFAULT_IMPORT_RECORDER,
NOOP_DEPENDENCY_TRACKER, /* annotateForClosureCompiler */ false),
NOOP_DEPENDENCY_TRACKER, this.injectableRegistry, /* annotateForClosureCompiler */ false),
// See the note in ngtsc about why this cast is needed.
// clang-format off
new DirectiveDecoratorHandler(
this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry,
NOOP_DEFAULT_IMPORT_RECORDER, this.isCore,
NOOP_DEFAULT_IMPORT_RECORDER, this.injectableRegistry, this.isCore,
/* annotateForClosureCompiler */ false) as DecoratorHandler<unknown, unknown, unknown>,
// clang-format on
// Pipe handler must be before injectable handler in list so pipe factories are printed
// before injectable factories (so injectable factories can delegate to them)
new PipeDecoratorHandler(
this.reflectionHost, this.evaluator, this.metaRegistry, this.scopeRegistry,
NOOP_DEFAULT_IMPORT_RECORDER, this.isCore),
NOOP_DEFAULT_IMPORT_RECORDER, this.injectableRegistry, this.isCore),
new InjectableDecoratorHandler(
this.reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, this.isCore,
/* strictCtorDeps */ false, /* errorOnDuplicateProv */ false),
/* strictCtorDeps */ false, this.injectableRegistry, /* errorOnDuplicateProv */ false),
new NgModuleDecoratorHandler(
this.reflectionHost, this.evaluator, this.fullMetaReader, this.fullRegistry,
this.scopeRegistry, this.referencesRegistry, this.isCore, /* routeAnalyzer */ null,
this.refEmitter,
/* factoryTracker */ null, NOOP_DEFAULT_IMPORT_RECORDER,
/* annotateForClosureCompiler */ false),
/* annotateForClosureCompiler */ false, this.injectableRegistry),
];
migrations: Migration[] = [
new UndecoratedParentMigration(),

View File

@ -15,7 +15,7 @@ import {absoluteFrom, relative} from '../../file_system';
import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
import {DependencyTracker} from '../../incremental/api';
import {IndexingContext} from '../../indexer';
import {DirectiveMeta, MetadataReader, MetadataRegistry, extractDirectiveGuards} from '../../metadata';
import {DirectiveMeta, InjectableClassRegistry, MetadataReader, MetadataRegistry, extractDirectiveGuards} from '../../metadata';
import {flattenInheritedDirectiveMetadata} from '../../metadata/src/inheritance';
import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
@ -26,10 +26,11 @@ import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
import {SubsetOfKeys} from '../../util/src/typescript';
import {ResourceLoader} from './api';
import {getProviderDiagnostics} from './diagnostics';
import {extractDirectiveMetadata, parseFieldArrayValue} from './directive';
import {compileNgFactoryDefField} from './factory';
import {generateSetClassMetadataCall} from './metadata';
import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, makeDuplicateDeclarationError, readBaseClass, unwrapExpression, wrapFunctionExpressionsInParens} from './util';
import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, makeDuplicateDeclarationError, readBaseClass, resolveProvidersRequiringFactory, unwrapExpression, wrapFunctionExpressionsInParens} from './util';
const EMPTY_MAP = new Map<string, Expression>();
const EMPTY_ARRAY: any[] = [];
@ -53,6 +54,18 @@ export interface ComponentAnalysisData {
guards: ReturnType<typeof extractDirectiveGuards>;
template: ParsedTemplateWithSource;
metadataStmt: Statement|null;
/**
* Providers extracted from the `providers` field of the component annotation which will require
* an Angular factory definition at runtime.
*/
providersRequiringFactory: Set<Reference<ClassDeclaration>>|null;
/**
* Providers extracted from the `viewProviders` field of the component annotation which will
* require an Angular factory definition at runtime.
*/
viewProvidersRequiringFactory: Set<Reference<ClassDeclaration>>|null;
}
export type ComponentResolutionData = Pick<R3ComponentMetadata, ComponentMetadataResolvedFields>;
@ -71,7 +84,9 @@ export class ComponentDecoratorHandler implements
private enableI18nLegacyMessageIdFormat: boolean, private moduleResolver: ModuleResolver,
private cycleAnalyzer: CycleAnalyzer, private refEmitter: ReferenceEmitter,
private defaultImportRecorder: DefaultImportRecorder,
private depTracker: DependencyTracker|null, private annotateForClosureCompiler: boolean) {}
private depTracker: DependencyTracker|null,
private injectableRegistry: InjectableClassRegistry,
private annotateForClosureCompiler: boolean) {}
private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
private elementSchemaRegistry = new DomElementSchemaRegistry();
@ -186,12 +201,27 @@ export class ComponentDecoratorHandler implements
}
}, undefined) !;
const viewProviders: Expression|null = component.has('viewProviders') ?
new WrappedNodeExpr(
this.annotateForClosureCompiler ?
wrapFunctionExpressionsInParens(component.get('viewProviders') !) :
component.get('viewProviders') !) :
null;
// Note that we could technically combine the `viewProvidersRequiringFactory` and
// `providersRequiringFactory` into a single set, but we keep the separate so that
// we can distinguish where an error is coming from when logging the diagnostics in `resolve`.
let viewProvidersRequiringFactory: Set<Reference<ClassDeclaration>>|null = null;
let providersRequiringFactory: Set<Reference<ClassDeclaration>>|null = null;
let wrappedViewProviders: Expression|null = null;
if (component.has('viewProviders')) {
const viewProviders = component.get('viewProviders') !;
viewProvidersRequiringFactory =
resolveProvidersRequiringFactory(viewProviders, this.reflector, this.evaluator);
wrappedViewProviders = new WrappedNodeExpr(
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(viewProviders) :
viewProviders);
}
if (component.has('providers')) {
providersRequiringFactory = resolveProvidersRequiringFactory(
component.get('providers') !, this.reflector, this.evaluator);
}
// Parse the template.
// If a preanalyze phase was executed, the template may already exist in parsed form, so check
@ -290,7 +320,7 @@ export class ComponentDecoratorHandler implements
// These will be replaced during the compilation step, after all `NgModule`s have been
// analyzed and the full compilation scope for the component can be realized.
animations,
viewProviders,
viewProviders: wrappedViewProviders,
i18nUseExternalIds: this.i18nUseExternalIds, relativeContextFilePath,
},
guards: extractDirectiveGuards(node, this.reflector),
@ -298,6 +328,8 @@ export class ComponentDecoratorHandler implements
node, this.reflector, this.defaultImportRecorder, this.isCore,
this.annotateForClosureCompiler),
template,
providersRequiringFactory,
viewProvidersRequiringFactory,
},
};
if (changeDetection !== null) {
@ -321,6 +353,8 @@ export class ComponentDecoratorHandler implements
isComponent: true,
baseClass: analysis.baseClass, ...analysis.guards,
});
this.injectableRegistry.registerInjectable(node);
}
index(
@ -395,14 +429,6 @@ export class ComponentDecoratorHandler implements
resolve(node: ClassDeclaration, analysis: Readonly<ComponentAnalysisData>):
ResolveResult<ComponentResolutionData> {
const duplicateDeclData = this.scopeRegistry.getDuplicateDeclarations(node);
if (duplicateDeclData !== null) {
// This component was declared twice (or more).
return {
diagnostics: [makeDuplicateDeclarationError(node, duplicateDeclData, 'Component')],
};
}
const context = node.getSourceFile();
// Check whether this component was registered with an NgModule. If so, it should be compiled
// under that module's compilation scope.
@ -505,6 +531,34 @@ export class ComponentDecoratorHandler implements
this.scopeRegistry.setComponentAsRequiringRemoteScoping(node);
}
}
const diagnostics: ts.Diagnostic[] = [];
if (analysis.providersRequiringFactory !== null &&
analysis.meta.providers instanceof WrappedNodeExpr) {
const providerDiagnostics = getProviderDiagnostics(
analysis.providersRequiringFactory, analysis.meta.providers !.node,
this.injectableRegistry);
diagnostics.push(...providerDiagnostics);
}
if (analysis.viewProvidersRequiringFactory !== null &&
analysis.meta.viewProviders instanceof WrappedNodeExpr) {
const viewProviderDiagnostics = getProviderDiagnostics(
analysis.viewProvidersRequiringFactory, analysis.meta.viewProviders !.node,
this.injectableRegistry);
diagnostics.push(...viewProviderDiagnostics);
}
const duplicateDeclData = this.scopeRegistry.getDuplicateDeclarations(node);
if (duplicateDeclData !== null) {
diagnostics.push(makeDuplicateDeclarationError(node, duplicateDeclData, 'Component'));
}
if (diagnostics.length > 0) {
return {diagnostics};
}
return {data};
}

View File

@ -0,0 +1,43 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';
import {ErrorCode, makeDiagnostic} from '../../diagnostics';
import {Reference} from '../../imports';
import {InjectableClassRegistry} from '../../metadata';
import {ClassDeclaration} from '../../reflection';
/**
* Gets the diagnostics for a set of provider classes.
* @param providerClasses Classes that should be checked.
* @param providersDeclaration Node that declares the providers array.
* @param registry Registry that keeps track of the registered injectable classes.
*/
export function getProviderDiagnostics(
providerClasses: Set<Reference<ClassDeclaration>>, providersDeclaration: ts.Expression,
registry: InjectableClassRegistry): ts.Diagnostic[] {
const diagnostics: ts.Diagnostic[] = [];
for (const provider of providerClasses) {
if (registry.isInjectable(provider.node)) {
continue;
}
const contextNode = provider.getOriginForDiagnostics(providersDeclaration);
diagnostics.push(makeDiagnostic(
ErrorCode.UNDECORATED_PROVIDER, contextNode,
`The class '${provider.node.name.text}' cannot be created via dependency injection, as it does not have an Angular decorator. This will result in an error at runtime.
Either add the @Injectable() decorator to '${provider.node.name.text}', or configure a different provider (such as a provider with 'useFactory').
`,
[{node: provider.node, messageText: `'${provider.node.name.text}' is declared here.`}]));
}
return diagnostics;
}

View File

@ -11,16 +11,17 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder, Reference} from '../../imports';
import {MetadataRegistry} from '../../metadata';
import {InjectableClassRegistry, MetadataRegistry} from '../../metadata';
import {extractDirectiveGuards} from '../../metadata/src/util';
import {DynamicValue, EnumValue, PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection';
import {LocalModuleScopeRegistry} from '../../scope';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform';
import {getProviderDiagnostics} from './diagnostics';
import {compileNgFactoryDefField} from './factory';
import {generateSetClassMetadataCall} from './metadata';
import {findAngularDecorator, getConstructorDependencies, isAngularDecorator, makeDuplicateDeclarationError, readBaseClass, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
import {findAngularDecorator, getConstructorDependencies, isAngularDecorator, makeDuplicateDeclarationError, readBaseClass, resolveProvidersRequiringFactory, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
const EMPTY_OBJECT: {[key: string]: string} = {};
const FIELD_DECORATORS = [
@ -37,13 +38,16 @@ export interface DirectiveHandlerData {
guards: ReturnType<typeof extractDirectiveGuards>;
meta: R3DirectiveMetadata;
metadataStmt: Statement|null;
providersRequiringFactory: Set<Reference<ClassDeclaration>>|null;
}
export class DirectiveDecoratorHandler implements
DecoratorHandler<Decorator|null, DirectiveHandlerData, unknown> {
constructor(
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry,
private defaultImportRecorder: DefaultImportRecorder, private isCore: boolean,
private defaultImportRecorder: DefaultImportRecorder,
private injectableRegistry: InjectableClassRegistry, private isCore: boolean,
private annotateForClosureCompiler: boolean) {}
readonly precedence = HandlerPrecedence.PRIMARY;
@ -88,6 +92,12 @@ export class DirectiveDecoratorHandler implements
return {};
}
let providersRequiringFactory: Set<Reference<ClassDeclaration>>|null = null;
if (directiveResult !== undefined && directiveResult.decorator.has('providers')) {
providersRequiringFactory = resolveProvidersRequiringFactory(
directiveResult.decorator.get('providers') !, this.reflector, this.evaluator);
}
return {
analysis: {
meta: analysis,
@ -95,7 +105,7 @@ export class DirectiveDecoratorHandler implements
node, this.reflector, this.defaultImportRecorder, this.isCore,
this.annotateForClosureCompiler),
baseClass: readBaseClass(node, this.reflector, this.evaluator),
guards: extractDirectiveGuards(node, this.reflector),
guards: extractDirectiveGuards(node, this.reflector), providersRequiringFactory
}
};
}
@ -115,18 +125,28 @@ export class DirectiveDecoratorHandler implements
isComponent: false,
baseClass: analysis.baseClass, ...analysis.guards,
});
this.injectableRegistry.registerInjectable(node);
}
resolve(node: ClassDeclaration): ResolveResult<unknown> {
resolve(node: ClassDeclaration, analysis: DirectiveHandlerData): ResolveResult<unknown> {
const diagnostics: ts.Diagnostic[] = [];
if (analysis.providersRequiringFactory !== null &&
analysis.meta.providers instanceof WrappedNodeExpr) {
const providerDiagnostics = getProviderDiagnostics(
analysis.providersRequiringFactory, analysis.meta.providers !.node,
this.injectableRegistry);
diagnostics.push(...providerDiagnostics);
}
const duplicateDeclData = this.scopeRegistry.getDuplicateDeclarations(node);
if (duplicateDeclData !== null) {
// This directive was declared twice (or more).
return {
diagnostics: [makeDuplicateDeclarationError(node, duplicateDeclData, 'Directive')],
};
diagnostics.push(makeDuplicateDeclarationError(node, duplicateDeclData, 'Directive'));
}
return {};
return {diagnostics: diagnostics.length > 0 ? diagnostics : undefined};
}
compile(
@ -160,10 +180,8 @@ export function extractDirectiveMetadata(
clazz: ClassDeclaration, decorator: Readonly<Decorator|null>, reflector: ReflectionHost,
evaluator: PartialEvaluator, defaultImportRecorder: DefaultImportRecorder, isCore: boolean,
flags: HandlerFlags, annotateForClosureCompiler: boolean,
defaultSelector: string | null = null): {
decorator: Map<string, ts.Expression>,
metadata: R3DirectiveMetadata,
}|undefined {
defaultSelector: string | null =
null): {decorator: Map<string, ts.Expression>, metadata: R3DirectiveMetadata}|undefined {
let directive: Map<string, ts.Expression>;
if (decorator === null || decorator.args === null || decorator.args.length === 0) {
directive = new Map<string, ts.Expression>();
@ -390,7 +408,6 @@ export function extractQueriesFromDecorator(
view: R3QueryMetadata[],
} {
const content: R3QueryMetadata[] = [], view: R3QueryMetadata[] = [];
const expr = unwrapExpression(queryData);
if (!ts.isObjectLiteralExpression(queryData)) {
throw new Error(`queries metadata must be an object literal`);
}

View File

@ -11,6 +11,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder} from '../../imports';
import {InjectableClassRegistry} from '../../metadata';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform';
@ -33,6 +34,7 @@ export class InjectableDecoratorHandler implements
constructor(
private reflector: ReflectionHost, private defaultImportRecorder: DefaultImportRecorder,
private isCore: boolean, private strictCtorDeps: boolean,
private injectableRegistry: InjectableClassRegistry,
/**
* What to do if the injectable already contains a ɵprov property.
*
@ -80,6 +82,8 @@ export class InjectableDecoratorHandler implements
};
}
register(node: ClassDeclaration): void { this.injectableRegistry.registerInjectable(node); }
compile(node: ClassDeclaration, analysis: Readonly<InjectableHandlerData>): CompileResult[] {
const res = compileIvyInjectable(analysis.meta);
const statements = res.statements;

View File

@ -11,8 +11,8 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics';
import {DefaultImportRecorder, Reference, ReferenceEmitter} from '../../imports';
import {MetadataReader, MetadataRegistry} from '../../metadata';
import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator';
import {InjectableClassRegistry, MetadataReader, MetadataRegistry} from '../../metadata';
import {PartialEvaluator, ResolvedValue, ResolvedValueArray} from '../../partial_evaluator';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection';
import {NgModuleRouteAnalyzer} from '../../routing';
import {LocalModuleScopeRegistry, ScopeData} from '../../scope';
@ -20,9 +20,10 @@ import {FactoryTracker} from '../../shims';
import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform';
import {getSourceFile} from '../../util/src/typescript';
import {getProviderDiagnostics} from './diagnostics';
import {generateSetClassMetadataCall} from './metadata';
import {ReferencesRegistry} from './references_registry';
import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
import {combineResolvers, findAngularDecorator, forwardRefResolver, getValidConstructorDependencies, isExpressionForwardReference, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from './util';
export interface NgModuleAnalysis {
mod: R3NgModuleMetadata;
@ -35,6 +36,8 @@ export interface NgModuleAnalysis {
exports: Reference<ClassDeclaration>[];
id: Expression|null;
factorySymbolName: string;
providersRequiringFactory: Set<Reference<ClassDeclaration>>|null;
providers: ts.Expression|null;
}
export interface NgModuleResolution { injectorImports: Expression[]; }
@ -54,7 +57,8 @@ export class NgModuleDecoratorHandler implements
private routeAnalyzer: NgModuleRouteAnalyzer|null, private refEmitter: ReferenceEmitter,
private factoryTracker: FactoryTracker|null,
private defaultImportRecorder: DefaultImportRecorder,
private annotateForClosureCompiler: boolean, private localeId?: string) {}
private annotateForClosureCompiler: boolean,
private injectableRegistry: InjectableClassRegistry, private localeId?: string) {}
readonly precedence = HandlerPrecedence.PRIMARY;
readonly name = NgModuleDecoratorHandler.name;
@ -240,7 +244,7 @@ export class NgModuleDecoratorHandler implements
};
const rawProviders = ngModule.has('providers') ? ngModule.get('providers') ! : null;
const providers = rawProviders !== null ?
const wrapperProviders = rawProviders !== null ?
new WrappedNodeExpr(
this.annotateForClosureCompiler ? wrapFunctionExpressionsInParens(rawProviders) :
rawProviders) :
@ -264,7 +268,7 @@ export class NgModuleDecoratorHandler implements
internalType,
deps: getValidConstructorDependencies(
node, this.reflector, this.defaultImportRecorder, this.isCore),
providers,
providers: wrapperProviders,
imports: injectorImports,
};
@ -277,6 +281,10 @@ export class NgModuleDecoratorHandler implements
declarations: declarationRefs, rawDeclarations,
imports: importRefs,
exports: exportRefs,
providers: rawProviders,
providersRequiringFactory: rawProviders ?
resolveProvidersRequiringFactory(rawProviders, this.reflector, this.evaluator) :
null,
metadataStmt: generateSetClassMetadataCall(
node, this.reflector, this.defaultImportRecorder, this.isCore,
this.annotateForClosureCompiler),
@ -301,6 +309,8 @@ export class NgModuleDecoratorHandler implements
if (this.factoryTracker !== null) {
this.factoryTracker.track(node.getSourceFile(), analysis.factorySymbolName);
}
this.injectableRegistry.registerInjectable(node);
}
resolve(node: ClassDeclaration, analysis: Readonly<NgModuleAnalysis>):
@ -313,6 +323,12 @@ export class NgModuleDecoratorHandler implements
diagnostics.push(...scopeDiagnostics);
}
if (analysis.providersRequiringFactory !== null) {
const providerDiagnostics = getProviderDiagnostics(
analysis.providersRequiringFactory, analysis.providers !, this.injectableRegistry);
diagnostics.push(...providerDiagnostics);
}
const data: NgModuleResolution = {
injectorImports: [],
};

View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {DefaultImportRecorder, Reference} from '../../imports';
import {MetadataRegistry} from '../../metadata';
import {InjectableClassRegistry, MetadataRegistry} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection';
import {LocalModuleScopeRegistry} from '../../scope';
@ -30,7 +30,8 @@ export class PipeDecoratorHandler implements DecoratorHandler<Decorator, PipeHan
constructor(
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry,
private defaultImportRecorder: DefaultImportRecorder, private isCore: boolean) {}
private defaultImportRecorder: DefaultImportRecorder,
private injectableRegistry: InjectableClassRegistry, private isCore: boolean) {}
readonly precedence = HandlerPrecedence.PRIMARY;
readonly name = PipeDecoratorHandler.name;
@ -115,6 +116,8 @@ export class PipeDecoratorHandler implements DecoratorHandler<Decorator, PipeHan
register(node: ClassDeclaration, analysis: Readonly<PipeHandlerData>): void {
const ref = new Reference(node);
this.metaRegistry.registerPipeMetadata({ref, name: analysis.meta.pipeName});
this.injectableRegistry.registerInjectable(node);
}
resolve(node: ClassDeclaration): ResolveResult<unknown> {

View File

@ -408,6 +408,50 @@ export function makeDuplicateDeclarationError(
`The ${kind} '${node.name.text}' is declared by more than one NgModule.`, context);
}
/**
* Resolves the given `rawProviders` into `ClassDeclarations` and returns
* a set containing those that are known to require a factory definition.
* @param rawProviders Expression that declared the providers array in the source.
*/
export function resolveProvidersRequiringFactory(
rawProviders: ts.Expression, reflector: ReflectionHost,
evaluator: PartialEvaluator): Set<Reference<ClassDeclaration>> {
const providers = new Set<Reference<ClassDeclaration>>();
const resolvedProviders = evaluator.evaluate(rawProviders);
if (!Array.isArray(resolvedProviders)) {
return providers;
}
resolvedProviders.forEach(function processProviders(provider) {
let tokenClass: Reference|null = null;
if (Array.isArray(provider)) {
// If we ran into an array, recurse into it until we've resolve all the classes.
provider.forEach(processProviders);
} else if (provider instanceof Reference) {
tokenClass = provider;
} else if (provider instanceof Map && provider.has('useClass') && !provider.has('deps')) {
const useExisting = provider.get('useClass') !;
if (useExisting instanceof Reference) {
tokenClass = useExisting;
}
}
if (tokenClass !== null && reflector.isClass(tokenClass.node)) {
const constructorParameters = reflector.getConstructorParameters(tokenClass.node);
// Note that we only want to capture providers with a non-trivial constructor,
// because they're the ones that might be using DI and need to be decorated.
if (constructorParameters !== null && constructorParameters.length > 0) {
providers.add(tokenClass as Reference<ClassDeclaration>);
}
}
});
return providers;
}
/**
* Create an R3Reference for a class.
*

View File

@ -10,7 +10,7 @@ import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@ -59,13 +59,14 @@ runInEachFileSystem(() => {
new ReferenceEmitter([]), null);
const metaReader = new CompoundMetadataReader([metaRegistry, dtsReader]);
const refEmitter = new ReferenceEmitter([]);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new ComponentDecoratorHandler(
reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, scopeRegistry,
/* isCore */ false, new NoopResourceLoader(), /* rootDirs */[''],
/* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true,
/* enableI18nLegacyMessageIdFormat */ false, moduleResolver, cycleAnalyzer, refEmitter,
NOOP_DEFAULT_IMPORT_RECORDER, /* depTracker */ null,
NOOP_DEFAULT_IMPORT_RECORDER, /* depTracker */ null, injectableRegistry,
/* annotateForClosureCompiler */ false);
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));

View File

@ -8,7 +8,7 @@
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
import {DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
import {DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {ClassDeclaration, TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@ -48,8 +48,10 @@ runInEachFileSystem(() => {
const scopeRegistry = new LocalModuleScopeRegistry(
metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
null);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new DirectiveDecoratorHandler(
reflectionHost, evaluator, scopeRegistry, scopeRegistry, NOOP_DEFAULT_IMPORT_RECORDER,
injectableRegistry,
/* isCore */ false, /* annotateForClosureCompiler */ false);
const analyzeDirective = (dirName: string) => {

View File

@ -9,6 +9,7 @@ import {ErrorCode, FatalDiagnosticError, ngErrorCode} from '../../diagnostics';
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../imports';
import {InjectableClassRegistry} from '../../metadata';
import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
import {getDeclaration, makeProgram} from '../../testing';
import {InjectableDecoratorHandler} from '../src/injectable';
@ -67,9 +68,10 @@ function setupHandler(errorOnDuplicateProv: boolean) {
]);
const checker = program.getTypeChecker();
const reflectionHost = new TypeScriptReflectionHost(checker);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new InjectableDecoratorHandler(
reflectionHost, NOOP_DEFAULT_IMPORT_RECORDER, /* isCore */ false,
/* strictCtorDeps */ false, errorOnDuplicateProv);
/* strictCtorDeps */ false, injectableRegistry, errorOnDuplicateProv);
const TestClass = getDeclaration(program, ENTRY_FILE, 'TestClass', isNamedClassDeclaration);
const ɵprov = reflectionHost.getMembersOfClass(TestClass).find(member => member.name === 'ɵprov');
if (ɵprov === undefined) {

View File

@ -12,7 +12,7 @@ import * as ts from 'typescript';
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {LocalIdentifierStrategy, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../metadata';
import {PartialEvaluator} from '../../partial_evaluator';
import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@ -66,11 +66,12 @@ runInEachFileSystem(() => {
metaRegistry, new MetadataDtsModuleScopeResolver(dtsReader, null),
new ReferenceEmitter([]), null);
const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]);
const injectableRegistry = new InjectableClassRegistry(reflectionHost);
const handler = new NgModuleDecoratorHandler(
reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry,
/* isCore */ false, /* routeAnalyzer */ null, refEmitter, /* factoryTracker */ null,
NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false);
NOOP_DEFAULT_IMPORT_RECORDER, /* annotateForClosureCompiler */ false, injectableRegistry);
const TestModule =
getDeclaration(program, _('/entry.ts'), 'TestModule', isNamedClassDeclaration);
const detected =

View File

@ -26,6 +26,9 @@ export enum ErrorCode {
PARAM_MISSING_TOKEN = 2003,
DIRECTIVE_MISSING_SELECTOR = 2004,
/** Raised when an undecorated class is passed in as a provider to a module or a directive. */
UNDECORATED_PROVIDER = 2005,
SYMBOL_NOT_EXPORTED = 3001,
SYMBOL_EXPORTED_UNDER_DIFFERENT_NAME = 3002,

View File

@ -8,5 +8,5 @@
export * from './src/api';
export {DtsMetadataReader} from './src/dts';
export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry';
export {CompoundMetadataRegistry, LocalMetadataRegistry, InjectableClassRegistry} from './src/registry';
export {extractDirectiveGuards, CompoundMetadataReader} from './src/util';

View File

@ -7,9 +7,10 @@
*/
import {Reference} from '../../imports';
import {ClassDeclaration} from '../../reflection';
import {ClassDeclaration, ReflectionHost} from '../../reflection';
import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from './api';
import {hasInjectableFields} from './util';
/**
* A registry of directive, pipe, and module metadata for types defined in the current compilation
@ -59,3 +60,22 @@ export class CompoundMetadataRegistry implements MetadataRegistry {
}
}
}
/**
* Registry that keeps track of classes that can be constructed via dependency injection (e.g.
* injectables, directives, pipes).
*/
export class InjectableClassRegistry {
private classes = new Set<ClassDeclaration>();
constructor(private host: ReflectionHost) {}
registerInjectable(declaration: ClassDeclaration): void { this.classes.add(declaration); }
isInjectable(declaration: ClassDeclaration): boolean {
// Figure out whether the class is injectable based on the registered classes, otherwise
// fall back to looking at its members since we might not have been able register the class
// if it was compiled already.
return this.classes.has(declaration) || hasInjectableFields(declaration, this.host);
}
}

View File

@ -174,3 +174,10 @@ function afterUnderscore(str: string): string {
}
return str.substr(pos + 1);
}
/** Returns whether a class declaration has the necessary class fields to make it injectable. */
export function hasInjectableFields(clazz: ClassDeclaration, host: ReflectionHost): boolean {
const members = host.getMembersOfClass(clazz);
return members.some(
({isStatic, name}) => isStatic && (name === 'ɵprov' || name === 'ɵfac' || name === 'ɵinj'));
}

View File

@ -23,6 +23,7 @@ import {IncrementalDriver} from './incremental';
import {IndexedComponent, IndexingContext} from './indexer';
import {generateAnalysis} from './indexer/src/transform';
import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry, MetadataReader} from './metadata';
import {InjectableClassRegistry} from './metadata/src/registry';
import {ModuleWithProvidersScanner} from './modulewithproviders';
import {PartialEvaluator} from './partial_evaluator';
import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf';
@ -629,6 +630,7 @@ export class NgtscProgram implements api.Program {
localMetaReader, depScopeReader, this.refEmitter, this.aliasingHost);
const scopeReader: ComponentScopeReader = this.scopeRegistry;
const metaRegistry = new CompoundMetadataRegistry([localMetaRegistry, this.scopeRegistry]);
const injectableRegistry = new InjectableClassRegistry(this.reflector);
this.metaReader = new CompoundMetadataReader([localMetaReader, dtsReader]);
@ -658,26 +660,27 @@ export class NgtscProgram implements api.Program {
this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false,
this.options.enableI18nLegacyMessageIdFormat !== false, this.moduleResolver,
this.cycleAnalyzer, this.refEmitter, this.defaultImportTracker,
this.incrementalDriver.depGraph, this.closureCompilerEnabled),
this.incrementalDriver.depGraph, injectableRegistry, this.closureCompilerEnabled),
// TODO(alxhub): understand why the cast here is necessary (something to do with `null` not
// being assignable to `unknown` when wrapped in `Readonly`).
// clang-format off
new DirectiveDecoratorHandler(
this.reflector, evaluator, metaRegistry, this.scopeRegistry, this.defaultImportTracker,
this.reflector, evaluator, metaRegistry, this.scopeRegistry, this.defaultImportTracker, injectableRegistry,
this.isCore, this.closureCompilerEnabled) as Readonly<DecoratorHandler<unknown, unknown, unknown>>,
// clang-format on
// Pipe handler must be before injectable handler in list so pipe factories are printed
// before injectable factories (so injectable factories can delegate to them)
new PipeDecoratorHandler(
this.reflector, evaluator, metaRegistry, this.scopeRegistry, this.defaultImportTracker,
this.isCore),
injectableRegistry, this.isCore),
new InjectableDecoratorHandler(
this.reflector, this.defaultImportTracker, this.isCore,
this.options.strictInjectionParameters || false),
this.options.strictInjectionParameters || false, injectableRegistry),
new NgModuleDecoratorHandler(
this.reflector, evaluator, this.metaReader, metaRegistry, this.scopeRegistry,
referencesRegistry, this.isCore, this.routeAnalyzer, this.refEmitter, this.factoryTracker,
this.defaultImportTracker, this.closureCompilerEnabled, this.options.i18nInLocale),
this.defaultImportTracker, this.closureCompilerEnabled, injectableRegistry,
this.options.i18nInLocale),
];
return new TraitCompiler(

View File

@ -363,7 +363,7 @@ export class TraitCompiler {
}
}
if (result.diagnostics !== undefined) {
if (result.diagnostics !== undefined && result.diagnostics.length > 0) {
trait = trait.toErrored(result.diagnostics);
} else {
if (result.data !== undefined) {

View File

@ -89,3 +89,5 @@ export class EventEmitter<T> {
export interface QueryList<T>/* implements Iterable<T> */ { [Symbol.iterator]: () => Iterator<T>; }
export type NgIterable<T> = Array<T>| Iterable<T>;
export class NgZone {}

View File

@ -5222,6 +5222,334 @@ export const Foo = Foo__PRE_R3__;
});
});
describe('undecorated providers', () => {
it('should error when an undecorated class, with a non-trivial constructor, is provided directly in a module',
() => {
env.write('test.ts', `
import {NgModule, NgZone} from '@angular/core';
class NotAService {
constructor(ngZone: NgZone) {}
}
@NgModule({
providers: [NotAService]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should error when an undecorated class is provided via useClass', () => {
env.write('test.ts', `
import {NgModule, Injectable, NgZone} from '@angular/core';
@Injectable({providedIn: 'root'})
class Service {}
class NotAService {
constructor(ngZone: NgZone) {}
}
@NgModule({
providers: [{provide: Service, useClass: NotAService}]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should not error when an undecorated class is provided via useClass with deps', () => {
env.write('test.ts', `
import {NgModule, Injectable, NgZone} from '@angular/core';
@Injectable({providedIn: 'root'})
class Service {}
class NotAService {
constructor(ngZone: NgZone) {}
}
@NgModule({
providers: [{provide: Service, useClass: NotAService, deps: [NgZone]}]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should error when an undecorated class is provided via an array', () => {
env.write('test.ts', `
import {NgModule, Injectable, NgZone} from '@angular/core';
@Injectable({providedIn: 'root'})
class Service {}
class NotAService {
constructor(ngZone: NgZone) {}
}
@NgModule({
providers: [Service, [NotAService]]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should error when an undecorated class is provided to a directive', () => {
env.write('test.ts', `
import {NgModule, Directive, NgZone} from '@angular/core';
class NotAService {
constructor(ngZone: NgZone) {}
}
@Directive({
selector: '[some-dir]',
providers: [NotAService]
})
class SomeDirective {}
@NgModule({
declarations: [SomeDirective]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should error when an undecorated class is provided to a component', () => {
env.write('test.ts', `
import {NgModule, Component, NgZone} from '@angular/core';
class NotAService {
constructor(ngZone: NgZone) {}
}
@Component({
selector: 'some-comp',
template: '',
providers: [NotAService]
})
class SomeComponent {}
@NgModule({
declarations: [SomeComponent]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should error when an undecorated class is provided to a component via viewProviders',
() => {
env.write('test.ts', `
import {NgModule, Component, NgZone} from '@angular/core';
class NotAService {
constructor(ngZone: NgZone) {}
}
@Component({
selector: 'some-comp',
template: '',
viewProviders: [NotAService]
})
class SomeComponent {}
@NgModule({
declarations: [SomeComponent]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should not error when a class with a factory is provided', () => {
env.write('test.ts', `
import {NgModule, Pipe} from '@angular/core';
@Pipe({
name: 'some-pipe'
})
class SomePipe {}
@NgModule({
declarations: [SomePipe],
providers: [SomePipe]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should not error when an NgModule is provided', () => {
env.write('test.ts', `
import {Injectable, NgModule} from '@angular/core';
@Injectable()
export class Service {}
@NgModule({
})
class SomeModule {
constructor(dep: Service) {}
}
@NgModule({
providers: [SomeModule],
})
export class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should not error when an undecorated class from a declaration file is provided', () => {
env.write('node_modules/@angular/core/testing/index.d.ts', `
export declare class Testability {
}
`);
env.write('test.ts', `
import {NgModule} from '@angular/core';
import {Testability} from '@angular/core/testing';
@NgModule({
providers: [Testability]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should not error when an undecorated class without a constructor from a declaration file is provided via useClass',
() => {
env.write('node_modules/@angular/core/testing/index.d.ts', `
export declare class Testability {
}
`);
env.write('test.ts', `
import {NgModule, Injectable} from '@angular/core';
import {Testability} from '@angular/core/testing';
@Injectable()
class TestingService {}
@NgModule({
providers: [{provide: TestingService, useClass: Testability}]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should not error if the undecorated class does not have a constructor or the constructor is blank',
() => {
env.write('test.ts', `
import {NgModule, NgZone} from '@angular/core';
class NoConstructorService {
}
class BlankConstructorService {
}
@NgModule({
providers: [NoConstructorService, BlankConstructorService]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
it('should error when an undecorated class with a non-trivial constructor in a declaration file is provided via useClass',
() => {
env.write('node_modules/@angular/core/testing/index.d.ts', `
export declare class NgZone {}
export declare class Testability {
constructor(ngZone: NgZone) {}
}
`);
env.write('test.ts', `
import {NgModule, Injectable} from '@angular/core';
import {Testability} from '@angular/core/testing';
@Injectable()
class TestingService {}
@NgModule({
providers: [{provide: TestingService, useClass: Testability}]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toContain('cannot be created via dependency injection');
});
it('should not error when an class with a factory definition and a non-trivial constructor in a declaration file is provided via useClass',
() => {
env.write('node_modules/@angular/core/testing/index.d.ts', `
import * as i0 from '@angular/core';
export declare class NgZone {}
export declare class Testability {
static ɵfac: i0.ɵɵFactoryDef<Testability>;
constructor(ngZone: NgZone) {}
}
`);
env.write('test.ts', `
import {NgModule, Injectable} from '@angular/core';
import {Testability} from '@angular/core/testing';
@Injectable()
class TestingService {}
@NgModule({
providers: [{provide: TestingService, useClass: Testability}]
})
export class SomeModule {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(0);
});
});
});
function expectTokenAtPosition<T extends ts.Node>(