From eb6975acaf4b0941be718f809d76352c6455bfda Mon Sep 17 00:00:00 2001
From: Alex Rickabaugh <alx+alxhub@alxandria.net>
Date: Fri, 1 Nov 2019 12:47:14 -0700
Subject: [PATCH] fix(ivy): don't infer template context types when in full
 mode (#33537)

The Ivy template type-checker is capable of inferring the type of a
structural directive (such as NgForOf<T>). Previously, this was done with
fullTemplateTypeCheck: true, even if strictTemplates was false. View Engine
previously did not do this inference, and so this causes breakages if the
type of the template context is not what the user expected.

In particular, consider the template:

```html
<div *ngFor="let user of users as all">
  {{user.index}} out of {{all.length}}
</div>
```

As long as `users` is an array, this seems reasonable, because it appears
that `all` is an alias for the `users` array. However, this is misleading.

In reality, `NgForOf` is rendered with a template context that contains
both a `$implicit` value (for the loop variable `user`) as well as a
`ngForOf` value, which is the actual value assigned to `all`. The type of
`NgForOf`'s template context is `NgForContext<T>`, which declares `ngForOf`'s
type to be `NgIterable<T>`, which does not have a `length` property (due to
its incorporation of the `Iterable` type).

This commit stops the template type-checker from inferring template context
types unless strictTemplates is set (and strictInputTypes is not disabled).

Fixes #33527.

PR Close #33537
---
 packages/compiler-cli/src/ngtsc/program.ts    |  3 +-
 .../test/ngtsc/template_typecheck_spec.ts     | 72 +++++++++++++------
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts
index aa9c343a77..1ec33392c0 100644
--- a/packages/compiler-cli/src/ngtsc/program.ts
+++ b/packages/compiler-cli/src/ngtsc/program.ts
@@ -423,7 +423,7 @@ export class NgtscProgram implements api.Program {
     if (this.options.fullTemplateTypeCheck) {
       const strictTemplates = !!this.options.strictTemplates;
       typeCheckingConfig = {
-        applyTemplateContextGuards: true,
+        applyTemplateContextGuards: strictTemplates,
         checkQueries: false,
         checkTemplateBodies: true,
         checkTypeOfInputBindings: strictTemplates,
@@ -468,6 +468,7 @@ export class NgtscProgram implements api.Program {
     // based on "fullTemplateTypeCheck".
     if (this.options.strictInputTypes !== undefined) {
       typeCheckingConfig.checkTypeOfInputBindings = this.options.strictInputTypes;
+      typeCheckingConfig.applyTemplateContextGuards = this.options.strictInputTypes;
     }
     if (this.options.strictNullInputTypes !== undefined) {
       typeCheckingConfig.strictNullInputBindings = this.options.strictNullInputTypes;
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
index d583fc1e0d..1f41873e0b 100644
--- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -43,6 +43,16 @@ export declare class IndexPipe {
   static ɵpipe: i0.ɵPipeDefWithMeta<IndexPipe, 'index'>;
 }
 
+export declare class SlicePipe {
+  transform<T>(value: ReadonlyArray<T>, start: number, end?: number): Array<T>;
+  transform(value: string, start: number, end?: number): string;
+  transform(value: null, start: number, end?: number): null;
+  transform(value: undefined, start: number, end?: number): undefined;
+  transform(value: any, start: number, end?: number): any;
+
+  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>;
@@ -56,7 +66,7 @@ export declare class NgIf {
 }
 
 export declare class CommonModule {
-  static ɵmod: i0.ɵɵNgModuleDefWithMeta<CommonModule, [typeof NgIf, typeof NgForOf, typeof IndexPipe], never, [typeof NgIf, typeof NgForOf, typeof IndexPipe]>;
+  static ɵmod: i0.ɵɵNgModuleDefWithMeta<CommonModule, [typeof NgIf, typeof NgForOf, typeof IndexPipe, typeof SlicePipe], never, [typeof NgIf, typeof NgForOf, typeof IndexPipe, typeof SlicePipe]>;
 }
 `);
       env.write('node_modules/@angular/animations/index.d.ts', `
@@ -862,30 +872,48 @@ export declare class AnimationEvent {
       expect(diags[0].length).toBe(19);
     });
 
-    it('should property type-check a microsyntax variable with the same name as the expression',
-       () => {
-         env.write('test.ts', `
-    import {CommonModule} from '@angular/common';
-    import {Component, Input, NgModule} from '@angular/core';
+    describe('microsyntax variables', () => {
+      beforeEach(() => {
+        // Use the same template for both tests
+        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}}
+            </div>
+            \`,
+          })
+          export class TestCmp {
+            foos: {name: string}[];
+          }
+      
+          @NgModule({
+            declarations: [TestCmp],
+            imports: [CommonModule],
+          })
+          export class Module {}
+        `);
+      });
 
-    @Component({
-      selector: 'test',
-      template: '<div *ngIf="foo as foo">{{foo}}</div>',
-    })
-    export class TestCmp<T extends {name: string}> {
-      foo: any;
-    }
+      it('should be treated as \'any\' without strictTemplates', () => {
+        env.tsconfig({fullTemplateTypeCheck: true, strictTemplates: false});
 
-    @NgModule({
-      declarations: [TestCmp],
-      imports: [CommonModule],
-    })
-    export class Module {}
-    `);
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(0);
+      });
 
-         const diags = env.driveDiagnostics();
-         expect(diags.length).toBe(0);
-       });
+      it('should be correctly inferred under strictTemplates', () => {
+        env.tsconfig({fullTemplateTypeCheck: true, strictTemplates: true});
+
+        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; }>'.`);
+      });
+    });
 
     it('should properly type-check inherited directives', () => {
       env.tsconfig({fullTemplateTypeCheck: true, strictInputTypes: true});