feat(ivy): more accurate type narrowing for `ngIf` directive (#30248)

A structural directive can specify a template guard for an input, such that
the type of that input's binding can be narrowed based on the guard's return
type. Previously, such template guards could only be methods, of which an
invocation would be inserted into the type-check block (TCB). For `NgIf`,
the template guard narrowed the type of its expression to be `NonNullable`
using the following declaration:

```typescript
export declare class NgIf {
  static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E): expr is NonNullable<E>
}
```

This works fine for usages such as `*ngIf="person"` but starts to introduce
false-positives when e.g. an explicit non-null check like
`*ngIf="person !== null"` is used, as the method invocation in the TCB
would not have the desired effect of narrowing `person` to become
non-nullable:

```typescript
if (NgIf.ngTemplateGuard_ngIf(directive, ctx.person !== null)) {
  // Usages of `ctx.person` within this block would
  // not have been narrowed to be non-nullable.
}
```

This commit introduces a new strategy for template guards to allow for the
binding expression itself to be used as template guard in the TCB. Now,
the TCB generated for `*ngIf="person !== null"` would look as follows:

```typescript
if (ctx.person !== null) {
  // This time `ctx.person` will successfully have
  // been narrowed to be non-nullable.
}
```

This strategy can be activated by declaring the template guard as a
property declaration with `'binding'` as literal return type.

See #30235 for an example where this led to a false positive.

PR Close #30248

packages/common/src/directives/ng_if.ts

@@ -219,12 +219,12 @@ export class NgIf {
   /**
    * Assert the correct type of the expression bound to the `ngIf` input within the template.
    *
-   * The presence of this method is a signal to the Ivy template type check compiler that when the
-   * `NgIf` structural directive renders its template, the type of the expression bound to `ngIf`
-   * should be narrowed in some way. For `NgIf`, it is narrowed to be non-null, which allows the
-   * strictNullChecks feature of TypeScript to work with `NgIf`.
+   * The presence of this static field is a signal to the Ivy template type check compiler that
+   * when the `NgIf` structural directive renders its template, the type of the expression bound
+   * to `ngIf` should be narrowed in some way. For `NgIf`, the binding expression itself is used to
+   * narrow its type, which allows the strictNullChecks feature of TypeScript to work with `NgIf`.
    */
-  static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E): expr is NonNullable<E> { return true; }
+  static ngTemplateGuard_ngIf: 'binding';
 }
 
 /**

packages/compiler-cli/src/ngtsc/metadata/src/api.ts

@@ -31,7 +31,7 @@ export interface DirectiveMeta extends T2DirectiveMeta {
    */
   selector: string;
   queries: string[];
-  ngTemplateGuards: string[];
+  ngTemplateGuards: TemplateGuardMeta[];
   hasNgTemplateContextGuard: boolean;
 
   /**
@@ -43,6 +43,25 @@ export interface DirectiveMeta extends T2DirectiveMeta {
   baseClass: Reference<ClassDeclaration>|'dynamic'|null;
 }
 
+/**
+ * Metadata that describes a template guard for one of the directive's inputs.
+ */
+export interface TemplateGuardMeta {
+  /**
+   * The input name that this guard should be applied to.
+   */
+  inputName: string;
+
+  /**
+   * Represents the type of the template guard.
+   *
+   * - 'invocation' means that a call to the template guard function is emitted so that its return
+   *   type can result in narrowing of the input type.
+   * - 'binding' means that the input binding expression itself is used as template guard.
+   */
+  type: 'invocation'|'binding';
+}
+
 /**
  * Metadata for a pipe within an NgModule's scope.
  */

packages/compiler-cli/src/ngtsc/metadata/src/util.ts

@@ -9,10 +9,10 @@
 import * as ts from 'typescript';
 
 import {Reference} from '../../imports';
-import {ClassDeclaration, ClassMemberKind, ReflectionHost, isNamedClassDeclaration, reflectTypeEntityToDeclaration} from '../../reflection';
+import {ClassDeclaration, ClassMember, ClassMemberKind, ReflectionHost, isNamedClassDeclaration, reflectTypeEntityToDeclaration} from '../../reflection';
 import {nodeDebugInfo} from '../../util/src/typescript';
 
