Skip to content

Commit

Permalink
fix(ivy): template type checking should consider non-decorated base c…
Browse files Browse the repository at this point in the history
…lasses

Fixes angular#30080
  • Loading branch information
JoostK committed Jun 22, 2019
1 parent 6f5d910 commit 5ce641d
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 12 deletions.
Expand Up @@ -90,7 +90,8 @@ export class DecorationAnalyzer {
importGraph = new ImportGraph(this.moduleResolver);
cycleAnalyzer = new CycleAnalyzer(this.importGraph);
handlers: DecoratorHandler<any, any>[] = [
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,
Expand Down
13 changes: 11 additions & 2 deletions packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts
Expand Up @@ -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) {
Expand All @@ -29,7 +31,7 @@ export class BaseDefDecoratorHandler implements
DecoratorHandler<R3BaseRefMetaData, R3BaseRefDecoratorDetection> {
constructor(
private reflector: ReflectionHost, private evaluator: PartialEvaluator,
private isCore: boolean) {}
private metaRegistry: MetadataRegistry, private isCore: boolean) {}

readonly precedence = HandlerPrecedence.WEAK;

Expand Down Expand Up @@ -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};
}

Expand Down
12 changes: 11 additions & 1 deletion packages/compiler-cli/src/ngtsc/incremental/src/state.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -100,6 +100,15 @@ export class IncrementalState implements DependencyTracker, MetadataReader, Meta
metadata.pipeMeta.set(meta.ref.node, meta);
}

getBaseMetadata(ref: Reference<ClassDeclaration>): 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);
Expand Down Expand Up @@ -131,4 +140,5 @@ class FileMetadata {
directiveMeta = new Map<ClassDeclaration, DirectiveMeta>();
ngModuleMeta = new Map<ClassDeclaration, NgModuleMeta>();
pipeMeta = new Map<ClassDeclaration, PipeMeta>();
baseMeta = new Map<ClassDeclaration, BaseMeta>();
}
31 changes: 31 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Expand Up @@ -70,6 +70,35 @@ export interface PipeMeta {
name: string;
}

/**
* Metadata for an undecorated class that uses Angular decorators
*/
export interface BaseMeta {
ref: Reference<ClassDeclaration>;

/**
* 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<ClassDeclaration>|'dynamic'|null;
}

/**
* Reads metadata for directives, pipes, and modules from a particular source, such as .d.ts files
* or a registry.
Expand All @@ -78,6 +107,7 @@ export interface MetadataReader {
getDirectiveMetadata(node: Reference<ClassDeclaration>): DirectiveMeta|null;
getNgModuleMetadata(node: Reference<ClassDeclaration>): NgModuleMeta|null;
getPipeMetadata(node: Reference<ClassDeclaration>): PipeMeta|null;
getBaseMetadata(node: Reference<ClassDeclaration>): BaseMeta|null;
}

/**
Expand All @@ -87,4 +117,5 @@ export interface MetadataRegistry {
registerDirectiveMetadata(meta: DirectiveMeta): void;
registerNgModuleMetadata(meta: NgModuleMeta): void;
registerPipeMetadata(meta: PipeMeta): void;
registerBaseMetadata(meta: BaseMeta): void;
}
11 changes: 10 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -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<ClassDeclaration>): 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):
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
Expand Up @@ -7,7 +7,7 @@
*/

import {Reference} from '../../imports';
import {DirectiveMeta, MetadataReader} from '../../metadata';
import {BaseMeta, DirectiveMeta, MetadataReader} from '../../metadata';
import {ClassDeclaration} from '../../reflection';

/**
Expand All @@ -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 {
Expand Down
13 changes: 12 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/registry.ts
Expand Up @@ -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
Expand All @@ -19,6 +19,7 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader {
private directives = new Map<ClassDeclaration, DirectiveMeta>();
private ngModules = new Map<ClassDeclaration, NgModuleMeta>();
private pipes = new Map<ClassDeclaration, PipeMeta>();
private baseClasses = new Map<ClassDeclaration, BaseMeta>();

getDirectiveMetadata(ref: Reference<ClassDeclaration>): DirectiveMeta|null {
return this.directives.has(ref.node) ? this.directives.get(ref.node) ! : null;
Expand All @@ -29,10 +30,14 @@ export class LocalMetadataRegistry implements MetadataRegistry, MetadataReader {
getPipeMetadata(ref: Reference<ClassDeclaration>): PipeMeta|null {
return this.pipes.has(ref.node) ? this.pipes.get(ref.node) ! : null;
}
getBaseMetadata(ref: Reference<ClassDeclaration>): 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); }
}

/**
Expand All @@ -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);
}
}
}
13 changes: 12 additions & 1 deletion packages/compiler-cli/src/ngtsc/metadata/src/util.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -144,6 +144,7 @@ export class CompoundMetadataReader implements MetadataReader {
}
return null;
}

getPipeMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): PipeMeta|null {
for (const reader of this.readers) {
const meta = reader.getPipeMetadata(node);
Expand All @@ -153,4 +154,14 @@ export class CompoundMetadataReader implements MetadataReader {
}
return null;
}

getBaseMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): BaseMeta|null {
for (const reader of this.readers) {
const meta = reader.getBaseMetadata(node);
if (meta !== null) {
return meta;
}
}
return null;
}
}
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngtsc/program.ts
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion packages/compiler-cli/src/ngtsc/scope/src/local.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down
39 changes: 39 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -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: '<div child [fromBase]="3" [fromChild]="4"></div>',
})
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'.`);
});
});

0 comments on commit 5ce641d

Please sign in to comment.