From 48d43444415f03e7b71b1af42d93f6f9be5e138a Mon Sep 17 00:00:00 2001 From: JoostK Date: Tue, 4 Jan 2022 19:03:30 +0100 Subject: [PATCH] fix(compiler-cli): exclude abstract classes from `strictInjectionParameters` requirement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In AOT compilations, the `strictInjectionParameters` compiler option can be enabled to report errors when an `@Injectable` annotated class has a constructor with parameters that do not provide an injection token, e.g. only a primitive type or interface. Since Ivy it's become required that any class with Angular behavior (e.g. the `ngOnDestroy` lifecycle hook) is decorated using an Angular decorator, which meant that `@Injectable()` may need to have been added to abstract base classes. Doing so would then report an error if `strictInjectionParameters` is enabled, if the abstract class has an incompatible constructor for DI purposes. This may be fine though, as a subclass may call the constructor explicitly without relying on Angular's DI mechanism. Therefore, this commit excludes abstract classes from the `strictInjectionParameters` check. This avoids an error from being reported at compile time. If the constructor ends up being used by Angular's DI system at runtime, then the factory function of the abstract class will throw an error by means of the `ɵɵinvalidFactory` instruction. In addition to the runtime error, this commit also analyzes the inheritance chain of an injectable without a constructor to verify that their inherited constructor is valid. Closes #37914 --- goldens/public-api/compiler-cli/error_code.md | 1 + .../ngcc/src/analysis/decoration_analyzer.ts | 14 +- .../src/ngtsc/annotations/common/index.ts | 1 + .../annotations/common/src/diagnostics.ts | 126 ++++-- .../common/src/injectable_registry.ts | 52 +++ .../src/ngtsc/annotations/common/src/util.ts | 5 + .../annotations/component/src/handler.ts | 24 +- .../component/test/component_spec.ts | 7 +- .../annotations/directive/src/handler.ts | 12 +- .../annotations/directive/test/BUILD.bazel | 1 + .../directive/test/directive_spec.ts | 6 +- .../annotations/ng_module/src/handler.ts | 8 +- .../ng_module/test/ng_module_spec.ts | 6 +- .../src/ngtsc/annotations/src/injectable.ts | 45 ++- .../src/ngtsc/annotations/src/pipe.ts | 8 +- .../src/ngtsc/annotations/test/BUILD.bazel | 1 + .../ngtsc/annotations/test/injectable_spec.ts | 8 +- .../compiler-cli/src/ngtsc/core/BUILD.bazel | 1 + .../src/ngtsc/core/src/compiler.ts | 17 +- .../src/ngtsc/diagnostics/src/error_code.ts | 6 + .../compiler-cli/src/ngtsc/metadata/index.ts | 4 +- .../src/ngtsc/metadata/src/registry.ts | 24 +- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 370 ++++++++++++++++++ 23 files changed, 634 insertions(+), 113 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts diff --git a/goldens/public-api/compiler-cli/error_code.md b/goldens/public-api/compiler-cli/error_code.md index c08d1d2be40332..4d8af40a613b51 100644 --- a/goldens/public-api/compiler-cli/error_code.md +++ b/goldens/public-api/compiler-cli/error_code.md @@ -43,6 +43,7 @@ export enum ErrorCode { IMPORT_CYCLE_DETECTED = 3003, IMPORT_GENERATION_FAILURE = 3004, INJECTABLE_DUPLICATE_PROV = 9001, + INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2013, INLINE_TCB_REQUIRED = 8900, INLINE_TYPE_CTOR_REQUIRED = 8901, INVALID_BANANA_IN_BOX = 8101, diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index a5a20fce19ff05..7f017c6d96a9c5 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -10,12 +10,13 @@ import ts from 'typescript'; import {ParsedConfiguration} from '../../..'; import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations'; +import {InjectableClassRegistry} from '../../../src/ngtsc/annotations/common'; import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../../src/ngtsc/cycles'; import {isFatalDiagnosticError} from '../../../src/ngtsc/diagnostics'; import {absoluteFromSourceFile, LogicalFileSystem, ReadonlyFileSystem} from '../../../src/ngtsc/file_system'; import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, PrivateExportAliasingHost, Reexport, ReferenceEmitter} from '../../../src/ngtsc/imports'; import {SemanticSymbol} from '../../../src/ngtsc/incremental/semantic_graph'; -import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, HostDirectivesResolver, InjectableClassRegistry, LocalMetadataRegistry, ResourceRegistry} from '../../../src/ngtsc/metadata'; +import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, ResourceRegistry} from '../../../src/ngtsc/metadata'; import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../src/ngtsc/perf'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../../src/ngtsc/scope'; @@ -97,7 +98,7 @@ export class DecorationAnalyzer { new PartialEvaluator(this.reflectionHost, this.typeChecker, /* dependencyTracker */ null); importGraph = new ImportGraph(this.typeChecker, NOOP_PERF_RECORDER); cycleAnalyzer = new CycleAnalyzer(this.importGraph); - injectableRegistry = new InjectableClassRegistry(this.reflectionHost); + injectableRegistry = new InjectableClassRegistry(this.reflectionHost, this.isCore); hostDirectivesResolver = new HostDirectivesResolver(this.fullMetaReader); typeCheckScopeRegistry = new TypeCheckScopeRegistry( this.scopeRegistry, this.fullMetaReader, this.hostDirectivesResolver); @@ -105,8 +106,9 @@ export class DecorationAnalyzer { new ComponentDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader, this.scopeRegistry, this.dtsModuleScopeResolver, this.scopeRegistry, - this.typeCheckScopeRegistry, new ResourceRegistry(), this.isCore, this.resourceManager, - this.rootDirs, !!this.compilerOptions.preserveWhitespaces, + this.typeCheckScopeRegistry, new ResourceRegistry(), this.isCore, + /* strictCtorDeps */ false, this.resourceManager, this.rootDirs, + !!this.compilerOptions.preserveWhitespaces, /* i18nUseExternalIds */ true, this.bundle.enableI18nLegacyMessageIdFormat, /* usePoisonedData */ false, /* i18nNormalizeLineEndingsInICUs */ false, this.moduleResolver, this.cycleAnalyzer, @@ -119,7 +121,7 @@ export class DecorationAnalyzer { // clang-format off new DirectiveDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry, - this.fullMetaReader, this.injectableRegistry, this.refEmitter, this.isCore, + this.fullMetaReader, this.injectableRegistry, this.refEmitter, this.isCore, /* strictCtorDeps */ false, /* semanticDepGraphUpdater */ null, !!this.compilerOptions.annotateForClosureCompiler, // In ngcc we want to compile undecorated classes with Angular features. As of @@ -136,7 +138,7 @@ export class DecorationAnalyzer { this.reflectionHost, this.evaluator, this.metaRegistry, this.scopeRegistry, this.injectableRegistry, this.isCore, NOOP_PERF_RECORDER), new InjectableDecoratorHandler( - this.reflectionHost, this.isCore, + this.reflectionHost, this.evaluator, this.isCore, /* strictCtorDeps */ false, this.injectableRegistry, NOOP_PERF_RECORDER, /* errorOnDuplicateProv */ false), new NgModuleDecoratorHandler( diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/index.ts b/packages/compiler-cli/src/ngtsc/annotations/common/index.ts index f6f6ba4deea3ab..31c9461ece6f83 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/index.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/index.ts @@ -11,6 +11,7 @@ export * from './src/di'; export * from './src/diagnostics'; export * from './src/evaluation'; export * from './src/factory'; +export * from './src/injectable_registry'; export * from './src/metadata'; export * from './src/references_registry'; export * from './src/schema'; diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts index 8eec9fbd7abf2d..9aa5ee6aef93d3 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/diagnostics.ts @@ -10,13 +10,14 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; import {Reference} from '../../../imports'; -import {HostDirectiveMeta, InjectableClassRegistry, MetadataReader} from '../../../metadata'; +import {HostDirectiveMeta, MetadataReader} from '../../../metadata'; import {describeResolvedType, DynamicValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator'; import {ClassDeclaration, ReflectionHost} from '../../../reflection'; import {DeclarationData, LocalModuleScopeRegistry} from '../../../scope'; import {identifierOfNode} from '../../../util/src/typescript'; -import {readBaseClass} from './util'; +import {InjectableClassRegistry} from './injectable_registry'; +import {isAbstractClassDeclaration, readBaseClass} from './util'; /** @@ -102,7 +103,10 @@ export function getProviderDiagnostics( const diagnostics: ts.Diagnostic[] = []; for (const provider of providerClasses) { - if (registry.isInjectable(provider.node)) { + const injectableMeta = registry.getInjectableMeta(provider.node); + if (injectableMeta !== null) { + // The provided type is recognized as injectable, so we don't report a diagnostic for this + // provider. continue; } @@ -124,9 +128,9 @@ Either add the @Injectable() decorator to '${ } export function getDirectiveDiagnostics( - node: ClassDeclaration, reader: MetadataReader, evaluator: PartialEvaluator, - reflector: ReflectionHost, scopeRegistry: LocalModuleScopeRegistry, - kind: string): ts.Diagnostic[]|null { + node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, + evaluator: PartialEvaluator, reflector: ReflectionHost, scopeRegistry: LocalModuleScopeRegistry, + strictInjectionParameters: boolean, kind: 'Directive'|'Component'): ts.Diagnostic[]|null { let diagnostics: ts.Diagnostic[]|null = []; const addDiagnostics = (more: ts.Diagnostic|ts.Diagnostic[]|null) => { @@ -147,7 +151,8 @@ export function getDirectiveDiagnostics( addDiagnostics(makeDuplicateDeclarationError(node, duplicateDeclarations, kind)); } - addDiagnostics(checkInheritanceOfDirective(node, reader, reflector, evaluator)); + addDiagnostics(checkInheritanceOfInjectable( + node, injectableRegistry, reflector, evaluator, strictInjectionParameters, kind)); return diagnostics; } @@ -192,9 +197,42 @@ export function getUndecoratedClassWithAngularFeaturesDiagnostic(node: ClassDecl `Angular decorator.`); } -export function checkInheritanceOfDirective( - node: ClassDeclaration, reader: MetadataReader, reflector: ReflectionHost, - evaluator: PartialEvaluator): ts.Diagnostic|null { +export function checkInheritanceOfInjectable( + node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, reflector: ReflectionHost, + evaluator: PartialEvaluator, strictInjectionParameters: boolean, + kind: 'Directive'|'Component'|'Pipe'|'Injectable'): ts.Diagnostic|null { + const classWithCtor = findInheritedCtor(node, injectableRegistry, reflector, evaluator); + if (classWithCtor === null || classWithCtor.isCtorValid) { + // The class does not inherit a constructor, or the inherited constructor is compatible + // with DI; no need to report a diagnostic. + return null; + } + + if (!classWithCtor.isDecorated) { + // The inherited constructor exists in a class that does not have an Angular decorator. + // This is an error, as there won't be a factory definition available for DI to invoke + // the constructor. + return getInheritedUndecoratedCtorDiagnostic(node, classWithCtor.ref, kind); + } + + if (!strictInjectionParameters || isAbstractClassDeclaration(node)) { + // An invalid constructor is only reported as error under `strictInjectionParameters` and + // only for concrete classes; follow the same exclusions for derived types. + return null; + } + + return getInheritedInvalidCtorDiagnostic(node, classWithCtor.ref, kind); +} + +interface ClassWithCtor { + ref: Reference; + isCtorValid: boolean; + isDecorated: boolean; +} + +export function findInheritedCtor( + node: ClassDeclaration, injectableRegistry: InjectableClassRegistry, reflector: ReflectionHost, + evaluator: PartialEvaluator): ClassWithCtor|null { if (!reflector.isClass(node) || reflector.getConstructorParameters(node) !== null) { // We should skip nodes that aren't classes. If a constructor exists, then no base class // definition is required on the runtime side - it's legal to inherit from any class. @@ -211,44 +249,64 @@ export function checkInheritanceOfDirective( return null; } - // We can skip the base class if it has metadata. - const baseClassMeta = reader.getDirectiveMetadata(baseClass); - if (baseClassMeta !== null) { - return null; - } - - // If the base class has a blank constructor we can skip it since it can't be using DI. - const baseClassConstructorParams = reflector.getConstructorParameters(baseClass.node); - const newParentClass = readBaseClass(baseClass.node, reflector, evaluator); - - if (baseClassConstructorParams !== null && baseClassConstructorParams.length > 0) { - // This class has a non-trivial constructor, that's an error! - return getInheritedUndecoratedCtorDiagnostic(node, baseClass, reader); - } else if (baseClassConstructorParams !== null || newParentClass === null) { - // This class has a trivial constructor, or no constructor + is the - // top of the inheritance chain, so it's okay. - return null; + const injectableMeta = injectableRegistry.getInjectableMeta(baseClass.node); + if (injectableMeta !== null) { + if (injectableMeta.ctorDeps !== null) { + // The class has an Angular decorator with a constructor. + return { + ref: baseClass, + isCtorValid: injectableMeta.ctorDeps !== 'invalid', + isDecorated: true, + }; + } + } else { + const baseClassConstructorParams = reflector.getConstructorParameters(baseClass.node); + if (baseClassConstructorParams !== null) { + // The class is not decorated, but it does have constructor. An undecorated class is only + // allowed to have a constructor without parameters, otherwise it is invalid. + return { + ref: baseClass, + isCtorValid: baseClassConstructorParams.length === 0, + isDecorated: false, + }; + } } // Go up the chain and continue - baseClass = newParentClass; + baseClass = readBaseClass(baseClass.node, reflector, evaluator); } return null; } +function getInheritedInvalidCtorDiagnostic( + node: ClassDeclaration, baseClass: Reference, + kind: 'Directive'|'Component'|'Pipe'|'Injectable') { + const baseClassName = baseClass.debugName; + + return makeDiagnostic( + ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR, node.name, + `The ${kind.toLowerCase()} ${node.name.text} inherits its constructor from ${ + baseClassName}, ` + + `but the latter has a constructor parameter that is not compatible with dependency injection. ` + + `Either add an explicit constructor to ${node.name.text} or change ${ + baseClassName}'s constructor to ` + + `use parameters that are valid for DI.`); +} + function getInheritedUndecoratedCtorDiagnostic( - node: ClassDeclaration, baseClass: Reference, reader: MetadataReader) { - const subclassMeta = reader.getDirectiveMetadata(new Reference(node))!; - const dirOrComp = subclassMeta.isComponent ? 'Component' : 'Directive'; + node: ClassDeclaration, baseClass: Reference, + kind: 'Directive'|'Component'|'Pipe'|'Injectable') { const baseClassName = baseClass.debugName; + const baseNeedsDecorator = + kind === 'Component' || kind === 'Directive' ? 'Directive' : 'Injectable'; return makeDiagnostic( ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR, node.name, - `The ${dirOrComp.toLowerCase()} ${node.name.text} inherits its constructor from ${ + `The ${kind.toLowerCase()} ${node.name.text} inherits its constructor from ${ baseClassName}, ` + `but the latter does not have an Angular decorator of its own. Dependency injection will not be able to ` + - `resolve the parameters of ${ - baseClassName}'s constructor. Either add a @Directive decorator ` + + `resolve the parameters of ${baseClassName}'s constructor. Either add a @${ + baseNeedsDecorator} decorator ` + `to ${baseClassName}, or add an explicit constructor to ${node.name.text}.`); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts new file mode 100644 index 00000000000000..0d1a94ab2aabb9 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/injectable_registry.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright Google LLC 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 {R3DependencyMetadata} from '@angular/compiler'; + +import {hasInjectableFields} from '../../../metadata'; +import {ClassDeclaration, ReflectionHost} from '../../../reflection'; + +import {getConstructorDependencies, unwrapConstructorDependencies} from './di'; + + +export interface InjectableMeta { + ctorDeps: R3DependencyMetadata[]|'invalid'|null; +} + +/** + * Registry that keeps track of classes that can be constructed via dependency injection (e.g. + * injectables, directives, pipes). + */ +export class InjectableClassRegistry { + private classes = new Map(); + + constructor(private host: ReflectionHost, private isCore: boolean) {} + + registerInjectable(declaration: ClassDeclaration, meta: InjectableMeta): void { + this.classes.set(declaration, meta); + } + + getInjectableMeta(declaration: ClassDeclaration): InjectableMeta|null { + // 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 to register the class + // if it was compiled in another compilation unit. + if (this.classes.has(declaration)) { + return this.classes.get(declaration)!; + } + + if (!hasInjectableFields(declaration, this.host)) { + return null; + } + + const ctorDeps = getConstructorDependencies(declaration, this.host, this.isCore); + const meta: InjectableMeta = { + ctorDeps: unwrapConstructorDependencies(ctorDeps), + }; + this.classes.set(declaration, meta); + return meta; + } +} diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts index d6bc9af79c1080..b6c1982493e8a2 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/util.ts @@ -400,3 +400,8 @@ export function getOriginNodeForDiagnostics( return container; } } + +export function isAbstractClassDeclaration(clazz: ClassDeclaration): boolean { + return clazz.modifiers !== undefined && + clazz.modifiers.some(mod => mod.kind === ts.SyntaxKind.AbstractKeyword); +} diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts index 50058c9692fec5..bd41257a64be3a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts @@ -16,7 +16,7 @@ import {assertSuccessfulReferenceEmit, ImportedFile, ModuleResolver, Reference, import {DependencyTracker} from '../../../incremental/api'; import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph'; import {IndexingContext} from '../../../indexer'; -import {DirectiveMeta, extractDirectiveTypeCheckMeta, HostDirectivesResolver, InjectableClassRegistry, MatchSource, MetadataReader, MetadataRegistry, MetaKind, PipeMeta, ResourceRegistry} from '../../../metadata'; +import {DirectiveMeta, extractDirectiveTypeCheckMeta, HostDirectivesResolver, MatchSource, MetadataReader, MetadataRegistry, MetaKind, PipeMeta, ResourceRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, DeclarationNode, Decorator, ReflectionHost, reflectObjectLiteral} from '../../../reflection'; @@ -26,7 +26,7 @@ import {TypeCheckableDirectiveMeta, TypeCheckContext} from '../../../typecheck/a import {ExtendedTemplateChecker} from '../../../typecheck/extended/api'; import {getSourceFile} from '../../../util/src/typescript'; import {Xi18nContext} from '../../../xi18n'; -import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getDirectiveDiagnostics, getProviderDiagnostics, isExpressionForwardReference, readBaseClass, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, validateHostDirectives, wrapFunctionExpressionsInParens,} from '../../common'; +import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getDirectiveDiagnostics, getProviderDiagnostics, InjectableClassRegistry, isExpressionForwardReference, readBaseClass, resolveEnumValue, resolveImportedFile, resolveLiteral, resolveProvidersRequiringFactory, ResourceLoader, toFactoryMetadata, validateHostDirectives, wrapFunctionExpressionsInParens,} from '../../common'; import {extractDirectiveMetadata, parseFieldArrayValue} from '../../directive'; import {createModuleWithProvidersResolver, NgModuleSymbol} from '../../ng_module'; @@ -51,12 +51,13 @@ export class ComponentDecoratorHandler implements private scopeRegistry: LocalModuleScopeRegistry, private typeCheckScopeRegistry: TypeCheckScopeRegistry, private resourceRegistry: ResourceRegistry, private isCore: boolean, - private resourceLoader: ResourceLoader, private rootDirs: ReadonlyArray, - private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean, - private enableI18nLegacyMessageIdFormat: boolean, private usePoisonedData: boolean, - private i18nNormalizeLineEndingsInICUs: boolean, private moduleResolver: ModuleResolver, - private cycleAnalyzer: CycleAnalyzer, private cycleHandlingStrategy: CycleHandlingStrategy, - private refEmitter: ReferenceEmitter, private depTracker: DependencyTracker|null, + private strictCtorDeps: boolean, private resourceLoader: ResourceLoader, + private rootDirs: ReadonlyArray, private defaultPreserveWhitespaces: boolean, + private i18nUseExternalIds: boolean, private enableI18nLegacyMessageIdFormat: boolean, + private usePoisonedData: boolean, private i18nNormalizeLineEndingsInICUs: boolean, + private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer, + private cycleHandlingStrategy: CycleHandlingStrategy, private refEmitter: ReferenceEmitter, + private depTracker: DependencyTracker|null, private injectableRegistry: InjectableClassRegistry, private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, private annotateForClosureCompiler: boolean, private perf: PerfRecorder, @@ -494,7 +495,9 @@ export class ComponentDecoratorHandler implements }); this.resourceRegistry.registerResources(analysis.resources, node); - this.injectableRegistry.registerInjectable(node); + this.injectableRegistry.registerInjectable(node, { + ctorDeps: analysis.meta.deps, + }); } index( @@ -848,7 +851,8 @@ export class ComponentDecoratorHandler implements } const directiveDiagnostics = getDirectiveDiagnostics( - node, this.metaReader, this.evaluator, this.reflector, this.scopeRegistry, 'Component'); + node, this.injectableRegistry, this.evaluator, this.reflector, this.scopeRegistry, + this.strictCtorDeps, 'Component'); if (directiveDiagnostics !== null) { diagnostics.push(...directiveDiagnostics); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts index 081423b4bf18d6..1b4b9244b0007a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts @@ -14,13 +14,13 @@ import {ErrorCode, FatalDiagnosticError} from '../../../diagnostics'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {ModuleResolver, Reference, ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, HostDirectivesResolver, InjectableClassRegistry, LocalMetadataRegistry, ResourceRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, ResourceRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver, TypeCheckScopeRegistry} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; -import {ResourceLoader, ResourceLoaderContext} from '../../common'; +import {InjectableClassRegistry, ResourceLoader, ResourceLoaderContext} from '../../common'; import {ComponentDecoratorHandler} from '../src/handler'; export class StubResourceLoader implements ResourceLoader { @@ -55,7 +55,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil const scopeRegistry = new LocalModuleScopeRegistry( metaRegistry, metaReader, dtsResolver, new ReferenceEmitter([]), null); const refEmitter = new ReferenceEmitter([]); - const injectableRegistry = new InjectableClassRegistry(reflectionHost); + const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); const resourceRegistry = new ResourceRegistry(); const hostDirectivesResolver = new HostDirectivesResolver(metaReader); const typeCheckScopeRegistry = @@ -73,6 +73,7 @@ function setup(program: ts.Program, options: ts.CompilerOptions, host: ts.Compil typeCheckScopeRegistry, resourceRegistry, /* isCore */ false, + /* strictCtorDeps */ false, resourceLoader, /* rootDirs */['/'], /* defaultPreserveWhitespaces */ false, diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts index e7498d96bbc245..6b7806b7cccf53 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/handler.ts @@ -11,13 +11,13 @@ import ts from 'typescript'; import {Reference, ReferenceEmitter} from '../../../imports'; import {extractSemanticTypeParameters, SemanticDepGraphUpdater} from '../../../incremental/semantic_graph'; -import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, HostDirectiveMeta, InjectableClassRegistry, MatchSource, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; +import {ClassPropertyMapping, DirectiveTypeCheckMeta, extractDirectiveTypeCheckMeta, HostDirectiveMeta, MatchSource, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, ReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry} from '../../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../../transform'; -import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, toFactoryMetadata, validateHostDirectives} from '../../common'; +import {compileDeclareFactory, compileNgFactoryDefField, compileResults, extractClassMetadata, findAngularDecorator, getDirectiveDiagnostics, getProviderDiagnostics, getUndecoratedClassWithAngularFeaturesDiagnostic, InjectableClassRegistry, isAngularDecorator, readBaseClass, resolveProvidersRequiringFactory, toFactoryMetadata, validateHostDirectives} from '../../common'; import {extractDirectiveMetadata} from './shared'; import {DirectiveSymbol} from './symbol'; @@ -53,6 +53,7 @@ export class DirectiveDecoratorHandler implements private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry, private metaReader: MetadataReader, private injectableRegistry: InjectableClassRegistry, private refEmitter: ReferenceEmitter, private isCore: boolean, + private strictCtorDeps: boolean, private semanticDepGraphUpdater: SemanticDepGraphUpdater|null, private annotateForClosureCompiler: boolean, private compileUndecoratedClassesWithAngularFeatures: boolean, private perf: PerfRecorder) {} @@ -161,7 +162,9 @@ export class DirectiveDecoratorHandler implements decorator: analysis.decorator, }); - this.injectableRegistry.registerInjectable(node); + this.injectableRegistry.registerInjectable(node, { + ctorDeps: analysis.meta.deps, + }); } resolve(node: ClassDeclaration, analysis: DirectiveHandlerData, symbol: DirectiveSymbol): @@ -180,7 +183,8 @@ export class DirectiveDecoratorHandler implements } const directiveDiagnostics = getDirectiveDiagnostics( - node, this.metaReader, this.evaluator, this.reflector, this.scopeRegistry, 'Directive'); + node, this.injectableRegistry, this.evaluator, this.reflector, this.scopeRegistry, + this.strictCtorDeps, 'Directive'); if (directiveDiagnostics !== null) { diagnostics.push(...directiveDiagnostics); } diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel index c4bff20194e985..a1a27f243d16e3 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/test/BUILD.bazel @@ -11,6 +11,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/annotations/directive", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts index 0da42f349ceaa0..8d4b6877cbaf34 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/test/directive_spec.ts @@ -11,12 +11,13 @@ import ts from 'typescript'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {ClassDeclaration, isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; +import {InjectableClassRegistry} from '../../common'; import {DirectiveDecoratorHandler} from '../index'; runInEachFileSystem(() => { @@ -168,11 +169,12 @@ runInEachFileSystem(() => { const scopeRegistry = new LocalModuleScopeRegistry( metaReader, new CompoundMetadataReader([metaReader, dtsReader]), new MetadataDtsModuleScopeResolver(dtsReader, null), refEmitter, null); - const injectableRegistry = new InjectableClassRegistry(reflectionHost); + const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); const handler = new DirectiveDecoratorHandler( reflectionHost, evaluator, scopeRegistry, scopeRegistry, metaReader, injectableRegistry, refEmitter, /*isCore*/ false, + /*strictCtorDeps*/ false, /*semanticDepGraphUpdater*/ null, /*annotateForClosureCompiler*/ false, /*detectUndecoratedClassesWithAngularFeatures*/ false, NOOP_PERF_RECORDER); diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts index a41961566945ec..bff2b5fb7de180 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/ng_module/src/handler.ts @@ -12,7 +12,7 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../../diagnostics'; import {assertSuccessfulReferenceEmit, Reference, ReferenceEmitter} from '../../../imports'; import {isArrayEqual, isReferenceEqual, isSymbolEqual, SemanticReference, SemanticSymbol} from '../../../incremental/semantic_graph'; -import {InjectableClassRegistry, MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; +import {MetadataReader, MetadataRegistry, MetaKind} from '../../../metadata'; import {PartialEvaluator, ResolvedValue, SyntheticValue} from '../../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../../perf'; import {ClassDeclaration, DeclarationNode, Decorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection'; @@ -21,7 +21,7 @@ import {getDiagnosticNode} from '../../../scope/src/util'; import {FactoryTracker} from '../../../shims/api'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../../transform'; import {getSourceFile} from '../../../util/src/typescript'; -import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, createValueHasWrongTypeError, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getProviderDiagnostics, getValidConstructorDependencies, isExpressionForwardReference, ReferencesRegistry, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common'; +import {combineResolvers, compileDeclareFactory, compileNgFactoryDefField, createValueHasWrongTypeError, extractClassMetadata, extractSchemas, findAngularDecorator, forwardRefResolver, getProviderDiagnostics, getValidConstructorDependencies, InjectableClassRegistry, isExpressionForwardReference, ReferencesRegistry, resolveProvidersRequiringFactory, toR3Reference, unwrapExpression, wrapFunctionExpressionsInParens, wrapTypeReference} from '../../common'; import {createModuleWithProvidersResolver, isResolvedModuleWithProviders} from './module_with_providers'; @@ -475,7 +475,9 @@ export class NgModuleDecoratorHandler implements }); } - this.injectableRegistry.registerInjectable(node); + this.injectableRegistry.registerInjectable(node, { + ctorDeps: analysis.fac.deps, + }); } resolve(node: ClassDeclaration, analysis: Readonly): diff --git a/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts index 0af3c2f3fd1ef7..19dbd885837506 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/ng_module/test/ng_module_spec.ts @@ -12,13 +12,13 @@ import ts from 'typescript'; import {absoluteFrom} from '../../../file_system'; import {runInEachFileSystem} from '../../../file_system/testing'; import {LocalIdentifierStrategy, ReferenceEmitter} from '../../../imports'; -import {CompoundMetadataReader, DtsMetadataReader, InjectableClassRegistry, LocalMetadataRegistry} from '../../../metadata'; +import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../../metadata'; import {PartialEvaluator} from '../../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../scope'; import {getDeclaration, makeProgram} from '../../../testing'; -import {NoopReferencesRegistry} from '../../common'; +import {InjectableClassRegistry, NoopReferencesRegistry} from '../../common'; import {NgModuleDecoratorHandler} from '../src/handler'; runInEachFileSystem(() => { @@ -67,7 +67,7 @@ runInEachFileSystem(() => { metaRegistry, metaReader, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]), null); const refEmitter = new ReferenceEmitter([new LocalIdentifierStrategy()]); - const injectableRegistry = new InjectableClassRegistry(reflectionHost); + const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); const handler = new NgModuleDecoratorHandler( reflectionHost, evaluator, metaReader, metaRegistry, scopeRegistry, referencesRegistry, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts index 0d63dcbdb3766d..722ddde37687d4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts @@ -9,12 +9,13 @@ import {compileClassMetadata, CompileClassMetadataFn, compileDeclareClassMetadata, compileDeclareInjectableFromMetadata, compileInjectable, createMayBeForwardRefExpression, FactoryTarget, ForwardRefHandling, LiteralExpr, MaybeForwardRefExpression, R3ClassMetadata, R3CompiledExpression, R3DependencyMetadata, R3InjectableMetadata, WrappedNodeExpr} from '@angular/compiler'; import ts from 'typescript'; +import {InjectableClassRegistry, isAbstractClassDeclaration} from '../../annotations/common'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; -import {InjectableClassRegistry} from '../../metadata'; +import {PartialEvaluator} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; -import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; -import {compileDeclareFactory, CompileFactoryFn, compileNgFactoryDefField, extractClassMetadata, findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, toFactoryMetadata, tryUnwrapForwardRef, unwrapConstructorDependencies, validateConstructorDependencies, wrapTypeReference} from '../common'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; +import {checkInheritanceOfInjectable, compileDeclareFactory, CompileFactoryFn, compileNgFactoryDefField, extractClassMetadata, findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, toFactoryMetadata, tryUnwrapForwardRef, unwrapConstructorDependencies, validateConstructorDependencies, wrapTypeReference,} from '../common'; export interface InjectableHandlerData { meta: R3InjectableMetadata; @@ -29,7 +30,8 @@ export interface InjectableHandlerData { export class InjectableDecoratorHandler implements DecoratorHandler { constructor( - private reflector: ReflectionHost, private isCore: boolean, private strictCtorDeps: boolean, + private reflector: ReflectionHost, private evaluator: PartialEvaluator, + private isCore: boolean, private strictCtorDeps: boolean, private injectableRegistry: InjectableClassRegistry, private perf: PerfRecorder, /** * What to do if the injectable already contains a ɵprov property. @@ -83,8 +85,26 @@ export class InjectableDecoratorHandler implements return null; } - register(node: ClassDeclaration): void { - this.injectableRegistry.registerInjectable(node); + register(node: ClassDeclaration, analysis: InjectableHandlerData): void { + this.injectableRegistry.registerInjectable(node, { + ctorDeps: analysis.ctorDeps, + }); + } + + resolve(node: ClassDeclaration, analysis: Readonly, symbol: null): + ResolveResult { + if (requiresValidCtor(analysis.meta)) { + const diagnostic = checkInheritanceOfInjectable( + node, this.injectableRegistry, this.reflector, this.evaluator, this.strictCtorDeps, + 'Injectable'); + if (diagnostic !== null) { + return { + diagnostics: [diagnostic], + }; + } + } + + return {}; } compileFull(node: ClassDeclaration, analysis: Readonly): CompileResult[] { @@ -244,7 +264,7 @@ function extractInjectableCtorDeps( // To deal with this, @Injectable() without an argument is more lenient, and if the // constructor signature does not work for DI then a factory definition (ɵfac) that throws is // generated. - if (strictCtorDeps) { + if (strictCtorDeps && !isAbstractClassDeclaration(clazz)) { ctorDeps = getValidConstructorDependencies(clazz, reflector, isCore); } else { ctorDeps = @@ -255,9 +275,9 @@ function extractInjectableCtorDeps( } else if (decorator.args.length === 1) { const rawCtorDeps = getConstructorDependencies(clazz, reflector, isCore); - if (strictCtorDeps && meta.useValue === undefined && meta.useExisting === undefined && - meta.useClass === undefined && meta.useFactory === undefined) { - // Since use* was not provided, validate the deps according to strictCtorDeps. + if (strictCtorDeps && !isAbstractClassDeclaration(clazz) && requiresValidCtor(meta)) { + // Since use* was not provided for a concrete class, validate the deps according to + // strictCtorDeps. ctorDeps = validateConstructorDependencies(clazz, rawCtorDeps); } else { ctorDeps = unwrapConstructorDependencies(rawCtorDeps); @@ -267,6 +287,11 @@ function extractInjectableCtorDeps( return ctorDeps; } +function requiresValidCtor(meta: R3InjectableMetadata): boolean { + return meta.useValue === undefined && meta.useExisting === undefined && + meta.useClass === undefined && meta.useFactory === undefined; +} + function getDep(dep: ts.Expression, reflector: ReflectionHost): R3DependencyMetadata { const meta: R3DependencyMetadata = { token: new WrappedNodeExpr(dep), diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts index 5181dc7cb0dd48..c66fece5e5a7a9 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts @@ -12,13 +12,13 @@ import ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; import {Reference} from '../../imports'; import {SemanticSymbol} from '../../incremental/semantic_graph'; -import {InjectableClassRegistry, MetadataRegistry, MetaKind} from '../../metadata'; +import {MetadataRegistry, MetaKind} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {PerfEvent, PerfRecorder} from '../../perf'; import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry} from '../../scope'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; -import {compileDeclareFactory, compileNgFactoryDefField, compileResults, createValueHasWrongTypeError, extractClassMetadata, findAngularDecorator, getValidConstructorDependencies, makeDuplicateDeclarationError, toFactoryMetadata, unwrapExpression, wrapTypeReference} from '../common'; +import {compileDeclareFactory, compileNgFactoryDefField, compileResults, createValueHasWrongTypeError, extractClassMetadata, findAngularDecorator, getValidConstructorDependencies, InjectableClassRegistry, makeDuplicateDeclarationError, toFactoryMetadata, unwrapExpression, wrapTypeReference,} from '../common'; export interface PipeHandlerData { meta: R3PipeMetadata; @@ -164,7 +164,9 @@ export class PipeDecoratorHandler implements decorator: analysis.decorator, }); - this.injectableRegistry.registerInjectable(node); + this.injectableRegistry.registerInjectable(node, { + ctorDeps: analysis.meta.deps, + }); } resolve(node: ClassDeclaration): ResolveResult { diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel index bfe1897493bba8..2ee2f98ddc49ce 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/annotations/test/BUILD.bazel @@ -12,6 +12,7 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/annotations", + "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/cycles", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts index 901463b7e4a710..6ecd3b24ce7bae 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/injectable_spec.ts @@ -5,10 +5,11 @@ * 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 {InjectableClassRegistry} from '../../annotations/common'; import {ErrorCode, FatalDiagnosticError, ngErrorCode} from '../../diagnostics'; import {absoluteFrom} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; -import {InjectableClassRegistry} from '../../metadata'; +import {PartialEvaluator} from '../../partial_evaluator'; import {NOOP_PERF_RECORDER} from '../../perf'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; @@ -67,9 +68,10 @@ function setupHandler(errorOnDuplicateProv: boolean) { ]); const checker = program.getTypeChecker(); const reflectionHost = new TypeScriptReflectionHost(checker); - const injectableRegistry = new InjectableClassRegistry(reflectionHost); + const injectableRegistry = new InjectableClassRegistry(reflectionHost, /* isCore */ false); + const evaluator = new PartialEvaluator(reflectionHost, checker, null); const handler = new InjectableDecoratorHandler( - reflectionHost, /* isCore */ false, + reflectionHost, evaluator, /* isCore */ false, /* strictCtorDeps */ false, injectableRegistry, NOOP_PERF_RECORDER, errorOnDuplicateProv); const TestClass = getDeclaration(program, ENTRY_FILE, 'TestClass', isNamedClassDeclaration); const ɵprov = reflectionHost.getMembersOfClass(TestClass).find(member => member.name === 'ɵprov'); diff --git a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel index 5bc84e3f1c186c..7974be6e2fc4fa 100644 --- a/packages/compiler-cli/src/ngtsc/core/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/core/BUILD.bazel @@ -13,6 +13,7 @@ ts_library( "//packages:types", "//packages/compiler", "//packages/compiler-cli/src/ngtsc/annotations", + "//packages/compiler-cli/src/ngtsc/annotations/common", "//packages/compiler-cli/src/ngtsc/cycles", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/entry_point", diff --git a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts index 3d224e575cadaa..21c2572da52659 100644 --- a/packages/compiler-cli/src/ngtsc/core/src/compiler.ts +++ b/packages/compiler-cli/src/ngtsc/core/src/compiler.ts @@ -9,6 +9,7 @@ import ts from 'typescript'; import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ReferencesRegistry} from '../../annotations'; +import {InjectableClassRegistry} from '../../annotations/common'; import {CycleAnalyzer, CycleHandlingStrategy, ImportGraph} from '../../cycles'; import {COMPILER_ERRORS_WITH_GUIDES, ERROR_DETAILS_PAGE_BASE_URL, ErrorCode, ngErrorCode} from '../../diagnostics'; import {checkForPrivateExports, ReferenceGraph} from '../../entry_point'; @@ -17,7 +18,7 @@ import {AbsoluteModuleStrategy, AliasingHost, AliasStrategy, DefaultImportTracke import {IncrementalBuildStrategy, IncrementalCompilation, IncrementalState} from '../../incremental'; import {SemanticSymbol} from '../../incremental/semantic_graph'; import {generateAnalysis, IndexedComponent, IndexingContext} from '../../indexer'; -import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, HostDirectivesResolver, InjectableClassRegistry, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata'; +import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, MetadataReader, PipeMeta, ResourceRegistry} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {ActivePerfRecorder, DelegatingPerfRecorder, PerfCheckpoint, PerfEvent, PerfPhase} from '../../perf'; import {FileUpdate, ProgramDriver, UpdateMode} from '../../program_driver'; @@ -962,6 +963,8 @@ export class NgCompiler { aliasingHost = new UnifiedModulesAliasingHost(this.adapter.unifiedModulesHost); } + const isCore = isAngularCorePackage(this.inputProgram); + const evaluator = new PartialEvaluator(reflector, checker, this.incrementalCompilation.depGraph); const dtsReader = new DtsMetadataReader(checker, reflector); @@ -977,7 +980,7 @@ export class NgCompiler { new CompoundComponentScopeReader([ngModuleScopeRegistry, standaloneScopeReader]); const semanticDepGraphUpdater = this.incrementalCompilation.semanticDepGraphUpdater; const metaRegistry = new CompoundMetadataRegistry([localMetaRegistry, ngModuleScopeRegistry]); - const injectableRegistry = new InjectableClassRegistry(reflector); + const injectableRegistry = new InjectableClassRegistry(reflector, isCore); const hostDirectivesResolver = new HostDirectivesResolver(metaReader); const typeCheckScopeRegistry = @@ -998,8 +1001,6 @@ export class NgCompiler { const dtsTransforms = new DtsTransformRegistry(); - const isCore = isAngularCorePackage(this.inputProgram); - const resourceRegistry = new ResourceRegistry(); // Note: If this compilation builds `@angular/core`, we always build in full compilation @@ -1016,11 +1017,13 @@ export class NgCompiler { CycleHandlingStrategy.UseRemoteScoping : CycleHandlingStrategy.Error; + const strictCtorDeps = this.options.strictInjectionParameters || false; + // Set up the IvyCompilation, which manages state for the Ivy transformer. const handlers: DecoratorHandler[] = [ new ComponentDecoratorHandler( reflector, evaluator, metaRegistry, metaReader, scopeReader, depScopeReader, - ngModuleScopeRegistry, typeCheckScopeRegistry, resourceRegistry, isCore, + ngModuleScopeRegistry, typeCheckScopeRegistry, resourceRegistry, isCore, strictCtorDeps, this.resourceManager, this.adapter.rootDirs, this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false, this.options.enableI18nLegacyMessageIdFormat !== false, this.usePoisonedData, @@ -1034,7 +1037,7 @@ export class NgCompiler { // clang-format off new DirectiveDecoratorHandler( reflector, evaluator, metaRegistry, ngModuleScopeRegistry, metaReader, - injectableRegistry, refEmitter, isCore, semanticDepGraphUpdater, + injectableRegistry, refEmitter, isCore, strictCtorDeps, semanticDepGraphUpdater, this.closureCompilerEnabled, /** compileUndecoratedClassesWithAngularFeatures */ false, this.delegatingPerfRecorder, ) as Readonly>, @@ -1045,7 +1048,7 @@ export class NgCompiler { reflector, evaluator, metaRegistry, ngModuleScopeRegistry, injectableRegistry, isCore, this.delegatingPerfRecorder), new InjectableDecoratorHandler( - reflector, isCore, this.options.strictInjectionParameters || false, injectableRegistry, + reflector, evaluator, isCore, strictCtorDeps, injectableRegistry, this.delegatingPerfRecorder), new NgModuleDecoratorHandler( reflector, evaluator, metaReader, metaRegistry, ngModuleScopeRegistry, referencesRegistry, diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 1c3a9cdaa5a99f..545c3a861d7b40 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -86,6 +86,12 @@ export enum ErrorCode { */ HOST_DIRECTIVE_COMPONENT = 2015, + /** + * Raised when a type with Angular decorator inherits its constructor from a base class + * which has a constructor that is incompatible with Angular DI. + */ + INJECTABLE_INHERITS_INVALID_CONSTRUCTOR = 2016, + SYMBOL_NOT_EXPORTED = 3001, /** * Raised when a relationship between directives and/or pipes would cause a cyclic import to be diff --git a/packages/compiler-cli/src/ngtsc/metadata/index.ts b/packages/compiler-cli/src/ngtsc/metadata/index.ts index c1b367543f9652..44d1612592733d 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/index.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/index.ts @@ -9,8 +9,8 @@ export * from './src/api'; export {DtsMetadataReader} from './src/dts'; export {flattenInheritedDirectiveMetadata} from './src/inheritance'; -export {CompoundMetadataRegistry, LocalMetadataRegistry, InjectableClassRegistry} from './src/registry'; +export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry'; export {ResourceRegistry, Resource, ComponentResources, isExternalResource, ExternalResource} from './src/resource_registry'; -export {extractDirectiveTypeCheckMeta, CompoundMetadataReader} from './src/util'; +export {extractDirectiveTypeCheckMeta, hasInjectableFields, CompoundMetadataReader} from './src/util'; export {BindingPropertyName, ClassPropertyMapping, ClassPropertyName, InputOrOutput} from './src/property_mapping'; export {HostDirectivesResolver} from './src/host_directives_resolver'; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts index c82e7adab1167c..716dc5dc85bf7f 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts @@ -7,10 +7,9 @@ */ import {Reference} from '../../imports'; -import {ClassDeclaration, ReflectionHost} from '../../reflection'; +import {ClassDeclaration} 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 @@ -67,24 +66,3 @@ 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(); - - 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); - } -} diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 72e2c7dc5cfaa8..ec8b0643a2481b 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2732,6 +2732,234 @@ function allTests(os: string) { expect(jsContents) .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); }); + + it('should not give a compile-time error if an invalid @Injectable without providedIn is an abstract class', + () => { + env.tsconfig({strictInjectionParameters: true}); + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + @Injectable() + export abstract class Test { + constructor(private notInjectable: string) {} + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents) + .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); + }); + + it('should not give a compile-time error if an invalid @Injectable with providedIn is an abstract class', + () => { + env.tsconfig({strictInjectionParameters: true}); + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + @Injectable({ + providedIn: 'root', + }) + export abstract class Test { + constructor(private notInjectable: string) {} + } + `); + + env.driveMain(); + const jsContents = env.getContents('test.js'); + expect(jsContents) + .toMatch(/function Test_Factory\(t\) { i0\.ɵɵinvalidFactory\(\)/ms); + }); + + it('should give a compile-time error when a derived Directive inherits an invalid constructor', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + @Directive() + export abstract class ParentDir { + constructor(private notInjectable: string) {} + } + + @Directive() + export abstract class AbstractMiddleDir extends ParentDir {} + + @Directive() + export class ConcreteMiddleDir extends AbstractMiddleDir {} + + @Directive() + export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} + + @Directive() + export class ConcreteDirWithCtor extends ConcreteMiddleDir { + constructor() { + super('correct'); + } + } + + @Directive() + export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + expect(diags[0].code) + .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); + expect(diags[0].messageText) + .toEqual( + `The directive ConcreteMiddleDir inherits its constructor from ParentDir, but the latter has ` + + `a constructor parameter that is not compatible with dependency injection. Either add an explicit ` + + `constructor to ConcreteMiddleDir or change ParentDir's constructor to use parameters that are ` + + `valid for DI.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('ConcreteMiddleDir'); + + expect(diags[1].code) + .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); + expect(diags[1].messageText) + .toEqual( + `The directive ConcreteDirWithoutCtor inherits its constructor from ParentDir, but the latter ` + + `has a constructor parameter that is not compatible with dependency injection. Either add an ` + + `explicit constructor to ConcreteDirWithoutCtor or change ParentDir's constructor to use ` + + `parameters that are valid for DI.`); + expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteDirWithoutCtor'); + }); + + it('should give a compile-time error when a derived Injectable inherits an invalid constructor', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + @Injectable() + export abstract class ParentService { + constructor(private notInjectable: string) {} + } + + @Injectable() + export abstract class AbstractMiddleService extends ParentService {} + + @Injectable() + export class ConcreteMiddleService extends AbstractMiddleService {} + + @Injectable() + export class ConcreteServiceWithoutCtor extends ConcreteMiddleService {} + + @Injectable() + export class ConcreteServiceWithCtor extends ConcreteMiddleService { + constructor() { + super('correct'); + } + } + + @Injectable() + export class ConcreteDerivedServiceWithoutCtor extends ConcreteServiceWithCtor {} + + @Injectable({ providedIn: 'root' }) + export class ProvidedInRootService extends ParentService {} + + @Injectable({ providedIn: 'root', useFactory: () => null }) + export class UseFactoryService extends ParentService {} + + @Injectable({ providedIn: 'root', useValue: null }) + export class UseValueService extends ParentService {} + + @Injectable({ providedIn: 'root', useClass: ConcreteServiceWithCtor }) + export class UseClassService extends ParentService {} + + @Injectable({ providedIn: 'root', useExisting: ConcreteServiceWithCtor }) + export class UseExistingService extends ParentService {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].code) + .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); + expect(diags[0].messageText) + .toEqual( + `The injectable ConcreteMiddleService inherits its constructor from ParentService, but the ` + + `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + + `an explicit constructor to ConcreteMiddleService or change ParentService's constructor to use ` + + `parameters that are valid for DI.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('ConcreteMiddleService'); + + expect(diags[1].code) + .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); + expect(diags[1].messageText) + .toEqual( + `The injectable ConcreteServiceWithoutCtor inherits its constructor from ParentService, but the ` + + `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + + `an explicit constructor to ConcreteServiceWithoutCtor or change ParentService's constructor to ` + + `use parameters that are valid for DI.`); + expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteServiceWithoutCtor'); + + expect(diags[2].code) + .toBe(ngErrorCode(ErrorCode.INJECTABLE_INHERITS_INVALID_CONSTRUCTOR)); + expect(diags[2].messageText) + .toEqual( + `The injectable ProvidedInRootService inherits its constructor from ParentService, but the ` + + `latter has a constructor parameter that is not compatible with dependency injection. Either add ` + + `an explicit constructor to ProvidedInRootService or change ParentService's constructor to use ` + + `parameters that are valid for DI.`); + expect(getDiagnosticSourceCode(diags[2])).toBe('ProvidedInRootService'); + }); + + it('should give a compile-time error when a derived Directive inherits from a non-decorated class', () => { + env.tsconfig({strictInjectionParameters: true}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + export abstract class ParentClass { + constructor(private notInjectable: string) {} + } + + @Directive() + export abstract class AbstractMiddleDir extends ParentClass {} + + @Directive() + export class ConcreteMiddleDir extends AbstractMiddleDir {} + + @Directive() + export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} + + @Directive() + export class ConcreteDirWithCtor extends ConcreteMiddleDir { + constructor() { + super('correct'); + } + } + + @Directive() + export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[0].messageText) + .toEqual( + `The directive AbstractMiddleDir inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + + `or add an explicit constructor to AbstractMiddleDir.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('AbstractMiddleDir'); + + expect(diags[1].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[1].messageText) + .toEqual( + `The directive ConcreteMiddleDir inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, or ` + + `add an explicit constructor to ConcreteMiddleDir.`); + expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteMiddleDir'); + + expect(diags[2].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[2].messageText) + .toEqual( + `The directive ConcreteDirWithoutCtor inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + + `or add an explicit constructor to ConcreteDirWithoutCtor.`); + expect(getDiagnosticSourceCode(diags[2])).toBe('ConcreteDirWithoutCtor'); + }); }); describe('with strictInjectionParameters = false', () => { @@ -2768,6 +2996,148 @@ function allTests(os: string) { expect(jsContents) .toContain('Test.ɵfac = function Test_Factory(t) { i0.ɵɵinvalidFactory()'); }); + + it('should compile when a derived Directive inherits an invalid constructor', () => { + env.tsconfig({strictInjectionParameters: false}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + @Directive() + export abstract class ParentDir { + constructor(private notInjectable: string) {} + } + + @Directive() + export abstract class AbstractMiddleDir extends ParentDir {} + + @Directive() + export class ConcreteMiddleDir extends AbstractMiddleDir {} + + @Directive() + export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} + + @Directive() + export class ConcreteDirWithCtor extends ConcreteMiddleDir { + constructor() { + super('correct'); + } + } + + @Directive() + export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} + `); + + env.driveMain(); + }); + + it('should compile when a derived Injectable inherits an invalid constructor', () => { + env.tsconfig({strictInjectionParameters: false}); + env.write('test.ts', ` + import {Injectable} from '@angular/core'; + + @Injectable() + export abstract class ParentService { + constructor(private notInjectable: string) {} + } + + @Injectable() + export abstract class AbstractMiddleService extends ParentService {} + + @Injectable() + export class ConcreteMiddleService extends AbstractMiddleService {} + + @Injectable() + export class ConcreteServiceWithoutCtor extends ConcreteMiddleService {} + + @Injectable() + export class ConcreteServiceWithCtor extends ConcreteMiddleService { + constructor() { + super('correct'); + } + } + + @Injectable() + export class ConcreteDerivedServiceWithoutCtor extends ConcreteServiceWithCtor {} + + @Injectable({ providedIn: 'root' }) + export class ProvidedInRootService extends ParentService {} + + @Injectable({ providedIn: 'root', useFactory: () => null }) + export class UseFactoryService extends ParentService {} + + @Injectable({ providedIn: 'root', useValue: null }) + export class UseValueService extends ParentService {} + + @Injectable({ providedIn: 'root', useClass: ConcreteServiceWithCtor }) + export class UseClassService extends ParentService {} + + @Injectable({ providedIn: 'root', useExisting: ConcreteServiceWithCtor }) + export class UseExistingService extends ParentService {} + `); + + env.driveMain(); + }); + + it('should give a compile-time error when a derived Directive inherits from a non-decorated class', () => { + // Errors for undecorated base classes should always be reported, even under + // `strictInjectionParameters`. + env.tsconfig({strictInjectionParameters: false}); + env.write('test.ts', ` + import {Directive} from '@angular/core'; + + export abstract class ParentClass { + constructor(private notInjectable: string) {} + } + + @Directive() + export abstract class AbstractMiddleDir extends ParentClass {} + + @Directive() + export class ConcreteMiddleDir extends AbstractMiddleDir {} + + @Directive() + export class ConcreteDirWithoutCtor extends ConcreteMiddleDir {} + + @Directive() + export class ConcreteDirWithCtor extends ConcreteMiddleDir { + constructor() { + super('correct'); + } + } + + @Directive() + export class ConcreteDerivedDirWithoutCtor extends ConcreteDirWithCtor {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(3); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[0].messageText) + .toEqual( + `The directive AbstractMiddleDir inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + + `or add an explicit constructor to AbstractMiddleDir.`); + expect(getDiagnosticSourceCode(diags[0])).toBe('AbstractMiddleDir'); + + expect(diags[1].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[1].messageText) + .toEqual( + `The directive ConcreteMiddleDir inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + + `or add an explicit constructor to ConcreteMiddleDir.`); + expect(getDiagnosticSourceCode(diags[1])).toBe('ConcreteMiddleDir'); + + expect(diags[2].code).toBe(ngErrorCode(ErrorCode.DIRECTIVE_INHERITS_UNDECORATED_CTOR)); + expect(diags[2].messageText) + .toEqual( + `The directive ConcreteDirWithoutCtor inherits its constructor from ParentClass, but the latter ` + + `does not have an Angular decorator of its own. Dependency injection will not be able to resolve ` + + `the parameters of ParentClass's constructor. Either add a @Directive decorator to ParentClass, ` + + `or add an explicit constructor to ConcreteDirWithoutCtor.`); + expect(getDiagnosticSourceCode(diags[2])).toBe('ConcreteDirWithoutCtor'); + }); }); });