From 5ce641def77b4f0ea36561b36e119a2ce089c61a Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 22 Jun 2019 22:21:23 +0200 Subject: [PATCH] fix(ivy): template type checking should consider non-decorated base classes Fixes #30080 --- .../ngcc/src/analysis/decoration_analyzer.ts | 3 +- .../src/ngtsc/annotations/src/base_def.ts | 13 ++++++- .../src/ngtsc/incremental/src/state.ts | 12 +++++- .../src/ngtsc/metadata/src/api.ts | 31 +++++++++++++++ .../src/ngtsc/metadata/src/dts.ts | 11 +++++- .../src/ngtsc/metadata/src/inheritance.ts | 7 ++-- .../src/ngtsc/metadata/src/registry.ts | 13 ++++++- .../src/ngtsc/metadata/src/util.ts | 13 ++++++- packages/compiler-cli/src/ngtsc/program.ts | 2 +- .../compiler-cli/src/ngtsc/scope/src/local.ts | 4 +- .../test/ngtsc/template_typecheck_spec.ts | 39 +++++++++++++++++++ 11 files changed, 136 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts index d483365ba7600..24e712cf5800a 100644 --- a/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts @@ -90,7 +90,8 @@ export class DecorationAnalyzer { importGraph = new ImportGraph(this.moduleResolver); cycleAnalyzer = new CycleAnalyzer(this.importGraph); handlers: DecoratorHandler[] = [ - new BaseDefDecoratorHandler(this.reflectionHost, this.evaluator, this.isCore), + new BaseDefDecoratorHandler( + this.reflectionHost, this.evaluator, this.metaRegistry, this.isCore), new ComponentDecoratorHandler( this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader, this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts index ce23c170a8488..7cd0ec572aea4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts @@ -8,12 +8,14 @@ import {ConstantPool, R3BaseRefMetaData, compileBaseDefFromMetadata, makeBindingParser} from '@angular/compiler'; +import {Reference} from '../../imports'; +import {MetadataRegistry} from '../../metadata'; import {PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, ClassMember, Decorator, ReflectionHost} from '../../reflection'; import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; import {extractHostBindings, queriesFromFields} from './directive'; -import {isAngularDecorator} from './util'; +import {isAngularDecorator, readBaseClass} from './util'; function containsNgTopLevelDecorator(decorators: Decorator[] | null, isCore: boolean): boolean { if (!decorators) { @@ -29,7 +31,7 @@ export class BaseDefDecoratorHandler implements DecoratorHandler { constructor( private reflector: ReflectionHost, private evaluator: PartialEvaluator, - private isCore: boolean) {} + private metaRegistry: MetadataRegistry, private isCore: boolean) {} readonly precedence = HandlerPrecedence.WEAK; @@ -145,6 +147,13 @@ export class BaseDefDecoratorHandler implements metadata.host, this.evaluator, this.isCore ? undefined : '@angular/core'); } + this.metaRegistry.registerBaseMetadata({ + ref: new Reference(node), + inputs: analysis.inputs || {}, + outputs: analysis.outputs || {}, + baseClass: readBaseClass(node, this.reflector, this.evaluator), + }); + return {analysis}; } diff --git a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts index 983dde26a59c6..13a521dfe506d 100644 --- a/packages/compiler-cli/src/ngtsc/incremental/src/state.ts +++ b/packages/compiler-cli/src/ngtsc/incremental/src/state.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; -import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; +import {BaseMeta, DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; import {DependencyTracker} from '../../partial_evaluator'; import {ClassDeclaration} from '../../reflection'; import {ResourceDependencyRecorder} from '../../util/src/resource_recorder'; @@ -100,6 +100,15 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta metadata.pipeMeta.set(meta.ref.node, meta); } + getBaseMetadata(ref: Reference): BaseMeta|null { + const metadata = this.metadata.get(ref.node.getSourceFile()) || null; + return metadata && metadata.baseMeta.get(ref.node) || null; + } + registerBaseMetadata(meta: BaseMeta): void { + const metadata = this.ensureMetadata(meta.ref.node.getSourceFile()); + metadata.baseMeta.set(meta.ref.node, meta); + } + recordResourceDependency(file: ts.SourceFile, resourcePath: string): void { const metadata = this.ensureMetadata(file); metadata.resourcePaths.add(resourcePath); @@ -131,4 +140,5 @@ class FileMetadata { directiveMeta = new Map(); ngModuleMeta = new Map(); pipeMeta = new Map(); + baseMeta = new Map(); } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts index c8770b758d57f..a4b4b9831efd9 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts @@ -70,6 +70,35 @@ export interface PipeMeta { name: string; } +/** + * Metadata for an undecorated class that uses Angular decorators + */ +export interface BaseMeta { + ref: Reference; + + /** + * Set of inputs which this directive claims. + * + * Goes from property names to field names. + */ + inputs: {[property: string]: string | [string, string]}; + + /** + * Set of outputs which this directive claims. + * + * Goes from property names to field names. + */ + outputs: {[property: string]: string}; + + /** + * A `Reference` to the base class for the directive, if one was detected. + * + * A value of `'dynamic'` indicates that while the analyzer detected that this directive extends + * another type, it could not statically determine the base class. + */ + baseClass: Reference|'dynamic'|null; +} + /** * Reads metadata for directives, pipes, and modules from a particular source, such as .d.ts files * or a registry. @@ -78,6 +107,7 @@ export interface MetadataReader { getDirectiveMetadata(node: Reference): DirectiveMeta|null; getNgModuleMetadata(node: Reference): NgModuleMeta|null; getPipeMetadata(node: Reference): PipeMeta|null; + getBaseMetadata(node: Reference): BaseMeta|null; } /** @@ -87,4 +117,5 @@ export interface MetadataRegistry { registerDirectiveMetadata(meta: DirectiveMeta): void; registerNgModuleMetadata(meta: NgModuleMeta): void; registerPipeMetadata(meta: PipeMeta): void; + registerBaseMetadata(meta: BaseMeta): void; } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 28347d5460a27..e0123d2f1af34 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {Reference} from '../../imports'; import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration} from '../../reflection'; -import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api'; +import {BaseMeta, DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api'; import {extractDirectiveGuards, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util'; /** @@ -115,6 +115,15 @@ export class DtsMetadataReader implements MetadataReader { const name = type.literal.text; return {ref, name}; } + + /** + * Read pipe metadata from a referenced class in a .d.ts file. + */ + getBaseMetadata(ref: Reference): BaseMeta|null { + // TODO(joost): Introduce BaseDefWithMeta so that the declaration + // has inputs and outputs that can be resolved here + return null; + } } function readBaseClass(clazz: ClassDeclaration, checker: ts.TypeChecker, reflector: ReflectionHost): diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts index bb9e9bc7b0af3..305f75e0760d1 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts @@ -7,7 +7,7 @@ */ import {Reference} from '../../imports'; -import {DirectiveMeta, MetadataReader} from '../../metadata'; +import {BaseMeta, DirectiveMeta, MetadataReader} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; /** @@ -29,11 +29,12 @@ export function flattenInheritedDirectiveMetadata( let outputs: {[key: string]: string} = {}; let isDynamic = false; - const addMetadata = (meta: DirectiveMeta): void => { + const addMetadata = (meta: DirectiveMeta | BaseMeta): void => { if (meta.baseClass === 'dynamic') { isDynamic = true; } else if (meta.baseClass !== null) { - const baseMeta = reader.getDirectiveMetadata(meta.baseClass); + const baseMeta = + reader.getDirectiveMetadata(meta.baseClass) || reader.getBaseMetadata(meta.baseClass); if (baseMeta !== null) { addMetadata(baseMeta); } else { diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts index f834218efd2e2..8b0f16361f24c 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/registry.ts @@ -9,7 +9,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration} from '../../reflection'; -import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from './api'; +import {BaseMeta, DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from './api'; /** * A registry of directive, pipe, and module metadata for types defined in the current compilation @@ -19,6 +19,7 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader { private directives = new Map(); private ngModules = new Map(); private pipes = new Map(); + private baseClasses = new Map(); getDirectiveMetadata(ref: Reference): DirectiveMeta|null { return this.directives.has(ref.node) ? this.directives.get(ref.node) ! : null; @@ -29,10 +30,14 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader { getPipeMetadata(ref: Reference): PipeMeta|null { return this.pipes.has(ref.node) ? this.pipes.get(ref.node) ! : null; } + getBaseMetadata(ref: Reference): BaseMeta|null { + return this.baseClasses.has(ref.node) ? this.baseClasses.get(ref.node) ! : null; + } registerDirectiveMetadata(meta: DirectiveMeta): void { this.directives.set(meta.ref.node, meta); } registerNgModuleMetadata(meta: NgModuleMeta): void { this.ngModules.set(meta.ref.node, meta); } registerPipeMetadata(meta: PipeMeta): void { this.pipes.set(meta.ref.node, meta); } + registerBaseMetadata(meta: BaseMeta): void { this.baseClasses.set(meta.ref.node, meta); } } /** @@ -58,4 +63,10 @@ export class CompoundMetadataRegistry implements MetadataRegistry { registry.registerPipeMetadata(meta); } } + + registerBaseMetadata(meta: BaseMeta): void { + for (const registry of this.registries) { + registry.registerBaseMetadata(meta); + } + } } diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index 31a39e9362933..5f55559b58cb4 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -12,7 +12,7 @@ import {Reference} from '../../imports'; import {ClassDeclaration, ClassMember, ClassMemberKind, ReflectionHost, isNamedClassDeclaration, reflectTypeEntityToDeclaration} from '../../reflection'; import {nodeDebugInfo} from '../../util/src/typescript'; -import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api'; +import {BaseMeta, DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api'; export function extractReferencesFromType( checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string | null, @@ -144,6 +144,7 @@ export class CompoundMetadataReader implements MetadataReader { } return null; } + getPipeMetadata(node: Reference>): PipeMeta|null { for (const reader of this.readers) { const meta = reader.getPipeMetadata(node); @@ -153,4 +154,14 @@ export class CompoundMetadataReader implements MetadataReader { } return null; } + + getBaseMetadata(node: Reference>): BaseMeta|null { + for (const reader of this.readers) { + const meta = reader.getBaseMetadata(node); + if (meta !== null) { + return meta; + } + } + return null; + } } diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 967d0e67d3922..df24dc8add5ce 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -496,7 +496,7 @@ export class NgtscProgram implements api.Program { // Set up the IvyCompilation, which manages state for the Ivy transformer. const handlers = [ - new BaseDefDecoratorHandler(this.reflector, evaluator, this.isCore), + new BaseDefDecoratorHandler(this.reflector, evaluator, metaRegistry, this.isCore), new ComponentDecoratorHandler( this.reflector, evaluator, metaRegistry, this.metaReader !, scopeRegistry, this.isCore, this.resourceManager, this.rootDirs, this.options.preserveWhitespaces || false, diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 1d7a95e21f3aa..dae082b85b0f0 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {ErrorCode, makeDiagnostic} from '../../diagnostics'; import {AliasGenerator, Reexport, Reference, ReferenceEmitter} from '../../imports'; -import {DirectiveMeta, LocalMetadataRegistry, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; +import {BaseMeta, DirectiveMeta, LocalMetadataRegistry, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {identifierOfNode, nodeNameForError} from '../../util/src/typescript'; @@ -119,6 +119,8 @@ export class LocalModuleScopeRegistry implements MetadataRegistry { registerPipeMetadata(pipe: PipeMeta): void {} + registerBaseMetadata(base: BaseMeta): void {} + getScopeForComponent(clazz: ClassDeclaration): LocalModuleScope|null { if (!this.declarationToModule.has(clazz)) { return null; diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 2ed017e3e9dd4..a684f92535a5d 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -336,4 +336,43 @@ describe('ngtsc type checking', () => { expect(diags[1].messageText) .toBe(`Type 'number' is not assignable to type 'boolean | undefined'.`); }); + + it('should properly type-check inherited base classes without Angular decorators', () => { + env.write('test.ts', ` + import {Component, Directive, Input, NgModule} from '@angular/core'; + + class BaseDir { + @Input() fromBase!: string; + } + + @Directive({ + selector: '[child]', + }) + class ChildDir extends BaseDir { + @Input() fromChild!: boolean; + } + + @Component({ + selector: 'test', + template: '
', + }) + class TestCmp {} + + @NgModule({ + declarations: [TestCmp, ChildDir], + }) + class Module {} + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(2); + + // Error from the binding to [fromBase]. + expect(diags[0].messageText) + .toBe(`Type 'number' is not assignable to type 'string | undefined'.`); + + // Error from the binding to [fromChild]. + expect(diags[1].messageText) + .toBe(`Type 'number' is not assignable to type 'boolean | undefined'.`); + }); });