From 699f9b5df7569e25ac4da359074a68826cff0551 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. BREAKING CHANGE: Invalid constructors for DI may now report compilation errors When a class inherits its constructor from a base class, the compiler may now report an error when that constructor cannot be used for DI purposes. This may either be because the base class is missing an Angular decorator such as `@Injectable()` or `@Directive()`, or because the constructor contains parameters which do not have an associated token (such as primitive types like `string`). These situations used to behave unexpectedly at runtime, where the class may be constructed without any of its constructor parameters, so this is now reported as an error during compilation. Any new errors that may be reported because of this change can be resolved either by decorating the base class from which the constructor is inherited, or by adding an explicit constructor to the class for which the error is reported. 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 c08d1d2be4033..ca622ccbff6be 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 = 2016, 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 a5a20fce19ff0..7f017c6d96a9c 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 f6f6ba4deea3a..31c9461ece6f8 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 8eec9fbd7abf2..9aa5ee6aef93d 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 0000000000000..0d1a94ab2aabb --- /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 d6bc9af79c108..b6c1982493e8a 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 50058c9692fec..bd41257a64be3 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 081423b4bf18d..1b4b9244b0007 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 e7498d96bbc24..6b7806b7cccf5 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 c4bff20194e98..a1a27f243d16e 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 0da42f349ceaa..8d4b6877cbaf3 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 a41961566945e..bff2b5fb7de18 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 0af3c2f3fd1ef..19dbd88583750 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 0d63dcbdb3766..722ddde37687d 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 5181dc7cb0dd4..c66fece5e5a7a 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 bfe1897493bba..2ee2f98ddc49c 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 901463b7e4a71..6ecd3b24ce7ba 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 5bc84e3f1c186..7974be6e2fc4f 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 6367d9577e504..295116cec7747 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, MetadataReaderWithIndex, PipeMeta, ResourceRegistry} from '../../metadata'; +import {ComponentResources, CompoundMetadataReader, CompoundMetadataRegistry, DirectiveMeta, DtsMetadataReader, HostDirectivesResolver, LocalMetadataRegistry, MetadataReader, MetadataReaderWithIndex, 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 1c3a9cdaa5a99..545c3a861d7b4 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 c1b367543f965..44d1612592733 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 44d2210082741..f0d0abb92c6a6 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, MetadataReaderWithIndex, MetadataRegistry, NgModuleMeta, PipeMeta} from './api'; -import {hasInjectableFields} from './util'; /** * A registry of directive, pipe, and module metadata for types defined in the current compilation @@ -71,24 +70,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 72e2c7dc5cfaa..ec8b0643a2481 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'); + }); }); });