From a6b6d74c006cebb24f43593e6400782434266629 Mon Sep 17 00:00:00 2001
From: crisbeto <crisbeto@abv.bg>
Date: Thu, 28 Nov 2019 19:42:00 +0100
Subject: [PATCH] fix(common): reflect input type in NgForOf context (#33997)

Fixes `NgForOf` not reflecting the type of its input in the `NgForOfContext`.

PR Close #33997
---
 packages/common/src/directives/ng_for_of.ts   |  41 ++++---
 .../test/ngtsc/template_typecheck_spec.ts     | 100 ++++++++++++------
 packages/core/test/render3/common_with_def.ts |   4 +-
 .../core/test/render3/instructions_spec.ts    |   5 +-
 tools/public_api_guard/common/common.d.ts     |  16 +--
 5 files changed, 98 insertions(+), 68 deletions(-)

diff --git a/packages/common/src/directives/ng_for_of.ts b/packages/common/src/directives/ng_for_of.ts
index 680e283d2c..c3d8e1c192 100644
--- a/packages/common/src/directives/ng_for_of.ts
+++ b/packages/common/src/directives/ng_for_of.ts
@@ -6,15 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, DoCheck, EmbeddedViewRef, Input, IterableChangeRecord, IterableChanges, IterableDiffer, IterableDiffers, NgIterable, TemplateRef, TrackByFunction, ViewContainerRef, forwardRef, isDevMode} from '@angular/core';
+import {Directive, DoCheck, EmbeddedViewRef, Input, IterableChangeRecord, IterableChanges, IterableDiffer, IterableDiffers, NgIterable, TemplateRef, TrackByFunction, ViewContainerRef, isDevMode} from '@angular/core';
 
 /**
  * @publicApi
  */
