feat(compiler-cli): support type-checking for generic signal inputs (#…

…53521)

This commit adds the last remaining piece for signal input
type-checking. Bound values to signal inputs are already checked
properly at this point, but inference of generic directive/component
types through their inputs is not implemented.

This commit fixes this. To achieve this, there are a couple of potential
solutions. The generics of a directive are inferred based on input
value expressions using a so-called type constructor. The constructor
looks something like this:

```
const _ctor = <T>(v: Pick<Dir<T>, 'input1', 'input2'>) => Dir<T>;

_ctor({input1: expr1, input2: expr2});
```

This works very well for non-signal inputs where the class member is
directly holding the input values. For signal inputs, this does NOT
work because the class member will actually hold the `InputSignal`
instance. There are a couple of solutions to this:

1. Calling `_ctor` with an `InputSignal<typeof value>`
2. Converting the `_ctor` input signal fields to their write types
   (unwrapping the input signals).

We've decided to go with the second option as TypeScript is very
sensitive with assignments and its checks. i.e. co-variance,
contravariance or bivariance. Semantically it makes more sense to unwrap
the input signal "write type" directly and "assign to it". This is safer
and conceptually also easier to follow. A type constructor continues to
only receive the "expresison values". This simplifies code as well.

It's worth noting that the unwrapping as per option 2 also comes at a
cost. We need to be able to generate imports in type constructors. This
was not possible until the previous commit because inline type constructors
did not have an associated type-check block `Environment` and we were
missing access to expression translation and correct import generation.

Overall, solution 2 is now implemented as works as expected. This commit
adds additional unit tests to ensure this.

PR Close #53521

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

@@ -22,6 +22,7 @@ import {makeTemplateDiagnostic} from '../diagnostics';
 import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';
+import {ReferenceEmitEnvironment} from './reference_emit_environment';
 import {TypeCheckShimGenerator} from './shim';
 import {TemplateSourceManager} from './source';
 import {requiresInlineTypeCheckBlock, TcbInliningRequirement} from './tcb_util';
@@ -338,7 +339,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {
     const ops = this.opMap.get(sf)!;
 
     // Push a `TypeCtorOp` into the operation queue for the source file.
-    ops.push(new TypeCtorOp(ref, ctorMeta));
+    ops.push(new TypeCtorOp(ref, this.reflector, ctorMeta));
     fileData.hasInlines = true;
   }
 
