From e7d9cb3e4ccbd491e5bde166f6d2f292d7da9223 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Wed, 29 Nov 2017 16:29:05 -0800 Subject: [PATCH] feat(compiler): narrow types of expressions used in *ngIf (#20702) Structural directives can now specify a type guard that describes what types can be inferred for an input expression inside the directive's template. NgIf was modified to declare an input guard on ngIf. After this change, `fullTemplateTypeCheck` will infer that usage of `ngIf` expression inside it's template is truthy. For example, if a component has a property `person?: Person` and a template of `
{{person.name}}
` the compiler will no longer report that `person` might be null or undefined. The template compiler will generate code similar to, ``` if (NgIf.ngIfTypeGuard(instance.person)) { instance.person.name } ``` to validate the template's use of the interpolation expression. Calling the type guard in this fashion allows TypeScript to infer that `person` is non-null. Fixes: #19756? PR Close #20702 --- packages/common/src/directives/ng_if.ts | 2 + .../test/diagnostics/check_types_spec.ts | 135 ++++++++++++++++++ .../test/metadata/collector_spec.ts | 19 +++ packages/compiler/src/aot/compiler.ts | 2 +- packages/compiler/src/aot/static_reflector.ts | 29 ++++ packages/compiler/src/compile_metadata.ts | 36 ++++- packages/compiler/src/compile_reflector.ts | 1 + .../src/compiler_util/expression_converter.ts | 13 +- packages/compiler/src/core.ts | 1 + packages/compiler/src/directive_resolver.ts | 16 ++- packages/compiler/src/metadata_resolver.ts | 3 + .../src/view_compiler/type_check_compiler.ts | 78 ++++++++-- .../src/view_compiler/view_compiler.ts | 4 +- .../compiler/test/directive_resolver_spec.ts | 3 + .../template_parser/template_parser_spec.ts | 47 +++--- .../platform_reflection_capabilities.ts | 1 + .../src/reflection/reflection_capabilities.ts | 2 + .../src/compiler_reflector.ts | 1 + tools/public_api_guard/common/common.d.ts | 1 + 19 files changed, 341 insertions(+), 53 deletions(-) diff --git a/packages/common/src/directives/ng_if.ts b/packages/common/src/directives/ng_if.ts index fe9ee6f8ef..d143a3913e 100644 --- a/packages/common/src/directives/ng_if.ts +++ b/packages/common/src/directives/ng_if.ts @@ -151,6 +151,8 @@ export class NgIf { } } } + + public static ngIfTypeGuard: (v: T|null|undefined|false) => v is T; } /** diff --git a/packages/compiler-cli/test/diagnostics/check_types_spec.ts b/packages/compiler-cli/test/diagnostics/check_types_spec.ts index a8a3244bae..0e634eef1a 100644 --- a/packages/compiler-cli/test/diagnostics/check_types_spec.ts +++ b/packages/compiler-cli/test/diagnostics/check_types_spec.ts @@ -81,6 +81,141 @@ describe('ng type checker', () => { }); }); + describe('type narrowing', () => { + const a = (files: MockFiles, options: object = {}) => { + accept(files, {fullTemplateTypeCheck: true, ...options}); + }; + + it('should narrow an *ngIf like directive', () => { + a({ + 'src/app.component.ts': '', + 'src/lib.ts': '', + 'src/app.module.ts': ` + import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core'; + + export interface Person { + name: string; + } + + @Component({ + selector: 'comp', + template: '
{{person.name}}
' + }) + export class MainComp { + person?: Person; + } + + export class MyIfContext { + public $implicit: any = null; + public myIf: any = null; + } + + @Directive({selector: '[myIf]'}) + export class MyIf { + constructor(templateRef: TemplateRef) {} + + @Input() + set myIf(condition: any) {} + + static myIfTypeGuard: (v: T | null | undefined | false) => v is T; + } + + @NgModule({ + declarations: [MainComp, MyIf], + }) + export class MainModule {}` + }); + }); + + it('should narrow a renamed *ngIf like directive', () => { + a({ + 'src/app.component.ts': '', + 'src/lib.ts': '', + 'src/app.module.ts': ` + import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core'; + + export interface Person { + name: string; + } + + @Component({ + selector: 'comp', + template: '
{{person.name}}
' + }) + export class MainComp { + person?: Person; + } + + export class MyIfContext { + public $implicit: any = null; + public myIf: any = null; + } + + @Directive({selector: '[my-if]'}) + export class MyIf { + constructor(templateRef: TemplateRef) {} + + @Input('my-if') + set myIf(condition: any) {} + + static myIfTypeGuard: (v: T | null | undefined | false) => v is T; + } + + @NgModule({ + declarations: [MainComp, MyIf], + }) + export class MainModule {}` + }); + }); + + it('should narrow a type in a nested *ngIf like directive', () => { + a({ + 'src/app.component.ts': '', + 'src/lib.ts': '', + 'src/app.module.ts': ` + import {NgModule, Component, Directive, HostListener, TemplateRef, Input} from '@angular/core'; + + export interface Address { + street: string; + } + + export interface Person { + name: string; + address?: Address; + } + + + @Component({ + selector: 'comp', + template: '
{{person.name}} {{person.address.street}}
' + }) + export class MainComp { + person?: Person; + } + + export class MyIfContext { + public $implicit: any = null; + public myIf: any = null; + } + + @Directive({selector: '[myIf]'}) + export class MyIf { + constructor(templateRef: TemplateRef) {} + + @Input() + set myIf(condition: any) {} + + static myIfTypeGuard: (v: T | null | undefined | false) => v is T; + } + + @NgModule({ + declarations: [MainComp, MyIf], + }) + export class MainModule {}` + }); + }); + }); + describe('regressions ', () => { const a = (files: MockFiles, options: object = {}) => { accept(files, {fullTemplateTypeCheck: true, ...options}); diff --git a/packages/compiler-cli/test/metadata/collector_spec.ts b/packages/compiler-cli/test/metadata/collector_spec.ts index e4caf48af8..e88e72aa33 100644 --- a/packages/compiler-cli/test/metadata/collector_spec.ts +++ b/packages/compiler-cli/test/metadata/collector_spec.ts @@ -1038,6 +1038,25 @@ describe('Collector', () => { expect(metadata).toBeUndefined(); }); + it('should collect type guards', () => { + const metadata = collectSource(` + import {Directive, Input, TemplateRef} from '@angular/core'; + + @Directive({selector: '[myIf]'}) + export class MyIf { + + constructor(private templateRef: TemplateRef) {} + + @Input() myIf: any; + + static typeGuard: (v: T | null | undefined): v is T; + } + `); + + expect((metadata.metadata.MyIf as any).statics.typeGuard) + .not.toBeUndefined('typeGuard was not collected'); + }); + it('should be able to collect an invalid access expression', () => { const source = createSource(` import {Component} from '@angular/core'; diff --git a/packages/compiler/src/aot/compiler.ts b/packages/compiler/src/aot/compiler.ts index e8c80d182d..e2c11de6a1 100644 --- a/packages/compiler/src/aot/compiler.ts +++ b/packages/compiler/src/aot/compiler.ts @@ -271,7 +271,7 @@ export class AotCompiler { const {template: parsedTemplate, pipes: usedPipes} = this._parseTemplate(compMeta, moduleMeta, directives); ctx.statements.push(...this._typeCheckCompiler.compileComponent( - componentId, compMeta, parsedTemplate, usedPipes, externalReferenceVars)); + componentId, compMeta, parsedTemplate, usedPipes, externalReferenceVars, ctx)); } emitMessageBundle(analyzeResult: NgAnalyzedModules, locale: string|null): MessageBundle { diff --git a/packages/compiler/src/aot/static_reflector.ts b/packages/compiler/src/aot/static_reflector.ts index 7f20150036..3576566945 100644 --- a/packages/compiler/src/aot/static_reflector.ts +++ b/packages/compiler/src/aot/static_reflector.ts @@ -29,6 +29,7 @@ const IGNORE = { const USE_VALUE = 'useValue'; const PROVIDE = 'provide'; const REFERENCE_SET = new Set([USE_VALUE, 'useFactory', 'data']); +const TYPEGUARD_POSTFIX = 'TypeGuard'; function shouldIgnore(value: any): boolean { return value && value.__symbolic == 'ignore'; @@ -43,6 +44,7 @@ export class StaticReflector implements CompileReflector { private propertyCache = new Map(); private parameterCache = new Map(); private methodCache = new Map(); + private staticCache = new Map(); private conversionMap = new Map any>(); private injectionToken: StaticSymbol; private opaqueToken: StaticSymbol; @@ -251,6 +253,18 @@ export class StaticReflector implements CompileReflector { return methodNames; } + private _staticMembers(type: StaticSymbol): string[] { + let staticMembers = this.staticCache.get(type); + if (!staticMembers) { + const classMetadata = this.getTypeMetadata(type); + const staticMemberData = classMetadata['statics'] || {}; + staticMembers = Object.keys(staticMemberData); + this.staticCache.set(type, staticMembers); + } + return staticMembers; + } + + private findParentType(type: StaticSymbol, classMetadata: any): StaticSymbol|undefined { const parentType = this.trySimplify(type, classMetadata['extends']); if (parentType instanceof StaticSymbol) { @@ -273,6 +287,21 @@ export class StaticReflector implements CompileReflector { } } + guards(type: any): {[key: string]: StaticSymbol} { + if (!(type instanceof StaticSymbol)) { + this.reportError( + new Error(`guards received ${JSON.stringify(type)} which is not a StaticSymbol`), type); + return {}; + } + const staticMembers = this._staticMembers(type); + const result: {[key: string]: StaticSymbol} = {}; + for (let name of staticMembers) { + result[name.substr(0, name.length - TYPEGUARD_POSTFIX.length)] = + this.getStaticSymbol(type.filePath, type.name, [name]); + } + return result; + } + private _registerDecoratorOrConstructor(type: StaticSymbol, ctor: any): void { this.conversionMap.set(type, (context: StaticSymbol, args: any[]) => new ctor(...args)); } diff --git a/packages/compiler/src/compile_metadata.ts b/packages/compiler/src/compile_metadata.ts index 174eff42f2..33c930ea01 100644 --- a/packages/compiler/src/compile_metadata.ts +++ b/packages/compiler/src/compile_metadata.ts @@ -254,6 +254,7 @@ export interface CompileDirectiveSummary extends CompileTypeSummary { providers: CompileProviderMetadata[]; viewProviders: CompileProviderMetadata[]; queries: CompileQueryMetadata[]; + guards: {[key: string]: any}; viewQueries: CompileQueryMetadata[]; entryComponents: CompileEntryComponentMetadata[]; changeDetection: ChangeDetectionStrategy|null; @@ -268,8 +269,8 @@ export interface CompileDirectiveSummary extends CompileTypeSummary { */ export class CompileDirectiveMetadata { static create({isHost, type, isComponent, selector, exportAs, changeDetection, inputs, outputs, - host, providers, viewProviders, queries, viewQueries, entryComponents, template, - componentViewType, rendererType, componentFactory}: { + host, providers, viewProviders, queries, guards, viewQueries, entryComponents, + template, componentViewType, rendererType, componentFactory}: { isHost: boolean, type: CompileTypeMetadata, isComponent: boolean, @@ -282,6 +283,7 @@ export class CompileDirectiveMetadata { providers: CompileProviderMetadata[], viewProviders: CompileProviderMetadata[], queries: CompileQueryMetadata[], + guards: {[key: string]: any}; viewQueries: CompileQueryMetadata[], entryComponents: CompileEntryComponentMetadata[], template: CompileTemplateMetadata, @@ -336,6 +338,7 @@ export class CompileDirectiveMetadata { providers, viewProviders, queries, + guards, viewQueries, entryComponents, template, @@ -358,6 +361,7 @@ export class CompileDirectiveMetadata { providers: CompileProviderMetadata[]; viewProviders: CompileProviderMetadata[]; queries: CompileQueryMetadata[]; + guards: {[key: string]: any}; viewQueries: CompileQueryMetadata[]; entryComponents: CompileEntryComponentMetadata[]; @@ -367,10 +371,27 @@ export class CompileDirectiveMetadata { rendererType: StaticSymbol|object|null; componentFactory: StaticSymbol|object|null; - constructor({isHost, type, isComponent, selector, exportAs, - changeDetection, inputs, outputs, hostListeners, hostProperties, - hostAttributes, providers, viewProviders, queries, viewQueries, - entryComponents, template, componentViewType, rendererType, componentFactory}: { + constructor({isHost, + type, + isComponent, + selector, + exportAs, + changeDetection, + inputs, + outputs, + hostListeners, + hostProperties, + hostAttributes, + providers, + viewProviders, + queries, + guards, + viewQueries, + entryComponents, + template, + componentViewType, + rendererType, + componentFactory}: { isHost: boolean, type: CompileTypeMetadata, isComponent: boolean, @@ -385,6 +406,7 @@ export class CompileDirectiveMetadata { providers: CompileProviderMetadata[], viewProviders: CompileProviderMetadata[], queries: CompileQueryMetadata[], + guards: {[key: string]: any}, viewQueries: CompileQueryMetadata[], entryComponents: CompileEntryComponentMetadata[], template: CompileTemplateMetadata|null, @@ -406,6 +428,7 @@ export class CompileDirectiveMetadata { this.providers = _normalizeArray(providers); this.viewProviders = _normalizeArray(viewProviders); this.queries = _normalizeArray(queries); + this.guards = guards; this.viewQueries = _normalizeArray(viewQueries); this.entryComponents = _normalizeArray(entryComponents); this.template = template; @@ -430,6 +453,7 @@ export class CompileDirectiveMetadata { providers: this.providers, viewProviders: this.viewProviders, queries: this.queries, + guards: this.guards, viewQueries: this.viewQueries, entryComponents: this.entryComponents, changeDetection: this.changeDetection, diff --git a/packages/compiler/src/compile_reflector.ts b/packages/compiler/src/compile_reflector.ts index 96cc30c8c9..44970091a2 100644 --- a/packages/compiler/src/compile_reflector.ts +++ b/packages/compiler/src/compile_reflector.ts @@ -17,6 +17,7 @@ export abstract class CompileReflector { abstract annotations(typeOrFunc: /*Type*/ any): any[]; abstract propMetadata(typeOrFunc: /*Type*/ any): {[key: string]: any[]}; abstract hasLifecycleHook(type: any, lcProperty: string): boolean; + abstract guards(typeOrFunc: /* Type */ any): {[key: string]: any}; abstract componentModuleUrl(type: /*Type*/ any, cmpMetadata: Component): string; abstract resolveExternalReference(ref: o.ExternalReference): any; } diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index a5abc8aa36..52fbb6faa6 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -90,6 +90,14 @@ export class ConvertPropertyBindingResult { constructor(public stmts: o.Statement[], public currValExpr: o.Expression) {} } +export enum BindingForm { + // The general form of binding expression, supports all expressions. + General, + + // Try to generate a simple binding (no temporaries or statements) + // otherise generate a general binding + TrySimple, +} /** * Converts the given expression AST into an executable output AST, assuming the expression * is used in property binding. The expression has to be preprocessed via @@ -97,7 +105,8 @@ export class ConvertPropertyBindingResult { */ export function convertPropertyBinding( localResolver: LocalResolver | null, implicitReceiver: o.Expression, - expressionWithoutBuiltins: cdAst.AST, bindingId: string): ConvertPropertyBindingResult { + expressionWithoutBuiltins: cdAst.AST, bindingId: string, + form: BindingForm): ConvertPropertyBindingResult { if (!localResolver) { localResolver = new DefaultLocalResolver(); } @@ -110,6 +119,8 @@ export function convertPropertyBinding( for (let i = 0; i < visitor.temporaryCount; i++) { stmts.push(temporaryDeclaration(bindingId, i)); } + } else if (form == BindingForm.TrySimple) { + return new ConvertPropertyBindingResult([], outputExpr); } stmts.push(currValExpr.set(outputExpr).toDeclStmt(null, [o.StmtModifier.Final])); diff --git a/packages/compiler/src/core.ts b/packages/compiler/src/core.ts index 57fdca02d5..e1bc491676 100644 --- a/packages/compiler/src/core.ts +++ b/packages/compiler/src/core.ts @@ -51,6 +51,7 @@ export interface Directive { providers?: Provider[]; exportAs?: string; queries?: {[key: string]: any}; + guards?: {[key: string]: any}; } export const createDirective = makeMetadataFactory('Directive', (dir: Directive = {}) => dir); diff --git a/packages/compiler/src/directive_resolver.ts b/packages/compiler/src/directive_resolver.ts index c32f328d1a..be786b4a9b 100644 --- a/packages/compiler/src/directive_resolver.ts +++ b/packages/compiler/src/directive_resolver.ts @@ -44,7 +44,8 @@ export class DirectiveResolver { const metadata = findLast(typeMetadata, isDirectiveMetadata); if (metadata) { const propertyMetadata = this._reflector.propMetadata(type); - return this._mergeWithPropertyMetadata(metadata, propertyMetadata, type); + const guards = this._reflector.guards(type); + return this._mergeWithPropertyMetadata(metadata, propertyMetadata, guards, type); } } @@ -56,12 +57,12 @@ export class DirectiveResolver { } private _mergeWithPropertyMetadata( - dm: Directive, propertyMetadata: {[key: string]: any[]}, directiveType: Type): Directive { + dm: Directive, propertyMetadata: {[key: string]: any[]}, guards: {[key: string]: any}, + directiveType: Type): Directive { const inputs: string[] = []; const outputs: string[] = []; const host: {[key: string]: string} = {}; const queries: {[key: string]: any} = {}; - Object.keys(propertyMetadata).forEach((propName: string) => { const input = findLast(propertyMetadata[propName], (a) => createInput.isTypeOf(a)); if (input) { @@ -105,18 +106,20 @@ export class DirectiveResolver { queries[propName] = query; } }); - return this._merge(dm, inputs, outputs, host, queries, directiveType); + return this._merge(dm, inputs, outputs, host, queries, guards, directiveType); } private _extractPublicName(def: string) { return splitAtColon(def, [null !, def])[1].trim(); } private _dedupeBindings(bindings: string[]): string[] { const names = new Set(); + const publicNames = new Set(); const reversedResult: string[] = []; // go last to first to allow later entries to overwrite previous entries for (let i = bindings.length - 1; i >= 0; i--) { const binding = bindings[i]; const name = this._extractPublicName(binding); + publicNames.add(name); if (!names.has(name)) { names.add(name); reversedResult.push(binding); @@ -127,14 +130,13 @@ export class DirectiveResolver { private _merge( directive: Directive, inputs: string[], outputs: string[], host: {[key: string]: string}, - queries: {[key: string]: any}, directiveType: Type): Directive { + queries: {[key: string]: any}, guards: {[key: string]: any}, directiveType: Type): Directive { const mergedInputs = this._dedupeBindings(directive.inputs ? directive.inputs.concat(inputs) : inputs); const mergedOutputs = this._dedupeBindings(directive.outputs ? directive.outputs.concat(outputs) : outputs); const mergedHost = directive.host ? {...directive.host, ...host} : host; const mergedQueries = directive.queries ? {...directive.queries, ...queries} : queries; - if (createComponent.isTypeOf(directive)) { const comp = directive as Component; return createComponent({ @@ -166,7 +168,7 @@ export class DirectiveResolver { host: mergedHost, exportAs: directive.exportAs, queries: mergedQueries, - providers: directive.providers + providers: directive.providers, guards }); } } diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index f9d71b4fab..15920b776a 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -208,6 +208,7 @@ export class CompileMetadataResolver { providers: [], viewProviders: [], queries: [], + guards: {}, viewQueries: [], componentViewType: hostViewType, rendererType: @@ -240,6 +241,7 @@ export class CompileMetadataResolver { providers: metadata.providers, viewProviders: metadata.viewProviders, queries: metadata.queries, + guards: metadata.guards, viewQueries: metadata.viewQueries, entryComponents: metadata.entryComponents, componentViewType: metadata.componentViewType, @@ -383,6 +385,7 @@ export class CompileMetadataResolver { providers: providers || [], viewProviders: viewProviders || [], queries: queries || [], + guards: dirMeta.guards || {}, viewQueries: viewQueries || [], entryComponents: entryComponentMetadata, componentViewType: nonNormalizedTemplateMetadata ? this.getComponentViewClass(directiveType) : diff --git a/packages/compiler/src/view_compiler/type_check_compiler.ts b/packages/compiler/src/view_compiler/type_check_compiler.ts index 3e1677f280..19b2221f7e 100644 --- a/packages/compiler/src/view_compiler/type_check_compiler.ts +++ b/packages/compiler/src/view_compiler/type_check_compiler.ts @@ -10,13 +10,15 @@ import {AotCompilerOptions} from '../aot/compiler_options'; import {StaticReflector} from '../aot/static_reflector'; import {StaticSymbol} from '../aot/static_symbol'; import {CompileDiDependencyMetadata, CompileDirectiveMetadata, CompilePipeSummary} from '../compile_metadata'; -import {BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter'; +import {BindingForm, BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter'; import {AST, ASTWithSource, Interpolation} from '../expression_parser/ast'; import {Identifiers} from '../identifiers'; import * as o from '../output/output_ast'; import {convertValueToOutputAst} from '../output/value_util'; import {ParseSourceSpan} from '../parse_util'; import {AttrAst, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, DirectiveAst, ElementAst, EmbeddedTemplateAst, NgContentAst, PropertyBindingType, ProviderAst, ProviderAstType, QueryMatch, ReferenceAst, TemplateAst, TemplateAstVisitor, TextAst, VariableAst, templateVisitAll} from '../template_parser/template_ast'; +import {OutputContext} from '../util'; + /** * Generates code that is used to type check templates. @@ -34,27 +36,33 @@ export class TypeCheckCompiler { */ compileComponent( componentId: string, component: CompileDirectiveMetadata, template: TemplateAst[], - usedPipes: CompilePipeSummary[], - externalReferenceVars: Map): o.Statement[] { + usedPipes: CompilePipeSummary[], externalReferenceVars: Map, + ctx: OutputContext): o.Statement[] { const pipes = new Map(); usedPipes.forEach(p => pipes.set(p.name, p.type.reference)); let embeddedViewCount = 0; - const viewBuilderFactory = (parent: ViewBuilder | null): ViewBuilder => { - const embeddedViewIndex = embeddedViewCount++; - return new ViewBuilder( - this.options, this.reflector, externalReferenceVars, parent, component.type.reference, - component.isHost, embeddedViewIndex, pipes, viewBuilderFactory); - }; + const viewBuilderFactory = + (parent: ViewBuilder | null, guards: GuardExpression[]): ViewBuilder => { + const embeddedViewIndex = embeddedViewCount++; + return new ViewBuilder( + this.options, this.reflector, externalReferenceVars, parent, component.type.reference, + component.isHost, embeddedViewIndex, pipes, guards, ctx, viewBuilderFactory); + }; - const visitor = viewBuilderFactory(null); + const visitor = viewBuilderFactory(null, []); visitor.visitAll([], template); return visitor.build(componentId); } } +interface GuardExpression { + guard: StaticSymbol; + expression: Expression; +} + interface ViewBuilderFactory { - (parent: ViewBuilder): ViewBuilder; + (parent: ViewBuilder, guards: GuardExpression[]): ViewBuilder; } // Note: This is used as key in Map and should therefore be @@ -94,6 +102,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { private externalReferenceVars: Map, private parent: ViewBuilder|null, private component: StaticSymbol, private isHostComponent: boolean, private embeddedViewIndex: number, private pipes: Map, + private guards: GuardExpression[], private ctx: OutputContext, private viewBuilderFactory: ViewBuilderFactory) {} private getOutputVar(type: o.BuiltinTypeName|StaticSymbol): string { @@ -112,6 +121,20 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { return varName; } + private getTypeGuardExpressions(ast: EmbeddedTemplateAst): GuardExpression[] { + const result = [...this.guards]; + for (let directive of ast.directives) { + for (let input of directive.inputs) { + const guard = directive.directive.guards[input.directiveName]; + if (guard) { + result.push( + {guard, expression: {context: this.component, value: input.value} as Expression}); + } + } + } + return result; + } + visitAll(variables: VariableAst[], astNodes: TemplateAst[]) { this.variables = variables; templateVisitAll(this, astNodes); @@ -119,7 +142,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { build(componentId: string, targetStatements: o.Statement[] = []): o.Statement[] { this.children.forEach((child) => child.build(componentId, targetStatements)); - const viewStmts: o.Statement[] = + let viewStmts: o.Statement[] = [o.variable(DYNAMIC_VAR_NAME).set(o.NULL_EXPR).toDeclStmt(o.DYNAMIC_TYPE)]; let bindingCount = 0; this.updates.forEach((expression) => { @@ -127,7 +150,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { const bindingId = `${bindingCount++}`; const nameResolver = context === this.component ? this : defaultResolver; const {stmts, currValExpr} = convertPropertyBinding( - nameResolver, o.variable(this.getOutputVar(context)), value, bindingId); + nameResolver, o.variable(this.getOutputVar(context)), value, bindingId, + BindingForm.General); stmts.push(new o.ExpressionStatement(currValExpr)); viewStmts.push(...stmts.map( (stmt: o.Statement) => o.applySourceSpanToStatementIfNeeded(stmt, sourceSpan))); @@ -142,6 +166,27 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { (stmt: o.Statement) => o.applySourceSpanToStatementIfNeeded(stmt, sourceSpan))); }); + if (this.guards.length) { + let guardExpression: o.Expression|undefined = undefined; + for (const guard of this.guards) { + const {context, value} = this.preprocessUpdateExpression(guard.expression); + const bindingId = `${bindingCount++}`; + const nameResolver = context === this.component ? this : defaultResolver; + // We only support support simple expressions and ignore others as they + // are unlikely to affect type narrowing. + const {stmts, currValExpr} = convertPropertyBinding( + nameResolver, o.variable(this.getOutputVar(context)), value, bindingId, + BindingForm.TrySimple); + if (stmts.length == 0) { + const callGuard = this.ctx.importExpr(guard.guard).callFn([currValExpr]); + guardExpression = guardExpression ? guardExpression.and(callGuard) : callGuard; + } + } + if (guardExpression) { + viewStmts = [new o.IfStmt(guardExpression, viewStmts)]; + } + } + const viewName = `_View_${componentId}_${this.embeddedViewIndex}`; const viewFactory = new o.DeclareFunctionStmt(viewName, [], viewStmts); targetStatements.push(viewFactory); @@ -163,7 +208,12 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // for the context in any embedded view. // We keep this behaivor behind a flag for now. if (this.options.fullTemplateTypeCheck) { - const childVisitor = this.viewBuilderFactory(this); + // Find any applicable type guards. For example, NgIf has a type guard on ngIf + // (see NgIf.ngIfTypeGuard) that can be used to indicate that a template is only + // stamped out if ngIf is truthy so any bindings in the template can assume that, + // if a nullable type is used for ngIf, that expression is not null or undefined. + const guards = this.getTypeGuardExpressions(ast); + const childVisitor = this.viewBuilderFactory(this, guards); this.children.push(childVisitor); childVisitor.visitAll(ast.variables, ast.children); } diff --git a/packages/compiler/src/view_compiler/view_compiler.ts b/packages/compiler/src/view_compiler/view_compiler.ts index 2c0b130f89..47299e6541 100644 --- a/packages/compiler/src/view_compiler/view_compiler.ts +++ b/packages/compiler/src/view_compiler/view_compiler.ts @@ -8,7 +8,7 @@ import {CompileDirectiveMetadata, CompilePipeSummary, rendererTypeName, tokenReference, viewClassName} from '../compile_metadata'; import {CompileReflector} from '../compile_reflector'; -import {BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter'; +import {BindingForm, BuiltinConverter, EventHandlerVars, LocalResolver, convertActionBinding, convertPropertyBinding, convertPropertyBindingBuiltins} from '../compiler_util/expression_converter'; import {ArgumentType, BindingFlags, ChangeDetectionStrategy, NodeFlags, QueryBindingType, QueryValueType, ViewFlags} from '../core'; import {AST, ASTWithSource, Interpolation} from '../expression_parser/ast'; import {Identifiers} from '../identifiers'; @@ -859,7 +859,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { const bindingId = `${updateBindingCount++}`; const nameResolver = context === COMP_VAR ? self : null; const {stmts, currValExpr} = - convertPropertyBinding(nameResolver, context, value, bindingId); + convertPropertyBinding(nameResolver, context, value, bindingId, BindingForm.General); updateStmts.push(...stmts.map( (stmt: o.Statement) => o.applySourceSpanToStatementIfNeeded(stmt, sourceSpan))); return o.applySourceSpanToExpressionIfNeeded(currValExpr, sourceSpan); diff --git a/packages/compiler/test/directive_resolver_spec.ts b/packages/compiler/test/directive_resolver_spec.ts index 3e4db96754..f8103b3987 100644 --- a/packages/compiler/test/directive_resolver_spec.ts +++ b/packages/compiler/test/directive_resolver_spec.ts @@ -126,6 +126,7 @@ export function main() { outputs: [], host: {}, queries: {}, + guards: {}, exportAs: undefined, providers: undefined })); @@ -154,6 +155,7 @@ export function main() { outputs: [], host: {}, queries: {}, + guards: {}, exportAs: undefined, providers: undefined })); @@ -164,6 +166,7 @@ export function main() { outputs: [], host: {}, queries: {}, + guards: {}, exportAs: undefined, providers: undefined })); diff --git a/packages/compiler/test/template_parser/template_parser_spec.ts b/packages/compiler/test/template_parser/template_parser_spec.ts index a15293ee9f..950a79cf3d 100644 --- a/packages/compiler/test/template_parser/template_parser_spec.ts +++ b/packages/compiler/test/template_parser/template_parser_spec.ts @@ -38,28 +38,29 @@ function createTypeMeta({reference, diDeps}: {reference: any, diDeps?: any[]}): return {reference: reference, diDeps: diDeps || [], lifecycleHooks: []}; } -function compileDirectiveMetadataCreate({isHost, type, isComponent, selector, exportAs, - changeDetection, inputs, outputs, host, providers, - viewProviders, queries, viewQueries, entryComponents, - template, componentViewType, rendererType}: { - isHost?: boolean, - type?: CompileTypeMetadata, - isComponent?: boolean, - selector?: string | null, - exportAs?: string | null, - changeDetection?: ChangeDetectionStrategy | null, - inputs?: string[], - outputs?: string[], - host?: {[key: string]: string}, - providers?: CompileProviderMetadata[] | null, - viewProviders?: CompileProviderMetadata[] | null, - queries?: CompileQueryMetadata[] | null, - viewQueries?: CompileQueryMetadata[], - entryComponents?: CompileEntryComponentMetadata[], - template?: CompileTemplateMetadata, - componentViewType?: StaticSymbol | ProxyClass | null, - rendererType?: StaticSymbol | RendererType2 | null, -}) { +function compileDirectiveMetadataCreate( + {isHost, type, isComponent, selector, exportAs, changeDetection, inputs, outputs, host, + providers, viewProviders, queries, guards, viewQueries, entryComponents, template, + componentViewType, rendererType}: { + isHost?: boolean, + type?: CompileTypeMetadata, + isComponent?: boolean, + selector?: string | null, + exportAs?: string | null, + changeDetection?: ChangeDetectionStrategy | null, + inputs?: string[], + outputs?: string[], + host?: {[key: string]: string}, + providers?: CompileProviderMetadata[] | null, + viewProviders?: CompileProviderMetadata[] | null, + queries?: CompileQueryMetadata[] | null, + guards?: {[key: string]: any}, + viewQueries?: CompileQueryMetadata[], + entryComponents?: CompileEntryComponentMetadata[], + template?: CompileTemplateMetadata, + componentViewType?: StaticSymbol | ProxyClass | null, + rendererType?: StaticSymbol | RendererType2 | null, + }) { return CompileDirectiveMetadata.create({ isHost: !!isHost, type: noUndefined(type) !, @@ -73,6 +74,7 @@ function compileDirectiveMetadataCreate({isHost, type, isComponent, selector, ex providers: providers || [], viewProviders: viewProviders || [], queries: queries || [], + guards: guards || {}, viewQueries: viewQueries || [], entryComponents: entryComponents || [], template: noUndefined(template) !, @@ -390,6 +392,7 @@ export function main() { providers: [], viewProviders: [], queries: [], + guards: {}, viewQueries: [], entryComponents: [], componentViewType: null, diff --git a/packages/core/src/reflection/platform_reflection_capabilities.ts b/packages/core/src/reflection/platform_reflection_capabilities.ts index 8cc5b7862b..0fee368520 100644 --- a/packages/core/src/reflection/platform_reflection_capabilities.ts +++ b/packages/core/src/reflection/platform_reflection_capabilities.ts @@ -13,6 +13,7 @@ export interface PlatformReflectionCapabilities { isReflectionEnabled(): boolean; factory(type: Type): Function; hasLifecycleHook(type: any, lcProperty: string): boolean; + guards(type: any): {[key: string]: any}; /** * Return a list of annotations/types for constructor parameters diff --git a/packages/core/src/reflection/reflection_capabilities.ts b/packages/core/src/reflection/reflection_capabilities.ts index 4f74baf886..1e6123e7de 100644 --- a/packages/core/src/reflection/reflection_capabilities.ts +++ b/packages/core/src/reflection/reflection_capabilities.ts @@ -207,6 +207,8 @@ export class ReflectionCapabilities implements PlatformReflectionCapabilities { return type instanceof Type && lcProperty in type.prototype; } + guards(type: any): {[key: string]: any} { return {}; } + getter(name: string): GetterFn { return new Function('o', 'return o.' + name + ';'); } setter(name: string): SetterFn { diff --git a/packages/platform-browser-dynamic/src/compiler_reflector.ts b/packages/platform-browser-dynamic/src/compiler_reflector.ts index 6575540806..b959b4870d 100644 --- a/packages/platform-browser-dynamic/src/compiler_reflector.ts +++ b/packages/platform-browser-dynamic/src/compiler_reflector.ts @@ -42,6 +42,7 @@ export class JitReflector implements CompileReflector { hasLifecycleHook(type: any, lcProperty: string): boolean { return this.reflectionCapabilities.hasLifecycleHook(type, lcProperty); } + guards(type: any): {[key: string]: any} { return this.reflectionCapabilities.guards(type); } resolveExternalReference(ref: ExternalReference): any { return builtinExternalReferences.get(ref) || ref.runtime; } diff --git a/tools/public_api_guard/common/common.d.ts b/tools/public_api_guard/common/common.d.ts index 07ccaf97dd..5262ad744e 100644 --- a/tools/public_api_guard/common/common.d.ts +++ b/tools/public_api_guard/common/common.d.ts @@ -276,6 +276,7 @@ export declare class NgIf { ngIfElse: TemplateRef; ngIfThen: TemplateRef; constructor(_viewContainer: ViewContainerRef, templateRef: TemplateRef); + static ngIfTypeGuard: (v: T | null | undefined | false) => v is T; } /** @stable */