fix(ivy): do not always accept undefined for directive inputs (angu…

…lar#33066)

Prior to this change, the template type checker would always allow a
value of type `undefined` to be passed into a directive's inputs, even
if the input's type did not allow for it. This was due to how the type
constructor for a directive was generated, where a `Partial` mapped
type was used to allow for inputs to be unset. This essentially
introduces the `undefined` type as acceptable type for all inputs.

This commit removes the `Partial` type from the type constructor, which
means that we can no longer omit any properties that were unset.
Instead, any properties that are not set will still be included in the
type constructor call, having their value assigned to `any`.

Before:

```typescript
class NgForOf<T> {
  static ngTypeCtor<T>(init: Partial<Pick<NgForOf<T>,
    'ngForOf'|'ngForTrackBy'|'ngForTemplate'>>): NgForOf<T>;
}

NgForOf.ngTypeCtor(init: {ngForOf: ['foo', 'bar']});
```

After:

```typescript
class NgForOf<T> {
  static ngTypeCtor<T>(init: Pick<NgForOf<T>,
    'ngForOf'|'ngForTrackBy'|'ngForTemplate'>): NgForOf<T>;
}

NgForOf.ngTypeCtor(init: {
  ngForOf: ['foo', 'bar'],
  ngForTrackBy: null as any,
  ngForTemplate: null as any,
});
```

This change only affects generated type check code, the generated
runtime code is not affected.

Fixes angular#32690
Resolves FW-1606

PR Close angular#33066

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

@@ -123,7 +123,7 @@ export class TypeCheckContext {
     const ops = this.opMap.get(sf) !;
 
     // Push a `TypeCtorOp` into the operation queue for the source file.
-    ops.push(new TypeCtorOp(ref, ctorMeta, this.config));
+    ops.push(new TypeCtorOp(ref, ctorMeta));
   }
 
   /**
@@ -287,7 +287,7 @@ class TcbOp implements Op {
 class TypeCtorOp implements Op {
   constructor(
       readonly ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
-      readonly meta: TypeCtorMetadata, private config: TypeCheckingConfig) {}
+      readonly meta: TypeCtorMetadata) {}
 
   /**
    * Type constructor operations are inserted immediately before the end of the directive class.
@@ -296,7 +296,7 @@ class TypeCtorOp implements Op {
 
   execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer):
       string {
-    const tcb = generateInlineTypeCtor(this.ref.node, this.meta, this.config);
+    const tcb = generateInlineTypeCtor(this.ref.node, this.meta);
     return printer.printNode(ts.EmitHint.Unspecified, tcb, sf);
   }
 }

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

@@ -12,7 +12,7 @@ import * as ts from 'typescript';
 import {TypeCheckingConfig} from './api';
 import {AbsoluteSpan, addParseSpanInfo, wrapForDiagnostics} from './diagnostics';
 
-const NULL_AS_ANY =
+export const NULL_AS_ANY =
     ts.createAsExpression(ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));
 const UNDEFINED = ts.createIdentifier('undefined');
 

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

@@ -16,7 +16,7 @@ import {TypeCheckBlockMetadata, TypeCheckableDirectiveMeta} from './api';
 import {addParseSpanInfo, addSourceId, toAbsoluteSpan, wrapForDiagnostics} from './diagnostics';
 import {DomSchemaChecker} from './dom';
 import {Environment} from './environment';
-import {astToTypescript} from './expression';
+import {NULL_AS_ANY, astToTypescript} from './expression';
 import {checkIfClassIsExported, checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateVariable, tsDeclareVariable} from './ts_util';
 
 
@@ -290,11 +290,11 @@ class TcbDirectiveOp extends TcbOp {
   execute(): ts.Identifier {
     const id = this.tcb.allocateId();
     // Process the directive and construct expressions for each of its bindings.
-    const bindings = tcbGetInputBindingExpressions(this.node, this.dir, this.tcb, this.scope);
+    const inputs = tcbGetDirectiveInputs(this.node, this.dir, this.tcb, this.scope);
 
     // Call the type constructor of the directive to infer a type, and assign the directive
     // instance.
-    const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, bindings);
+    const typeCtor = tcbCallTypeCtor(this.dir, this.tcb, inputs);
     addParseSpanInfo(typeCtor, this.node.sourceSpan);
     this.scope.addStatement(tsCreateVariable(id, typeCtor));
     return id;
@@ -774,18 +774,25 @@ function tcbExpression(
  * the directive instance from any bound inputs.
  */
 function tcbCallTypeCtor(
-    dir: TypeCheckableDirectiveMeta, tcb: Context, bindings: TcbBinding[]): ts.Expression {
+    dir: TypeCheckableDirectiveMeta, tcb: Context, inputs: TcbDirectiveInput[]): ts.Expression {
   const typeCtor = tcb.env.typeCtorFor(dir);
 
-  // Construct an array of `ts.PropertyAssignment`s for each input of the directive that has a
-  // matching binding.
-  const members = bindings.map(({field, expression, sourceSpan}) => {
-    if (!tcb.env.config.checkTypeOfInputBindings) {
-      expression = tsCastToAny(expression);
+  // Construct an array of `ts.PropertyAssignment`s for each of the directive's inputs.
+  const members = inputs.map(input => {
+    if (input.type === 'binding') {
+      // For bound inputs, the property is assigned the binding expression.
+      let expression = input.expression;
+      if (!tcb.env.config.checkTypeOfInputBindings) {
+        expression = tsCastToAny(expression);
+      }
+      const assignment = ts.createPropertyAssignment(input.field, wrapForDiagnostics(expression));
+      addParseSpanInfo(assignment, input.sourceSpan);
+      return assignment;
+    } else {
+      // A type constructor is required to be called with all input properties, so any unset
+      // inputs are simply assigned a value of type `any` to ignore them.
+      return ts.createPropertyAssignment(input.field, NULL_AS_ANY);
     }
-    const assignment = ts.createPropertyAssignment(field, wrapForDiagnostics(expression));
-    addParseSpanInfo(assignment, sourceSpan);
-    return assignment;
   });
 
   // Call the `ngTypeCtor` method on the directive class, with an object literal argument created
@@ -796,17 +803,18 @@ function tcbCallTypeCtor(
       /* argumentsArray */[ts.createObjectLiteral(members)]);
 }
 
-interface TcbBinding {
+type TcbDirectiveInput = {
+  type: 'binding'; field: string; expression: ts.Expression; sourceSpan: ParseSourceSpan;
+} |
+{
+  type: 'unset';
   field: string;
-  property: string;
-  expression: ts.Expression;
-  sourceSpan: ParseSourceSpan;
-}
+};
 
-function tcbGetInputBindingExpressions(
+function tcbGetDirectiveInputs(
     el: TmplAstElement | TmplAstTemplate, dir: TypeCheckableDirectiveMeta, tcb: Context,
-    scope: Scope): TcbBinding[] {
-  const bindings: TcbBinding[] = [];
+    scope: Scope): TcbDirectiveInput[] {
+  const directiveInputs: TcbDirectiveInput[] = [];
   // `dir.inputs` is an object map of field names on the directive class to property names.
   // This is backwards from what's needed to match bindings - a map of properties to field names
   // is desired. Invert `dir.inputs` into `propMatch` to create this map.
@@ -817,24 +825,38 @@ function tcbGetInputBindingExpressions(
                                  propMatch.set(inputs[key] as string, key);
   });
 
+  // To determine which of directive's inputs are unset, we keep track of the set of field names
+  // that have not been seen yet. A field is removed from this set once a binding to it is found.
+  const unsetFields = new Set(propMatch.values());
+
   el.inputs.forEach(processAttribute);
   if (el instanceof TmplAstTemplate) {
     el.templateAttrs.forEach(processAttribute);
   }
-  return bindings;
+
+  // Add unset directive inputs for each of the remaining unset fields.
+  for (const field of unsetFields) {
+    directiveInputs.push({type: 'unset', field});
+  }
+
+  return directiveInputs;
 
   /**
    * Add a binding expression to the map for each input/template attribute of the directive that has
    * a matching binding.
    */
   function processAttribute(attr: TmplAstBoundAttribute | TmplAstTextAttribute): void {
     if (attr instanceof TmplAstBoundAttribute && propMatch.has(attr.name)) {
+      const field = propMatch.get(attr.name) !;
+
+      // Remove the field from the set of unseen fields, now that it's been assigned to.
+      unsetFields.delete(field);
+
       // Produce an expression representing the value of the binding.
       const expr = tcbExpression(attr.value, tcb, scope, attr.valueSpan || attr.sourceSpan);
-      // Call the callback.
-      bindings.push({
-        property: attr.name,
-        field: propMatch.get(attr.name) !,
+      directiveInputs.push({
+        type: 'binding',
+        field: field,
         expression: expr,
         sourceSpan: attr.sourceSpan,
       });

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

@@ -24,7 +24,7 @@ export function generateTypeCtorDeclarationFn(
       node.typeParameters !== undefined ? generateGenericArgs(node.typeParameters) : undefined;
   const rawType: ts.TypeNode = ts.createTypeReferenceNode(nodeTypeRef, rawTypeArgs);
 
-  const initParam = constructTypeCtorParameter(node, meta, rawType, config.checkQueries);
+  const initParam = constructTypeCtorParameter(node, meta, rawType);
 
   const typeParameters = typeParametersWithDefaultTypes(node.typeParameters);
 
@@ -67,12 +67,19 @@ export function generateTypeCtorDeclarationFn(
  *
  * An inline type constructor for NgFor looks like:
  *
- * static ngTypeCtor<T>(init: Partial<Pick<NgForOf<T>, 'ngForOf'|'ngForTrackBy'|'ngForTemplate'>>):
+ * static ngTypeCtor<T>(init: Pick<NgForOf<T>, 'ngForOf'|'ngForTrackBy'|'ngForTemplate'>):
  *   NgForOf<T>;
  *
  * A typical constructor would be:
  *
- * NgForOf.ngTypeCtor(init: {ngForOf: ['foo', 'bar']}); // Infers a type of NgForOf<string>.
+ * NgForOf.ngTypeCtor(init: {
+ *   ngForOf: ['foo', 'bar'],
+ *   ngForTrackBy: null as any,
+ *   ngForTemplate: null as any,
+ * }); // Infers a type of NgForOf<string>.
+ *
+ * Any inputs declared on the type for which no property binding is present are assigned a value of
+ * type `any`, to avoid producing any type errors for unset inputs.
  *
  * Inline type constructors are used when the type being created has bounded generic types which
  * make writing a declared type constructor (via `generateTypeCtorDeclarationFn`) difficult or
@@ -84,16 +91,15 @@ export function generateTypeCtorDeclarationFn(
  * @returns a `ts.MethodDeclaration` for the type constructor.
  */
 export function generateInlineTypeCtor(
-    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
-    config: TypeCheckingConfig): ts.MethodDeclaration {
+    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata): ts.MethodDeclaration {
   // Build rawType, a `ts.TypeNode` of the class with its generic parameters passed through from
   // the definition without any type bounds. For example, if the class is
   // `FooDirective<T extends Bar>`, its rawType would be `FooDirective<T>`.
   const rawTypeArgs =
       node.typeParameters !== undefined ? generateGenericArgs(node.typeParameters) : undefined;
   const rawType: ts.TypeNode = ts.createTypeReferenceNode(node.name, rawTypeArgs);
 
-  const initParam = constructTypeCtorParameter(node, meta, rawType, config.checkQueries);
+  const initParam = constructTypeCtorParameter(node, meta, rawType);
 
   // If this constructor is being generated into a .ts file, then it needs a fake body. The body
   // is set to a return of `null!`. If the type constructor is being generated into a .d.ts file,
@@ -119,27 +125,20 @@ export function generateInlineTypeCtor(
 }
 
 function constructTypeCtorParameter(
-    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata, rawType: ts.TypeNode,
-    includeQueries: boolean): ts.ParameterDeclaration {
+    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
+    rawType: ts.TypeNode): ts.ParameterDeclaration {
   // initType is the type of 'init', the single argument to the type constructor method.
-  // If the Directive has any inputs, outputs, or queries, its initType will be:
+  // If the Directive has any inputs, its initType will be:
   //
-  // Partial<Pick<rawType, 'inputField'|'outputField'|'queryField'>>
+  // Pick<rawType, 'inputA'|'inputB'>
   //
   // Pick here is used to select only those fields from which the generic type parameters of the
-  // directive will be inferred. Partial is used because inputs are optional, so there may not be
-  // bindings for each field.
+  // directive will be inferred.
   //
-  // In the special case there are no inputs/outputs/etc, initType is set to {}.
+  // In the special case there are no inputs, initType is set to {}.
   let initType: ts.TypeNode;
 
-  const keys: string[] = [
-    ...meta.fields.inputs,
-    ...meta.fields.outputs,
-  ];
-  if (includeQueries) {
-    keys.push(...meta.fields.queries);
-  }
+  const keys: string[] = meta.fields.inputs;
   if (keys.length === 0) {
     // Special case - no inputs, outputs, or other fields which could influence the result type.
     initType = ts.createTypeLiteralNode([]);
@@ -149,10 +148,7 @@ function constructTypeCtorParameter(
         keys.map(key => ts.createLiteralTypeNode(ts.createStringLiteral(key))));
 
     // Construct the Pick<rawType, keyTypeUnion>.
-    const pickType = ts.createTypeReferenceNode('Pick', [rawType, keyTypeUnion]);
-
-    // Construct the Partial<pickType>.
-    initType = ts.createTypeReferenceNode('Partial', [pickType]);
+    initType = ts.createTypeReferenceNode('Pick', [rawType, keyTypeUnion]);
   }
 
   // Create the 'init' parameter itself.
@@ -187,20 +183,20 @@ export function requiresInlineTypeCtor(node: ClassDeclaration<ts.ClassDeclaratio
  *   ngForOf: T[];
  * }
  *
- * declare function ctor<T>(o: Partial<Pick<NgFor<T>, 'ngForOf'>>): NgFor<T>;
+ * declare function ctor<T>(o: Pick<NgFor<T>, 'ngForOf'|'ngForTrackBy'|'ngForTemplate'>): NgFor<T>;
  * ```
  *
  * An invocation looks like:
  *
  * ```
- * var _t1 = ctor({ngForOf: [1, 2]});
+ * var _t1 = ctor({ngForOf: [1, 2], ngForTrackBy: null as any, ngForTemplate: null as any});
  * ```
  *
  * This correctly infers the type `NgFor<number>` for `_t1`, since `T` is inferred from the
  * assignment of type `number[]` to `ngForOf`'s type `T[]`. However, if `any` is passed instead:
  *
  * ```
- * var _t2 = ctor({ngForOf: [1, 2] as any});
+ * var _t2 = ctor({ngForOf: [1, 2] as any, ngForTrackBy: null as any, ngForTemplate: null as any});
  * ```
  *
  * then inference for `T` fails (it cannot be inferred from `T[] = any`). In this case, `T` takes
@@ -210,7 +206,7 @@ export function requiresInlineTypeCtor(node: ClassDeclaration<ts.ClassDeclaratio
  * default type will be used in the event that inference fails.
  *
  * ```
- * declare function ctor<T = any>(o: Partial<Pick<NgFor<T>, 'ngForOf'>>): NgFor<T>;
+ * declare function ctor<T = any>(o: Pick<NgFor<T>, 'ngForOf'>): NgFor<T>;
  *
  * var _t3 = ctor({ngForOf: [1, 2] as any});
  * ```

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

@@ -33,7 +33,7 @@ runInEachFileSystem(() => {
           }]);
 
       expect(messages).toEqual(
-          [`synthetic.html(1, 10): Type 'string' is not assignable to type 'number | undefined'.`]);
+          [`synthetic.html(1, 10): Type 'string' is not assignable to type 'number'.`]);
     });
 
     it('infers type of template variables', () => {

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

@@ -67,6 +67,21 @@ describe('type check blocks', () => {
     expect(tcb(TEMPLATE)).toContain('var _t2 = _t1.$implicit;');
   });
 
+  it('should handle missing property bindings', () => {
+    const TEMPLATE = `<div dir [inputA]="foo"></div>`;
+    const DIRECTIVES: TestDeclaration[] = [{
+      type: 'directive',
+      name: 'Dir',
+      selector: '[dir]',
+      inputs: {
+        fieldA: 'inputA',
+        fieldB: 'inputB',
+      },
+    }];
+    expect(tcb(TEMPLATE, DIRECTIVES))
+        .toContain('var _t2 = Dir.ngTypeCtor({ fieldA: ((ctx).foo), fieldB: (null as any) });');
+  });
+
   it('should generate a forward element reference correctly', () => {
     const TEMPLATE = `
       {{ i.value }}

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

@@ -336,12 +336,10 @@ export declare class CommonModule {
 
       const diags = env.driveDiagnostics();
       expect(diags.length).toBe(2);
-      expect(diags[0].messageText)
-          .toBe(`Type 'number' is not assignable to type 'string | undefined'.`);
+      expect(diags[0].messageText).toBe(`Type 'number' is not assignable to type 'string'.`);
       expect(diags[0].start).toEqual(386);
       expect(diags[0].length).toEqual(14);
-      expect(diags[1].messageText)
-          .toBe(`Type 'number' is not assignable to type 'boolean | undefined'.`);
+      expect(diags[1].messageText).toBe(`Type 'number' is not assignable to type 'boolean'.`);
       expect(diags[1].start).toEqual(401);
       expect(diags[1].length).toEqual(15);
     });