-import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api';
+import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta, TemplateGuardMeta} from './api';
 
 export function extractReferencesFromType(
     checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string | null,
@@ -80,20 +80,39 @@ export function readStringArrayType(type: ts.TypeNode): string[] {
 
 
 export function extractDirectiveGuards(node: ClassDeclaration, reflector: ReflectionHost): {
-  ngTemplateGuards: string[],
+  ngTemplateGuards: TemplateGuardMeta[],
   hasNgTemplateContextGuard: boolean,
 } {
-  const methods = nodeStaticMethodNames(node, reflector);
-  const ngTemplateGuards = methods.filter(method => method.startsWith('ngTemplateGuard_'))
-                               .map(method => method.split('_', 2)[1]);
-  const hasNgTemplateContextGuard = methods.some(name => name === 'ngTemplateContextGuard');
+  const staticMembers = reflector.getMembersOfClass(node).filter(member => member.isStatic);
+  const ngTemplateGuards = staticMembers.map(extractTemplateGuard)
+                               .filter((guard): guard is TemplateGuardMeta => guard !== null);
+  const hasNgTemplateContextGuard = staticMembers.some(
+      member => member.kind === ClassMemberKind.Method && member.name === 'ngTemplateContextGuard');
   return {hasNgTemplateContextGuard, ngTemplateGuards};
 }
 
-function nodeStaticMethodNames(node: ClassDeclaration, reflector: ReflectionHost): string[] {
-  return reflector.getMembersOfClass(node)
-      .filter(member => member.kind === ClassMemberKind.Method && member.isStatic)
-      .map(member => member.name);
+function extractTemplateGuard(member: ClassMember): TemplateGuardMeta|null {
+  if (!member.name.startsWith('ngTemplateGuard_')) {
+    return null;
+  }
+  const inputName = member.name.split('_', 2)[1];
+  if (member.kind === ClassMemberKind.Property) {
+    let type: string|null = null;
+    if (member.type !== null && ts.isLiteralTypeNode(member.type) &&
+        ts.isStringLiteral(member.type.literal)) {
+      type = member.type.literal.text;
+    }
+
+    // Only property members with string literal type 'binding' are considered as template guard.
+    if (type !== 'binding') {
+      return null;
+    }
+    return {inputName, type};
+  } else if (member.kind === ClassMemberKind.Method) {
+    return {inputName, type: 'invocation'};
+  } else {
+    return null;
+  }
 }
 
 /**

packages/compiler-cli/src/ngtsc/typecheck/src/api.ts

@@ -10,6 +10,7 @@ import {BoundTarget, DirectiveMeta} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {Reference} from '../../imports';
+import {TemplateGuardMeta} from '../../metadata';
 import {ClassDeclaration} from '../../reflection';
 
 /**
@@ -19,7 +20,7 @@ import {ClassDeclaration} from '../../reflection';
 export interface TypeCheckableDirectiveMeta extends DirectiveMeta {
   ref: Reference<ClassDeclaration>;
   queries: string[];
-  ngTemplateGuards: string[];
+  ngTemplateGuards: TemplateGuardMeta[];
   hasNgTemplateContextGuard: boolean;
 }
 

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

@@ -185,23 +185,29 @@ class TcbTemplateBodyOp extends TcbOp {
         // There are two kinds of guards. Template guards (ngTemplateGuards) allow type narrowing of
         // the expression passed to an @Input of the directive. Scan the directive to see if it has
         // any template guards, and generate them if needed.
-        dir.ngTemplateGuards.forEach(inputName => {
+        dir.ngTemplateGuards.forEach(guard => {
           // For each template guard function on the directive, look for a binding to that input.
-          const boundInput = this.template.inputs.find(i => i.name === inputName) ||
+          const boundInput = this.template.inputs.find(i => i.name === guard.inputName) ||
               this.template.templateAttrs.find(
                   (i: TmplAstTextAttribute | TmplAstBoundAttribute): i is TmplAstBoundAttribute =>
-                      i instanceof TmplAstBoundAttribute && i.name === inputName);
+                      i instanceof TmplAstBoundAttribute && i.name === guard.inputName);
           if (boundInput !== undefined) {
             // If there is such a binding, generate an expression for it.
             const expr = tcbExpression(boundInput.value, this.tcb, this.scope);
+
+            if (guard.type === 'binding') {
+              // Use the binding expression itself as guard.
+              directiveGuards.push(expr);
+            } else {
               // Call the guard function on the directive with the directive instance and that
               // expression.
-            const guardInvoke = tsCallMethod(dirId, `ngTemplateGuard_${inputName}`, [
+              const guardInvoke = tsCallMethod(dirId, `ngTemplateGuard_${guard.inputName}`, [
                 dirInstId,
                 expr,
               ]);
               directiveGuards.push(guardInvoke);
             }
+          }
         });
 
         // The second kind of guard is a template context guard. This guard narrows the template

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

@@ -82,6 +82,40 @@ describe('type check blocks', () => {
     expect(block).toContain('(ctx.a as any);');
   });
 
+  describe('template guards', () => {
+    it('should emit invocation guards', () => {
+      const DIRECTIVES: TestDeclaration[] = [{
+        type: 'directive',
+        name: 'NgIf',
+        selector: '[ngIf]',
+        inputs: {'ngIf': 'ngIf'},
+        ngTemplateGuards: [{
+          inputName: 'ngIf',
+          type: 'invocation',
+        }]
+      }];
+      const TEMPLATE = `<div *ngIf="person"></div>`;
+      const block = tcb(TEMPLATE, DIRECTIVES);
+      expect(block).toContain('if (NgIf.ngTemplateGuard_ngIf(_t1, ctx.person))');
+    });
+
+    it('should emit binding guards', () => {
+      const DIRECTIVES: TestDeclaration[] = [{
+        type: 'directive',
+        name: 'NgIf',
+        selector: '[ngIf]',
+        inputs: {'ngIf': 'ngIf'},
+        ngTemplateGuards: [{
+          inputName: 'ngIf',
+          type: 'binding',
+        }]
+      }];
+      const TEMPLATE = `<div *ngIf="person !== null"></div>`;
+      const block = tcb(TEMPLATE, DIRECTIVES);
+      expect(block).toContain('if (ctx.person !== null)');
+    });
+  });
+
   describe('config', () => {
     const DIRECTIVES: TestDeclaration[] = [{
       type: 'directive',

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

@@ -39,7 +39,7 @@ export declare class NgForOf<T> {
 
 export declare class NgIf {
   ngIf: any;
-  static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E): expr is NonNullable<E>
+  static ngTemplateGuard_ngIf: 'binding';
   static ngDirectiveDef: i0.ΔDirectiveDefWithMeta<NgForOf<any>, '[ngIf]', never, {'ngIf': 'ngIf'}, {}, never>;
 }
 
@@ -100,6 +100,29 @@ describe('ngtsc type checking', () => {
     env.driveMain();
   });
 
+  it('should check usage of NgIf with explicit non-null guard', () => {
+    env.write('test.ts', `
+    import {CommonModule} from '@angular/common';
+    import {Component, NgModule} from '@angular/core';
+
+    @Component({
+      selector: 'test',
+      template: '<div *ngIf="user !== null">{{user.name}}</div>',
+    })
+    class TestCmp {
+      user: {name: string}|null;
+    }
+
+    @NgModule({
+      declarations: [TestCmp],
+      imports: [CommonModule],
+    })
+    class Module {}
+    `);
+
+    env.driveMain();
+  });
+
   it('should check basic usage of NgFor', () => {
     env.write('test.ts', `
     import {CommonModule} from '@angular/common';

tools/public_api_guard/common/common.d.ts

@@ -263,7 +263,7 @@ export declare class NgIf {
     ngIfElse: TemplateRef<NgIfContext> | null;
     ngIfThen: TemplateRef<NgIfContext> | null;
     constructor(_viewContainer: ViewContainerRef, templateRef: TemplateRef<NgIfContext>);
-    static ngTemplateGuard_ngIf<E>(dir: NgIf, expr: E): expr is NonNullable<E>;
+    static ngTemplateGuard_ngIf: 'binding';
 }
 
 export declare class NgIfContext {