refactor(compiler-cli): initial type-checking for signal inputs (angular#53521)

This commit introduces the initial type-checking for signal inputs.
To enable type-checking od signal inputs, there are a couple of tricks
needed. It's not trivial as it would look like at first glance.

Initial attempts could have been to generate additional statements in
type-checking blocks for signal inputs to simply call a method like
`InputSignal#applyNewValue`. This would seem natural, as it would match
what will happen at runtime, but this would break the language-service
auto completion in a highly subtle way. Consider the case where multiple
directives match the same input. Consider the directives have some
overlap in accepted input values, but they also have distinct diverging
values, like:

```ts
class DirA {
  value = input<'apple'|'shared'>();
}

class DirB {
  value = input<'orange'|'shared'>();
}
```

In such cases, auto completion for the binding expression should suggest
the following values: `apple`, `shared`, `orange` and `undefined`.

The language service achieves this by getting completions in the
type-check block where the user expression would live. This BREAKS if
we'd have multiple places where the expression from the user is used.

Two different places, or more, surface additional problems with
diagnostic collection. Previously diagnostics would surface the union
type of allowed values, but with multiple places, we'd have to work with
potentially 1+ diagnostics. This is non-ideal.

Another important consideration is test coverage. It might sound
problematic to consider the existing test infrastructure as relevant,
but in practice, we have thousands of diagnostic type check block tests
that would greatly benefit if the general emit structure would still
match conceptually. This is another bonus argument on why changing the
way inputs are applied is probably an option we should consider as a
last resort.

Ultimately, there is a good solution where we unwrap directive signal
inputs, based on metadata, and access a brand type field on the
`InputSignal`. This ensures auto-completion continues to work as is, and
also the structure of type check blocks doesn't change conceptually. In
future commits we also need to handle type-inference for generic signal
inputs.

Note: Another alternative considered, in terms of using metadata or not.
We could have type helpers to unwrap signal inputs using type helpers
like: `T extends InputSignal<any, WriteT> ? WriteT : T`. This would
allow us to drop the input signal metadata dependency, but in reality,
this has a few issues:

- users might have `@Input`'s passing around `InputSignal`'s. This is
  unlikely, but shows that the solution would not be fully correct.
- we need the metadata regardless, as we plan on accessing it at runtime
  as well, to distinguish between signal inputs and normal inputs when
  applying new values. This was not clear when this option was
  considered initially.

PR Close angular#53521

Author: devversion

packages/compiler-cli/src/ngtsc/metadata/src/util.ts

@@ -120,9 +120,16 @@ export function extractDirectiveTypeCheckMeta(
   const hasNgTemplateContextGuard = staticMembers.some(
       member => member.kind === ClassMemberKind.Method && member.name === 'ngTemplateContextGuard');
 
-  const coercedInputFields =
-      new Set(staticMembers.map(extractCoercedInput)
-                  .filter((inputName): inputName is ClassPropertyName => inputName !== null));
+  const coercedInputFields = new Set<ClassPropertyName>(
+      staticMembers.map(extractCoercedInput).filter((inputName): inputName is ClassPropertyName => {
+        // If the input refers to a signal input, we will not respect coercion members.
+        // A transform function should be used instead.
+        // TODO(signals): consider throwing a diagnostic?
+        if (inputName === null || inputs.getByClassPropertyName(inputName)?.isSignal) {
+          return false;
+        }
+        return true;
+      }));
 
   const restrictedInputFields = new Set<ClassPropertyName>();
   const stringLiteralInputFields = new Set<ClassPropertyName>();

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

@@ -172,6 +172,15 @@ export class Environment implements ReferenceEmitEnvironment {
         this.reflector, this.refEmitter, this.importManager);
   }
 
+  /**
+   * Generate a `ts.Expression` that refers to the external symbol. This
+   * may result in new imports being generated.
+   */
+  referenceExternalSymbol(moduleName: string, name: string): ts.Expression {
+    const external = new ExternalExpr({moduleName, name});
+    return translateExpression(external, this.importManager);
+  }
+
   /**
    * Generates a `ts.TypeNode` representing a type that is being referenced from a different place
    * in the program. Any type references inside the transplanted type will be rewritten so that

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
+import {AST, BindingPipe, BindingType, BoundTarget, Call, createCssSelectorFromNode, CssSelector, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, SelectorMatcher, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstText, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
 import ts from 'typescript';
 
 import {Reference} from '../../imports';
@@ -707,13 +707,19 @@ class TcbDirectiveInputsOp extends TcbOp {
 
       let assignment: ts.Expression = wrapForDiagnostics(expr);
 
-      for (const {fieldName, required, transformType} of attr.inputs) {
+      for (const {fieldName, required, transformType, isSignal} of attr.inputs) {
         let target: ts.LeftHandSideExpression;
 
         if (required) {
           seenRequiredInputs.add(fieldName);
         }
 
+        // Note: There is no special logic for transforms/coercion with signal inputs.
+        // For signal inputs, a `transformType` will never be set as we do not capture
+        // the transform in the compiler metadata. Signal inputs incorporate their
+        // transform write type into their member type, and we extract it below when
+        // unwrapping the `InputSignal<ReadT, WriteT>`.
+
         if (this.dir.coercedInputFields.has(fieldName)) {
           let type: ts.TypeNode;
 
@@ -781,6 +787,24 @@ class TcbDirectiveInputsOp extends TcbOp {
                   dirId, ts.factory.createIdentifier(fieldName));
         }
 
+        // For signal inputs, we unwrap the target `InputSignal`. Note that
+        // we intentionally do the following things:
+        //   1. keep the direct access to `dir.[field]` so that modifiers are honored.
+        //   2. follow the existing pattern where multiple targets assign a single expression.
+        //      This is a significant requirement for language service auto-completion.
+        if (isSignal) {
+          const inputSignalBrandWriteSymbol = this.tcb.env.referenceExternalSymbol(
+              R3Identifiers.InputSignalBrandWriteType.moduleName,
+              R3Identifiers.InputSignalBrandWriteType.name);
+          if (!ts.isIdentifier(inputSignalBrandWriteSymbol) &&
+              !ts.isPropertyAccessExpression(inputSignalBrandWriteSymbol)) {
+            throw new Error(`Expected identifier or property access for reference to ${
+                R3Identifiers.InputSignalBrandWriteType.name}`);
+          }
+
+          target = ts.factory.createElementAccessExpression(target, inputSignalBrandWriteSymbol);
+        }
+
         if (attr.attribute.keySpan !== undefined) {
           addParseSpanInfo(target, attr.attribute.keySpan);
         }

packages/compiler/src/render3/r3_identifiers.ts

@@ -389,4 +389,7 @@ export class Identifiers {
       o.ExternalReference = {name: 'ɵɵtrustConstantResourceUrl', moduleName: CORE};
   static validateIframeAttribute:
       o.ExternalReference = {name: 'ɵɵvalidateIframeAttribute', moduleName: CORE};
+
+  // type-checking
+  static InputSignalBrandWriteType = {name: 'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE', moduleName: CORE};
 }

packages/core/src/authoring.ts

@@ -10,4 +10,4 @@
 // https://docs.google.com/document/d/1RXb1wYwsbJotO1KBgSDsAtKpduGmIHod9ADxuXcAvV4/edit?tab=t.0.
 
 export {InputFunction as ɵInputFunction, inputFunction as ɵinputFunctionForApiGuard, inputRequiredFunction as ɵinputFunctionRequiredForApiGuard} from './authoring/input';
-export {InputOptions as ɵInputOptions, InputOptionsWithoutTransform as ɵInputOptionsWithoutTransform, InputOptionsWithTransform as ɵInputOptionsWithTransform, InputSignal} from './authoring/input_signal';
+export {InputOptions as ɵInputOptions, InputOptionsWithoutTransform as ɵInputOptionsWithoutTransform, InputOptionsWithTransform as ɵInputOptionsWithTransform, InputSignal, ɵINPUT_SIGNAL_BRAND_WRITE_TYPE} from './authoring/input_signal';

packages/core/test/bundling/defer/bundle.golden_symbols.json

@@ -1472,6 +1472,9 @@
   {
     "name": "init_input"
   },
+  {
+    "name": "init_input_signal"
+  },
   {
     "name": "init_input_transforms_feature"
   },

packages/core/test/render3/jit_environment_spec.ts

@@ -30,6 +30,9 @@ const INTERFACE_EXCEPTIONS = new Set<string>([
 const AOT_ONLY = new Set<string>([
   'ɵsetClassMetadata',
   'ɵsetClassMetadataAsync',
+
+  // used in type-checking.
+  'ɵINPUT_SIGNAL_BRAND_WRITE_TYPE',
 ]);
 
 /**