@@ -550,7 +551,7 @@ class InlineTcbOp implements Op {
 class TypeCtorOp implements Op {
   constructor(
       readonly ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
-      readonly meta: TypeCtorMetadata) {}
+      readonly reflector: ReflectionHost, readonly meta: TypeCtorMetadata) {}
 
   /**
    * Type constructor operations are inserted immediately before the end of the directive class.
@@ -561,7 +562,8 @@ class TypeCtorOp implements Op {
 
   execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer):
       string {
-    const tcb = generateInlineTypeCtor(this.ref.node, this.meta);
+    const emitEnv = new ReferenceEmitEnvironment(im, refEmitter, this.reflector, sf);
+    const tcb = generateInlineTypeCtor(emitEnv, this.ref.node, this.meta);
     return printer.printNode(ts.EmitHint.Unspecified, tcb, sf);
   }
 }

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

@@ -84,7 +84,7 @@ export class Environment extends ReferenceEmitEnvironment {
         coercedInputFields: dir.coercedInputFields,
       };
       const typeParams = this.emitTypeParameters(node);
-      const typeCtor = generateTypeCtorDeclarationFn(node, meta, nodeTypeRef.typeName, typeParams);
+      const typeCtor = generateTypeCtorDeclarationFn(this, meta, nodeTypeRef.typeName, typeParams);
       this.typeCtorStatements.push(typeCtor);
       const fnId = ts.factory.createIdentifier(fnName);
       this.typeCtors.set(node, fnId);

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

@@ -650,9 +650,13 @@ class TcbDirectiveCtorOp extends TcbOp {
         }
 
         const expression = translateInput(attr.attribute, this.tcb, this.scope);
-        genericInputs.set(
-            fieldName,
-            {type: 'binding', field: fieldName, expression, sourceSpan: attr.attribute.sourceSpan});
+
+        genericInputs.set(fieldName, {
+          type: 'binding',
+          field: fieldName,
+          expression,
+          sourceSpan: attr.attribute.sourceSpan,
+        });
       }
     }
 
@@ -2412,7 +2416,6 @@ function tcbCallTypeCtor(
     if (input.type === 'binding') {
       // For bound inputs, the property is assigned the binding expression.
       const expr = widenBinding(input.expression, tcb);
-
       const assignment =
           ts.factory.createPropertyAssignment(propertyName, wrapForDiagnostics(expr));
       addParseSpanInfo(assignment, input.sourceSpan);

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

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {ExpressionType, R3Identifiers, WrappedNodeExpr} from '@angular/compiler';
 import ts from 'typescript';
 
 import {ClassDeclaration, ReflectionHost} from '../../reflection';
@@ -16,12 +17,12 @@ import {checkIfGenericTypeBoundsCanBeEmitted} from './tcb_util';
 import {tsCreateTypeQueryForCoercedInput} from './ts_util';
 
 export function generateTypeCtorDeclarationFn(
-    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata, nodeTypeRef: ts.EntityName,
+    env: ReferenceEmitEnvironment, meta: TypeCtorMetadata, nodeTypeRef: ts.EntityName,
     typeParams: ts.TypeParameterDeclaration[]|undefined): ts.Statement {
   const rawTypeArgs = typeParams !== undefined ? generateGenericArgs(typeParams) : undefined;
   const rawType = ts.factory.createTypeReferenceNode(nodeTypeRef, rawTypeArgs);
 
-  const initParam = constructTypeCtorParameter(node, meta, rawType);
+  const initParam = constructTypeCtorParameter(env, meta, rawType);
 
   const typeParameters = typeParametersWithDefaultTypes(typeParams);
 
@@ -89,15 +90,16 @@ export function generateTypeCtorDeclarationFn(
  * @returns a `ts.MethodDeclaration` for the type constructor.
  */
 export function generateInlineTypeCtor(
-    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata): ts.MethodDeclaration {
+    env: ReferenceEmitEnvironment, 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.factory.createTypeReferenceNode(node.name, rawTypeArgs);
 
-  const initParam = constructTypeCtorParameter(node, meta, rawType);
+  const initParam = constructTypeCtorParameter(env, 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,
@@ -123,7 +125,7 @@ export function generateInlineTypeCtor(
 }
 
 function constructTypeCtorParameter(
-    node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
+    env: ReferenceEmitEnvironment, meta: TypeCtorMetadata,
     rawType: ts.TypeReferenceNode): ts.ParameterDeclaration {
   // initType is the type of 'init', the single argument to the type constructor method.
   // If the Directive has any inputs, its initType will be:
@@ -138,9 +140,13 @@ function constructTypeCtorParameter(
 
   const plainKeys: ts.LiteralTypeNode[] = [];
   const coercedKeys: ts.PropertySignature[] = [];
+  const signalInputKeys: ts.LiteralTypeNode[] = [];
 
-  for (const {classPropertyName, transform} of meta.fields.inputs) {
-    if (!meta.coercedInputFields.has(classPropertyName)) {
+  for (const {classPropertyName, transform, isSignal} of meta.fields.inputs) {
+    if (isSignal) {
+      signalInputKeys.push(
+          ts.factory.createLiteralTypeNode(ts.factory.createStringLiteral(classPropertyName)));
+    } else if (!meta.coercedInputFields.has(classPropertyName)) {
       plainKeys.push(
           ts.factory.createLiteralTypeNode(ts.factory.createStringLiteral(classPropertyName)));
     } else {
@@ -169,6 +175,23 @@ function constructTypeCtorParameter(
         ts.factory.createIntersectionTypeNode([initType, coercedLiteral]) :
         coercedLiteral;
   }
+  if (signalInputKeys.length > 0) {
+    const keyTypeUnion = ts.factory.createUnionTypeNode(signalInputKeys);
+
+    // Construct the UnwrapDirectiveSignalInputs<rawType, keyTypeUnion>.
+    const unwrapDirectiveSignalInputsExpr = env.referenceExternalType(
+        R3Identifiers.UnwrapDirectiveSignalInputs.moduleName,
+        R3Identifiers.UnwrapDirectiveSignalInputs.name, [
+          // TODO:
+          new ExpressionType(new WrappedNodeExpr(rawType)),
+          new ExpressionType(new WrappedNodeExpr(keyTypeUnion))
+        ]);
+
+
+    initType = initType !== null ?
+        ts.factory.createIntersectionTypeNode([initType, unwrapDirectiveSignalInputsExpr]) :
+        unwrapDirectiveSignalInputsExpr;
+  }
 
   if (initType === null) {
     // Special case - no inputs, outputs, or other fields which could influence the result type.
@@ -200,16 +223,17 @@ export function requiresInlineTypeCtor(
 /**
  * Add a default `= any` to type parameters that don't have a default value already.
  *
- * TypeScript uses the default type of a type parameter whenever inference of that parameter fails.
- * This can happen when inferring a complex type from 'any'. For example, if `NgFor`'s inference is
- * done with the TCB code:
+ * TypeScript uses the default type of a type parameter whenever inference of that parameter
+ * fails. This can happen when inferring a complex type from 'any'. For example, if `NgFor`'s
+ * inference is done with the TCB code:
  *
  * ```
  * class NgFor<T> {
  *   ngForOf: T[];
  * }
  *
- * declare function ctor<T>(o: Pick<NgFor<T>, 'ngForOf'|'ngForTrackBy'|'ngForTemplate'>): NgFor<T>;
+ * declare function ctor<T>(o: Pick<NgFor<T>, 'ngForOf'|'ngForTrackBy'|'ngForTemplate'>):
+ * NgFor<T>;
  * ```
  *
  * An invocation looks like:
@@ -222,14 +246,15 @@ export function requiresInlineTypeCtor(
  * assignment of type `number[]` to `ngForOf`'s type `T[]`. However, if `any` is passed instead:
  *
  * ```
- * var _t2 = ctor({ngForOf: [1, 2] as any, ngForTrackBy: null as any, ngForTemplate: null 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
- * the type `{}`, and so `_t2` is inferred as `NgFor<{}>`. This is obviously wrong.
+ * then inference for `T` fails (it cannot be inferred from `T[] = any`). In this case, `T`
+ * takes the type `{}`, and so `_t2` is inferred as `NgFor<{}>`. This is obviously wrong.
  *
- * Adding a default type to the generic declaration in the constructor solves this problem, as the
- * default type will be used in the event that inference fails.
+ * Adding a default type to the generic declaration in the constructor solves this problem, as
+ * the default type will be used in the event that inference fails.
  *
  * ```
  * declare function ctor<T = any>(o: Pick<NgFor<T>, 'ngForOf'>): NgFor<T>;

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

@@ -18,6 +18,7 @@ runInEachFileSystem(() => {
       inputs: Record<string, {type: string, isSignal: boolean, restrictionModifier?: string}>,
       template: string,
       extraDirectiveMembers?: string[],
+      directiveGenerics?: string,
       component?: string, expected: (string|jasmine.AsymmetricMatcher<string>)[],
       options?: Partial<TypeCheckingConfig>,
       focus?: boolean,
@@ -253,6 +254,151 @@ runInEachFileSystem(() => {
             template: `<div dir [pattern]="false">`,
             expected: [],
           },
+          // with generics (type constructor tests)
+          {
+            id: 'generic inference and binding to directive, all signal inputs',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              }
+            },
+            directiveGenerics: '<T>',
+            template: `<div dir [gen]="false" [other]="'invalid'">`,
+            expected: [
+              `TestComponent.html(1, 25): Type 'string' is not assignable to type 'boolean'.`,
+            ],
+          },
+          {
+            id: 'generic inference and binding to directive, mix of zone and signal',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'T',
+                isSignal: false,
+              }
+            },
+            directiveGenerics: '<T>',
+            template: `<div dir [gen]="false" [other]="'invalid'">`,
+            expected: [
+              `TestComponent.html(1, 11): Type 'boolean' is not assignable to type 'string'.`,
+            ],
+          },
+          {
+            id: 'generic inference and binding to directive (with `extends boolean`), all signal inputs',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              }
+            },
+            directiveGenerics: '<T extends boolean>',
+            template: `<div dir [gen]="false" [other]="'invalid'">`,
+            expected: [
+              `TestComponent.html(1, 25): Type '"invalid"' is not assignable to type 'false'.`,
+            ],
+          },
+          {
+            id: 'generic inference and binding to directive (with `extends boolean`), mix of zone and signal inputs',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'T',
+                isSignal: false,
+              }
+            },
+            directiveGenerics: '<T extends boolean>',
+            template: `<div dir [gen]="false" [other]="'invalid'">`,
+            expected: [
+              `TestComponent.html(1, 25): Type 'string' is not assignable to type 'boolean'.`,
+            ],
+          },
+          {
+            id: 'generic multi-inference and bindings to directive, all signal inputs',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'InputSignal<U, U>',
+                isSignal: true,
+              }
+            },
+            extraDirectiveMembers: [
+              'tester: {t: T, u: U} = null!',
+            ],
+            directiveGenerics: '<T, U>',
+            template: `
+              <div dir [gen]="false" [other]="'text'"
+                       #ref="dir" (click)="ref.tester = {t: 1, u: 0}">`,
+            expected: [
+              `TestComponent.html(3, 61): Type 'number' is not assignable to type 'boolean'.`,
+              `TestComponent.html(3, 67): Type 'number' is not assignable to type 'string'.`,
+            ],
+          },
+          {
+            id: 'generic multi-inference and bindings to directive, mix of zone and signal inputs',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'U',
+                isSignal: false,
+              }
+            },
+            extraDirectiveMembers: [
+              'tester: {t: T, u: U} = null!',
+            ],
+            directiveGenerics: '<T, U>',
+            template: `
+              <div dir [gen]="false" [other]="'text'"
+                       #ref="dir" (click)="ref.tester = {t: 1, u: 0}">`,
+            expected: [
+              `TestComponent.html(3, 61): Type 'number' is not assignable to type 'boolean'.`,
+              `TestComponent.html(3, 67): Type 'number' is not assignable to type 'string'.`,
+            ],
+          },
+          {
+            id: 'generic multi-inference and bindings to directive, more complicated generic inference',
+            inputs: {
+              gen: {
+                type: 'InputSignal<T, T>',
+                isSignal: true,
+              },
+              other: {
+                type: 'InputSignal<{u: U}, {u: U}>',
+                isSignal: true,
+              }
+            },
+            extraDirectiveMembers: [
+              'tester: {t: T, u: U} = null!',
+            ],
+            directiveGenerics: '<T, U>',
+            template: `
+              <div dir [gen]="false" [other]="{u: null}"
+                   #ref="dir" (click)="ref.tester = {t: 1, u: 0}">`,
+            expected: [
+              `TestComponent.html(3, 57): Type 'number' is not assignable to type 'boolean'.`,
+              `TestComponent.html(3, 63): Type 'number' is not assignable to type 'null'.`,
+            ],
+          },
         ];
 
     for (const c of bindingCases) {
@@ -264,7 +410,7 @@ runInEachFileSystem(() => {
         const testComponent = `
               import {InputSignal} from '@angular/core';
 
-              class Dir {
+              class Dir${c.directiveGenerics ?? ''} {
                 ${inputFields.join('\n')}
                 ${c.extraDirectiveMembers?.join('\n') ?? ''}
               }
@@ -294,7 +440,7 @@ runInEachFileSystem(() => {
                 name: 'Dir',
                 selector: '[dir]',
                 exportAs: ['dir'],
-                isGeneric: false,
+                isGeneric: c.directiveGenerics !== undefined,
                 inputs: inputDeclarations,
                 restrictedInputFields: Object.entries(c.inputs)
                                            .filter(([_, i]) => i.restrictionModifier !== undefined)

packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts

@@ -112,6 +112,9 @@ export function angularCoreDts(): TestFile {
       [ɵINPUT_SIGNAL_BRAND_READ_TYPE]: ReadT;
       [ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]: WriteT;
     };
+
+    export type ɵUnwrapInputSignalWriteType<Field> = Field extends InputSignal<unknown, infer WriteT>? WriteT : never;
+    export type ɵUnwrapDirectiveSignalInputs<Dir, Fields extends keyof Dir> = {[P in Fields]: ɵUnwrapInputSignalWriteType<Dir[P]>};
    `
   };
 }

packages/compiler/src/render3/r3_identifiers.ts

@@ -392,4 +392,5 @@ export class Identifiers {
 
   // type-checking
   static InputSignalBrandWriteType = {name: 'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE', moduleName: CORE};
+  static UnwrapDirectiveSignalInputs = {name: 'ɵUnwrapDirectiveSignalInputs', moduleName: CORE};
 }