-export class NgForOfContext<T> {
-  constructor(
-      public $implicit: T, public ngForOf: NgIterable<T>, public index: number,
-      public count: number) {}
+export class NgForOfContext<T, U extends NgIterable<T>> {
+  constructor(public $implicit: T, public ngForOf: U, public index: number, public count: number) {}
 
   get first(): boolean { return this.index === 0; }
 
@@ -123,13 +121,13 @@ export class NgForOfContext<T> {
  * @publicApi
  */
 @Directive({selector: '[ngFor][ngForOf]'})
-export class NgForOf<T> implements DoCheck {
+export class NgForOf<T, U extends NgIterable<T>> implements DoCheck {
   /**
    * The value of the iterable expression, which can be used as a
    * [template input variable](guide/structural-directives#template-input-variable).
    */
   @Input()
-  set ngForOf(ngForOf: NgIterable<T>|undefined|null) {
+  set ngForOf(ngForOf: U&NgIterable<T>|undefined|null) {
     this._ngForOf = ngForOf;
     this._ngForOfDirty = true;
   }
@@ -165,22 +163,22 @@ export class NgForOf<T> implements DoCheck {
 
   get ngForTrackBy(): TrackByFunction<T> { return this._trackByFn; }
 
-  private _ngForOf: NgIterable<T>|undefined|null = null;
+  private _ngForOf: U|undefined|null = null;
   private _ngForOfDirty: boolean = true;
   private _differ: IterableDiffer<T>|null = null;
   // TODO(issue/24571): remove '!'.
   private _trackByFn !: TrackByFunction<T>;
 
   constructor(
-      private _viewContainer: ViewContainerRef, private _template: TemplateRef<NgForOfContext<T>>,
-      private _differs: IterableDiffers) {}
+      private _viewContainer: ViewContainerRef,
+      private _template: TemplateRef<NgForOfContext<T, U>>, private _differs: IterableDiffers) {}
 
   /**
    * A reference to the template that is stamped out for each item in the iterable.
    * @see [template reference variable](guide/template-syntax#template-reference-variables--var-)
    */
   @Input()
-  set ngForTemplate(value: TemplateRef<NgForOfContext<T>>) {
+  set ngForTemplate(value: TemplateRef<NgForOfContext<T, U>>) {
     // TODO(TS2.1): make TemplateRef<Partial<NgForRowOf<T>>> once we move to TS v2.1
     // The current type is too restrictive; a template that just uses index, for example,
     // should be acceptable.
@@ -213,7 +211,7 @@ export class NgForOf<T> implements DoCheck {
   }
 
   private _applyChanges(changes: IterableChanges<T>) {
-    const insertTuples: RecordViewTuple<T>[] = [];
+    const insertTuples: RecordViewTuple<T, U>[] = [];
     changes.forEachOperation(
         (item: IterableChangeRecord<any>, adjustedPreviousIndex: number | null,
          currentIndex: number | null) => {
@@ -222,9 +220,9 @@ export class NgForOf<T> implements DoCheck {
             // that a new item needs to be inserted from the iterable. This implies that
             // there is an iterable value for "_ngForOf".
             const view = this._viewContainer.createEmbeddedView(
-                this._template, new NgForOfContext<T>(null !, this._ngForOf !, -1, -1),
+                this._template, new NgForOfContext<T, U>(null !, this._ngForOf !, -1, -1),
                 currentIndex === null ? undefined : currentIndex);
-            const tuple = new RecordViewTuple<T>(item, view);
+            const tuple = new RecordViewTuple<T, U>(item, view);
             insertTuples.push(tuple);
           } else if (currentIndex == null) {
             this._viewContainer.remove(
@@ -232,7 +230,7 @@ export class NgForOf<T> implements DoCheck {
           } else if (adjustedPreviousIndex !== null) {
             const view = this._viewContainer.get(adjustedPreviousIndex) !;
             this._viewContainer.move(view, currentIndex);
-            const tuple = new RecordViewTuple(item, <EmbeddedViewRef<NgForOfContext<T>>>view);
+            const tuple = new RecordViewTuple(item, <EmbeddedViewRef<NgForOfContext<T, U>>>view);
             insertTuples.push(tuple);
           }
         });
@@ -242,7 +240,7 @@ export class NgForOf<T> implements DoCheck {
     }
 
     for (let i = 0, ilen = this._viewContainer.length; i < ilen; i++) {
-      const viewRef = <EmbeddedViewRef<NgForOfContext<T>>>this._viewContainer.get(i);
+      const viewRef = <EmbeddedViewRef<NgForOfContext<T, U>>>this._viewContainer.get(i);
       viewRef.context.index = i;
       viewRef.context.count = ilen;
       viewRef.context.ngForOf = this._ngForOf !;
@@ -250,13 +248,13 @@ export class NgForOf<T> implements DoCheck {
 
     changes.forEachIdentityChange((record: any) => {
       const viewRef =
-          <EmbeddedViewRef<NgForOfContext<T>>>this._viewContainer.get(record.currentIndex);
+          <EmbeddedViewRef<NgForOfContext<T, U>>>this._viewContainer.get(record.currentIndex);
       viewRef.context.$implicit = record.item;
     });
   }
 
   private _perViewChange(
-      view: EmbeddedViewRef<NgForOfContext<T>>, record: IterableChangeRecord<any>) {
+      view: EmbeddedViewRef<NgForOfContext<T, U>>, record: IterableChangeRecord<any>) {
     view.context.$implicit = record.item;
   }
 
@@ -266,13 +264,14 @@ export class NgForOf<T> implements DoCheck {
    * The presence of this method is a signal to the Ivy template type-check compiler that the
    * `NgForOf` structural directive renders its template with a specific context type.
    */
-  static ngTemplateContextGuard<T>(dir: NgForOf<T>, ctx: any): ctx is NgForOfContext<T> {
+  static ngTemplateContextGuard<T, U extends NgIterable<T>>(dir: NgForOf<T, U>, ctx: any):
+      ctx is NgForOfContext<T, U> {
     return true;
   }
 }
 
-class RecordViewTuple<T> {
-  constructor(public record: any, public view: EmbeddedViewRef<NgForOfContext<T>>) {}
+class RecordViewTuple<T, U extends NgIterable<T>> {
+  constructor(public record: any, public view: EmbeddedViewRef<NgForOfContext<T, U>>) {}
 }
 
 function getTypeName(type: any): string {
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
index d102c204b4..b35ea5cc05 100644
--- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -26,15 +26,16 @@ runInEachFileSystem(() => {
       env.write('node_modules/@angular/common/index.d.ts', `
 import * as i0 from '@angular/core';
 
-export declare class NgForOfContext<T> {
+export declare class NgForOfContext<T, U extends NgIterable<T>> {
   $implicit: T;
-  ngForOf: i0.NgIterable<T>;
-  index: number;
   count: number;
-  readonly first: boolean;
-  readonly last: boolean;
   readonly even: boolean;
+  readonly first: boolean;
+  index: number;
+  readonly last: boolean;
+  ngForOf: U;
   readonly odd: boolean;
+  constructor($implicit: T, ngForOf: U, index: number, count: number);
 }
 
 export declare class IndexPipe {
@@ -53,9 +54,13 @@ export declare class SlicePipe {
   static ɵpipe: i0.ɵPipeDefWithMeta<SlicePipe, 'slice'>;
 }
 
-export declare class NgForOf<T> {
-  ngForOf: i0.NgIterable<T>;
-  static ngTemplateContextGuard<T>(dir: NgForOf<T>, ctx: any): ctx is NgForOfContext<T>;
+export declare class NgForOf<T, U extends i0.NgIterable<T>> implements DoCheck {
+  ngForOf: (U & i0.NgIterable<T>) | undefined | null;
+  ngForTemplate: TemplateRef<NgForOfContext<T, U>>;
+  ngForTrackBy: TrackByFunction<T>;
+  constructor(_viewContainer: ViewContainerRef, _template: TemplateRef<NgForOfContext<T, U>>, _differs: IterableDiffers);
+  ngDoCheck(): void;
+  static ngTemplateContextGuard<T, U extends i0.NgIterable<T>>(dir: NgForOf<T, U>, ctx: any): ctx is NgForOfContext<T, U>;
   static ɵdir: i0.ɵɵDirectiveDefWithMeta<NgForOf<any>, '[ngFor][ngForOf]', never, {'ngForOf': 'ngForOf'}, {}, never>;
 }
 
@@ -100,18 +105,18 @@ export declare class AnimationEvent {
           {fullTemplateTypeCheck: true, strictInputTypes: true, strictAttributeTypes: true});
       env.write('test.ts', `
         import {Component, Directive, NgModule, Input} from '@angular/core';
-    
+
         @Component({
           selector: 'test',
           template: '<div dir foo="2"></div>',
         })
         class TestCmp {}
-    
+
         @Directive({selector: '[dir]'})
         class TestDir {
           @Input() foo: number;
         }
-    
+
         @NgModule({
           declarations: [TestCmp, TestDir],
         })
@@ -128,7 +133,7 @@ export declare class AnimationEvent {
           {fullTemplateTypeCheck: true, strictInputTypes: true, strictOutputEventTypes: true});
       env.write('test.ts', `
         import {Component, Directive, NgModule, EventEmitter} from '@angular/core';
-    
+
         @Component({
           selector: 'test',
           template: '<div dir [some-input.xs]="2" (some-output)="handleEvent($event)"></div>',
@@ -136,7 +141,7 @@ export declare class AnimationEvent {
         class TestCmp {
           handleEvent(event: number): void {}
         }
-    
+
         @Directive({
           selector: '[dir]',
           inputs: ['some-input.xs'],
@@ -146,7 +151,7 @@ export declare class AnimationEvent {
           'some-input.xs': string;
           'some-output': EventEmitter<string>;
         }
-    
+
         @NgModule({
           declarations: [TestCmp, TestDir],
         })
@@ -164,7 +169,7 @@ export declare class AnimationEvent {
       env.tsconfig({fullTemplateTypeCheck: true, strictOutputEventTypes: true});
       env.write('test.ts', `
         import {Component, Directive, EventEmitter, NgModule, Output} from '@angular/core';
-    
+
         @Component({
           selector: 'test',
           template: '<div dir (update)="update($event); updated = true" (focus)="update($event); focused = true"></div>',
@@ -172,12 +177,12 @@ export declare class AnimationEvent {
         class TestCmp {
           update(data: string) {}
         }
-    
+
         @Directive({selector: '[dir]'})
         class TestDir {
           @Output() update = new EventEmitter<number>();
         }
-    
+
         @NgModule({
           declarations: [TestCmp, TestDir],
         })
@@ -589,7 +594,7 @@ export declare class AnimationEvent {
       beforeEach(() => {
         env.write('test.ts', `
           import {Component, NgModule} from '@angular/core';
-      
+
           @Component({
             selector: 'test',
             template: '<div (focus)="invalid; update($event)"></div>',
@@ -597,7 +602,7 @@ export declare class AnimationEvent {
           class TestCmp {
             update(data: string) {}
           }
-   
+
           @NgModule({
             declarations: [TestCmp],
           })
@@ -785,6 +790,31 @@ export declare class AnimationEvent {
       env.driveMain();
     });
 
+    it('should infer the context of NgFor', () => {
+      env.tsconfig({fullTemplateTypeCheck: true, strictTemplates: true});
+      env.write('test.ts', `
+        import {CommonModule} from '@angular/common';
+        import {Component, NgModule} from '@angular/core';
+
+        @Component({
+          selector: 'test',
+          template: '<div *ngFor="let user of users as all">{{all.length}}</div>',
+        })
+        class TestCmp {
+          users: {name: string}[];
+        }
+
+        @NgModule({
+          declarations: [TestCmp],
+          imports: [CommonModule],
+        })
+        class Module {}
+      `);
+
+      const diags = env.driveDiagnostics();
+      expect(diags.length).toBe(0);
+    });
+
     it('should report an error with an unknown local ref target', () => {
       env.write('test.ts', `
         import {Component, NgModule} from '@angular/core';
@@ -915,18 +945,18 @@ export declare class AnimationEvent {
         env.write('test.ts', `
           import {CommonModule} from '@angular/common';
           import {Component, NgModule} from '@angular/core';
-      
+
           @Component({
             selector: 'test',
             template: \`<div *ngFor="let foo of foos as foos">
-              {{foo.name}} of {{foos.length}}
+              {{foo.name}} of {{foos.nonExistingProp}}
             </div>
             \`,
           })
           export class TestCmp {
             foos: {name: string}[];
           }
-      
+
           @NgModule({
             declarations: [TestCmp],
             imports: [CommonModule],
@@ -947,8 +977,8 @@ export declare class AnimationEvent {
 
         const diags = env.driveDiagnostics();
         expect(diags.length).toBe(1);
-        expect((diags[0].messageText as ts.DiagnosticMessageChain).messageText)
-            .toBe(`Property 'length' does not exist on type 'NgIterable<{ name: string; }>'.`);
+        expect(diags[0].messageText)
+            .toBe(`Property 'nonExistingProp' does not exist on type '{ name: string; }[]'.`);
       });
     });
 
@@ -1015,7 +1045,7 @@ export declare class AnimationEvent {
 
           static ɵdir: i0.ɵɵDirectiveDefWithMeta<BaseDir, '[base]', never, {'fromBase': 'fromBase'}, never, never>;
         }
-        
+
         export declare class ExternalModule {
           static ɵmod: i0.ɵɵNgModuleDefWithMeta<ExternalModule, [typeof BaseDir], never, [typeof BaseDir]>;
         }
@@ -1024,20 +1054,20 @@ export declare class AnimationEvent {
       env.write('test.ts', `
         import {Component, Directive, Input, NgModule} from '@angular/core';
         import {BaseDir, ExternalModule} from 'external';
-    
+
         @Directive({
           selector: '[child]',
         })
         class ChildDir extends BaseDir {
           @Input() fromChild!: boolean;
         }
-    
+
         @Component({
           selector: 'test',
           template: '<div child [fromAbstract]="true" [fromBase]="3" [fromChild]="4"></div>',
         })
         class TestCmp {}
-    
+
         @NgModule({
           declarations: [TestCmp, ChildDir],
           imports: [ExternalModule],
@@ -1260,13 +1290,13 @@ export declare class AnimationEvent {
          () => {
            env.write('test.ts', `
             import {Component, NgModule, CUSTOM_ELEMENTS_SCHEMA} from '@angular/core';
-      
+
             @Component({
               selector: 'blah',
               template: '<custom-element [foo]="1">test</custom-element>',
             })
             export class FooCmp {}
-      
+
             @NgModule({
               declarations: [FooCmp],
               schemas: [CUSTOM_ELEMENTS_SCHEMA],
@@ -1280,13 +1310,13 @@ export declare class AnimationEvent {
       it('should not produce diagnostics when using the NO_ERRORS_SCHEMA', () => {
         env.write('test.ts', `
         import {Component, NgModule, NO_ERRORS_SCHEMA} from '@angular/core';
-  
+
         @Component({
           selector: 'blah',
           template: '<foo [bar]="1"></foo>',
         })
         export class FooCmp {}
-  
+
         @NgModule({
           declarations: [FooCmp],
           schemas: [NO_ERRORS_SCHEMA],
@@ -1314,7 +1344,7 @@ export declare class AnimationEvent {
         it('should be correct for direct templates', async() => {
           env.write('test.ts', `
           import {Component, NgModule} from '@angular/core';
-      
+
           @Component({
             selector: 'test',
             template: \`<p>
@@ -1334,7 +1364,7 @@ export declare class AnimationEvent {
         it('should be correct for indirect templates', async() => {
           env.write('test.ts', `
           import {Component, NgModule} from '@angular/core';
-      
+
           const TEMPLATE = \`<p>
             {{user.does_not_exist}}
           </p>\`;
@@ -1360,7 +1390,7 @@ export declare class AnimationEvent {
         </p>`);
           env.write('test.ts', `
           import {Component, NgModule} from '@angular/core';
-      
+
 
           @Component({
             selector: 'test',
diff --git a/packages/core/test/render3/common_with_def.ts b/packages/core/test/render3/common_with_def.ts
index 502c22b5d5..0ebf12405a 100644
--- a/packages/core/test/render3/common_with_def.ts
+++ b/packages/core/test/render3/common_with_def.ts
@@ -7,11 +7,11 @@
  */
 
 import {NgForOf as NgForOfDef, NgIf as NgIfDef, NgTemplateOutlet as NgTemplateOutletDef} from '@angular/common';
-import {IterableDiffers, TemplateRef, ViewContainerRef} from '@angular/core';
+import {IterableDiffers, NgIterable, TemplateRef, ViewContainerRef} from '@angular/core';
 
 import {DirectiveType, ɵɵNgOnChangesFeature, ɵɵdefineDirective, ɵɵdirectiveInject} from '../../src/render3/index';
 
-export const NgForOf: DirectiveType<NgForOfDef<any>> = NgForOfDef as any;
+export const NgForOf: DirectiveType<NgForOfDef<any, NgIterable<any>>> = NgForOfDef as any;
 export const NgIf: DirectiveType<NgIfDef> = NgIfDef as any;
 export const NgTemplateOutlet: DirectiveType<NgTemplateOutletDef> = NgTemplateOutletDef as any;
 
diff --git a/packages/core/test/render3/instructions_spec.ts b/packages/core/test/render3/instructions_spec.ts
index fbe13c2534..be6f6b5fa2 100644
--- a/packages/core/test/render3/instructions_spec.ts
+++ b/packages/core/test/render3/instructions_spec.ts
@@ -205,7 +205,8 @@ describe('instructions', () => {
 
   describe('performance counters', () => {
     it('should create tViews only once for each nested level', () => {
-      function ToDoAppComponent_NgForOf_Template_0(rf: RenderFlags, ctx0: NgForOfContext<any>) {
+      function ToDoAppComponent_NgForOf_Template_0(
+          rf: RenderFlags, ctx0: NgForOfContext<any, any>) {
         if (rf & RenderFlags.Create) {
           ɵɵelementStart(0, 'ul');
           ɵɵtemplate(1, ToDoAppComponent_NgForOf_NgForOf_Template_1, 2, 1, 'li', 0);
@@ -219,7 +220,7 @@ describe('instructions', () => {
       }
 
       function ToDoAppComponent_NgForOf_NgForOf_Template_1(
-          rf: RenderFlags, ctx1: NgForOfContext<any>) {
+          rf: RenderFlags, ctx1: NgForOfContext<any, any>) {
         if (rf & RenderFlags.Create) {
           ɵɵelementStart(0, 'li');
           ɵɵtext(1);
diff --git a/tools/public_api_guard/common/common.d.ts b/tools/public_api_guard/common/common.d.ts
index d12af28e6f..2c71a5d748 100644
--- a/tools/public_api_guard/common/common.d.ts
+++ b/tools/public_api_guard/common/common.d.ts
@@ -214,25 +214,25 @@ export declare class NgComponentOutlet implements OnChanges, OnDestroy {
     ngOnDestroy(): void;
 }
 
-export declare class NgForOf<T> implements DoCheck {
-    ngForOf: NgIterable<T> | undefined | null;
-    ngForTemplate: TemplateRef<NgForOfContext<T>>;
+export declare class NgForOf<T, U extends NgIterable<T>> implements DoCheck {
+    ngForOf: (U & NgIterable<T>) | undefined | null;
+    ngForTemplate: TemplateRef<NgForOfContext<T, U>>;
     ngForTrackBy: TrackByFunction<T>;
-    constructor(_viewContainer: ViewContainerRef, _template: TemplateRef<NgForOfContext<T>>, _differs: IterableDiffers);
+    constructor(_viewContainer: ViewContainerRef, _template: TemplateRef<NgForOfContext<T, U>>, _differs: IterableDiffers);
     ngDoCheck(): void;
-    static ngTemplateContextGuard<T>(dir: NgForOf<T>, ctx: any): ctx is NgForOfContext<T>;
+    static ngTemplateContextGuard<T, U extends NgIterable<T>>(dir: NgForOf<T, U>, ctx: any): ctx is NgForOfContext<T, U>;
 }
 
-export declare class NgForOfContext<T> {
+export declare class NgForOfContext<T, U extends NgIterable<T>> {
     $implicit: T;
     count: number;
     readonly even: boolean;
     readonly first: boolean;
     index: number;
     readonly last: boolean;
-    ngForOf: NgIterable<T>;
+    ngForOf: U;
     readonly odd: boolean;
-    constructor($implicit: T, ngForOf: NgIterable<T>, index: number, count: number);
+    constructor($implicit: T, ngForOf: U, index: number, count: number);
 }
 
 export declare class NgIf {