From 7525f3afc114ba65c72bb98818d83c7c34d49042 Mon Sep 17 00:00:00 2001
From: JoostK <joost.koehoorn@gmail.com>
Date: Tue, 28 Jul 2020 20:16:57 +0200
Subject: [PATCH] fix(compiler-cli): type-check inputs that include undefined
 when there's coercion members (#38273)

For attribute bindings that target a directive's input, the template
type checker is able to verify that the type of the input expression is
compatible with the directive's declaration for said input. This
checking adheres to the `strictNullChecks` flag as configured in the
TypeScript compilation, such that errors are reported for expressions
that include `undefined` or `null` in their type if the input's
declaration does not include those types.

There was a bug with this level of type-checking for directives that
also declare coercion members, where binding an expression that includes
the `undefined` type to a directive's input that does not include the
`undefined` type would not be reported as error.

This commit fixes the bug by changing the type-constructor in type-check
code to use an intersection type of regular inputs and coerced inputs,
instead of a union type. The union type would inadvertently allow
`undefined` types to be assigned into the regular inputs, as that would
still satisfy the characteristics of a union type.

As a result of this change, you may start to see build failures if
`strictTemplates` is enabled and `strictInputTypes` is not disabled.
These errors are legitimate and some action is required to achieve a
successful build:

1. Update the templates for which an error is reported and introduce the
   non-null assertion operator at the end of the expression. This
   removes the `undefined` type from the expression's type, making it
   appear as a valid assignment.
2. Disable `strictNullInputTypes` in the compiler options. This will
   implicitly add the non-null assertion operators similar to option 1,
   but all templates in the compilation are affected.
3. Update the directive's input declaration to include the `undefined`
   type, if the directive is not implemented in an external library.

PR Close #38273
---
 .../ngtsc/typecheck/src/type_constructor.ts   |  4 +--
 .../typecheck/test/type_constructor_spec.ts   |  2 +-
 .../test/ngtsc/template_typecheck_spec.ts     | 33 +++++++++++++++++++
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts
index 139ec6a232..f80fd0fc69 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts
@@ -166,8 +166,8 @@ function constructTypeCtorParameter(
   if (coercedKeys.length > 0) {
     const coercedLiteral = ts.createTypeLiteralNode(coercedKeys);
 
-    initType =
-        initType !== null ? ts.createUnionTypeNode([initType, coercedLiteral]) : coercedLiteral;
+    initType = initType !== null ? ts.createIntersectionTypeNode([initType, coercedLiteral]) :
+                                   coercedLiteral;
   }
 
   if (initType === null) {
diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts
index 54c399f1ba..f490cda7f5 100644
--- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts
+++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts
@@ -179,7 +179,7 @@ TestClass.ngTypeCtor({value: 'test'});
         const typeCtor = TestClassWithCtor.members.find(isTypeCtor)!;
         const ctorText = typeCtor.getText().replace(/[ \r\n]+/g, ' ');
         expect(ctorText).toContain(
-            'init: Pick<TestClass, "foo"> | { bar: typeof TestClass.ngAcceptInputType_bar; }');
+            'init: Pick<TestClass, "foo"> & { bar: typeof TestClass.ngAcceptInputType_bar; }');
       });
     });
   });
diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
index 377179e736..7c9ec4dbf4 100644
--- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
+++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
@@ -1498,6 +1498,39 @@ export declare class AnimationEvent {
            expect(diags[0].messageText)
                .toBe(`Type 'boolean' is not assignable to type 'string | number'.`);
          });
+
+      it('should give an error for undefined bindings into regular inputs when coercion members are present',
+         () => {
+           env.tsconfig({strictTemplates: true});
+           env.write('test.ts', `
+            import {Component, Directive, NgModule, Input} from '@angular/core';
+
+            @Component({
+              selector: 'blah',
+              template: '<input dir [regular]="undefined" [coerced]="1">',
+            })
+            export class FooCmp {
+              invalidType = true;
+            }
+
+            @Directive({selector: '[dir]'})
+            export class CoercionDir {
+              @Input() regular: string;
+              @Input() coerced: boolean;
+
+              static ngAcceptInputType_coerced: boolean|number;
+            }
+
+            @NgModule({
+              declarations: [FooCmp, CoercionDir],
+            })
+            export class FooModule {}
+        `);
+           const diags = env.driveDiagnostics();
+           expect(diags.length).toBe(1);
+           expect(diags[0].messageText)
+               .toBe(`Type 'undefined' is not assignable to type 'string'.`);
+         });
     });
 
     describe('legacy schema checking with the DOM schema', () => {