WIP: fixing tests and completion with signal based type checking

NOTE: This is very experimental and manual type check block code is not
updated. All of this complexity arises from the fact of attempting to
support bindings where they go to the same directive with multiple
inputs of the same name.

Potentially we can deprecate this behavior for input signal based
components and avoid this complexity. Alternative solution is to simply
repeat expression diagnostics if a binding goes to two same fields.

Author: devversion

packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts

@@ -123,7 +123,7 @@ export interface TemplateTypeChecker {
       expr: PropertyRead|SafePropertyRead, component: ts.ClassDeclaration): TcbLocation|null;
 
   /**
-   * For the given node represents a `LiteralPrimitive`(the `TextAttribute` represents a string
+   * For the given node represents a `LiteralPrimitive` (the `TextAttribute` represents a string
    * literal), retrieve a `TcbLocation` that can be used to perform autocompletion at that point in
    * the node, if such a location exists.
    */

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

@@ -78,25 +78,22 @@ export class CompletionEngine {
     }
 
     let nodeContext: TcbLocation|null = null;
-    if (node instanceof EmptyExpr) {
-      const nodeLocation = findFirstMatchingNode(this.tcb, {
-        filter: ts.isIdentifier,
-        withSpan: node.sourceSpan,
-      });
-      if (nodeLocation !== null) {
-        nodeContext = {
-          tcbPath: this.tcbPath,
-          isShimFile: this.tcbIsShim,
-          positionInFile: nodeLocation.getStart(),
-        };
-      }
-    }
 
-    if (node instanceof PropertyRead && node.receiver instanceof ImplicitReceiver) {
+    // Two types of expressions:
+    //  - `[bla]=""` — Empty expression
+    //  - `[bla]="smth"` - Property Read with implicit receiver.
+    if (node instanceof EmptyExpr ||
+        (node instanceof PropertyRead && node.receiver instanceof ImplicitReceiver)) {
       const nodeLocation = findFirstMatchingNode(this.tcb, {
-        filter: ts.isPropertyAccessExpression,
+        // We look for input binding TCBs of the following formats:
+        //  `const _t1 = <value>; myDir.inputField = *_t1`.
+        //  `const _t1 = <value>; myDir.inputField.set(*_t1)`;
+        // Note: We would like to autocomplete at the `*` character,
+        // so that we get suggestions in the context of input target.
+        filter: isInputTcbValueExpression,
         withSpan: node.sourceSpan,
       });
+
       if (nodeLocation) {
         nodeContext = {
           tcbPath: this.tcbPath,
@@ -165,37 +162,48 @@ export class CompletionEngine {
       return this.expressionCompletionCache.get(expr)!;
     }
 
-    let tsExpr: ts.StringLiteral|ts.NumericLiteral|null = null;
-
-    if (expr instanceof TmplAstTextAttribute) {
-      const strNode = findFirstMatchingNode(this.tcb, {
-        filter: ts.isParenthesizedExpression,
+    let completionIndex: number|undefined = undefined;
+
+    completionIndex =
+        findFirstMatchingNode(this.tcb, {
+          // We look for input binding TCBs of the following formats:
+          //  `const _t1 = <value>; myDir.inputField = *_t1`.
+          //  `const _t1 = <value>; myDir.inputField.set(*_t1)`;
+          // Note: We would like to autocomplete at the `*` character,
+          // so that we get suggestions in the context of input target.
+          filter: isInputTcbValueExpression,
+          withSpan: expr instanceof TmplAstTextAttribute && expr.valueSpan !== undefined ?
+              expr.valueSpan :
+              expr.sourceSpan,
+        })?.getStart();
+
+    // If we are completing in the middle of an expression, in a literal,
+    // we look for the actual string literal or numeric literal and complete there.
+    if (completionIndex === undefined && expr instanceof LiteralPrimitive) {
+      const literalNode = findFirstMatchingNode(this.tcb, {
+        filter: (n: ts.Node): n is ts.StringLiteral | ts.NumericLiteral =>
+            ts.isStringLiteralLike(n) || ts.isNumericLiteral(n),
         withSpan: expr.sourceSpan,
       });
-      if (strNode !== null && ts.isStringLiteral(strNode.expression)) {
-        tsExpr = strNode.expression;
+
+      completionIndex = literalNode?.getStart();
+
+      if (literalNode !== null && ts.isStringLiteralLike(literalNode)) {
+        // Completion needs to be inside the string literal, so we account
+        // for the opening quotes.
+        completionIndex = literalNode.getStart() + 1;
       }
-    } else {
-      tsExpr = findFirstMatchingNode(this.tcb, {
-        filter: (n: ts.Node): n is ts.NumericLiteral | ts.StringLiteral =>
-            ts.isStringLiteral(n) || ts.isNumericLiteral(n),
-        withSpan: expr.sourceSpan,
-      });
     }
 
-    if (tsExpr === null) {
+
+    if (completionIndex === undefined) {
       return null;
     }
 
-    let positionInShimFile = tsExpr.getEnd();
-    if (ts.isStringLiteral(tsExpr)) {
-      // In the shimFile, if `tsExpr` is a string, the position should be in the quotes.
-      positionInShimFile -= 1;
-    }
     const res: TcbLocation = {
       tcbPath: this.tcbPath,
       isShimFile: this.tcbIsShim,
-      positionInFile: positionInShimFile,
+      positionInFile: completionIndex,
     };
     this.expressionCompletionCache.set(expr, res);
     return res;
@@ -233,3 +241,14 @@ export class CompletionEngine {
     return templateContext;
   }
 }
+
+/**
+ * Whether the given node refers to an TCB input value binding. e.g.
+ * with the formats:
+ *
+ *   `const _t1 = <value>; myDir.inputField = *_t1*`.
+ *   `const _t1 = <value>; myDir.inputField.set(*_t1*)`;
+ */
+function isInputTcbValueExpression(n: ts.Node): n is ts.Identifier {
+  return (ts.isBinaryExpression(n.parent) || ts.isCallExpression(n.parent)) && ts.isIdentifier(n);
+}

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

@@ -347,10 +347,12 @@ export class SymbolBuilder {
       return host !== null ? {kind: SymbolKind.DomBinding, host} : null;
     }
 
-    const nodes = findAllMatchingNodes(
-        this.typeCheckBlock, {withSpan: binding.sourceSpan, filter: isAssignment});
+    const inputStatementNodes = findAllMatchingNodes(
+        this.typeCheckBlock, {withSpan: binding.sourceSpan, filter: isInputStatement});
     const bindings: BindingSymbol[] = [];
-    for (const node of nodes) {
+    for (const statementNode of inputStatementNodes) {
+      const node = statementNode.expression;
+
       if (!isAccessExpression(node.left)) {
         continue;
       }
@@ -652,6 +654,19 @@ export class SymbolBuilder {
   }
 }
 
+function isInputStatement(n: ts.Node): n is ts.ExpressionStatement&{
+  expression: ts.BinaryExpression
+}
+{
+  if (ts.isExpressionStatement(n) && isAssignment(n.expression)) {
+    return true;
+  }
+
+  // TODO(signals): Handle signal statements.
+
+  return false;
+}
+
 /** Filter predicate function that matches any AST node. */
 function anyNodeFilter(n: ts.Node): n is ts.Node {
   return true;

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, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SafeCall, SafePropertyRead, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
+import {AST, BindingPipe, BindingType, BoundTarget, Call, DYNAMIC_TYPE, ImplicitReceiver, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, R3Identifiers, SafeCall, SafePropertyRead, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstDeferredBlock, TmplAstDeferredBlockTriggers, TmplAstElement, TmplAstForLoopBlock, TmplAstHoverDeferredTrigger, TmplAstIcu, TmplAstIfBlock, TmplAstIfBlockBranch, TmplAstInteractionDeferredTrigger, TmplAstNode, TmplAstReference, TmplAstSwitchBlock, TmplAstSwitchBlockCase, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable, TmplAstViewportDeferredTrigger, TransplantedType} from '@angular/compiler';
 import ts from 'typescript';
 
 import {Reference} from '../../imports';
@@ -707,6 +707,13 @@ class TcbDirectiveInputsOp extends TcbOp {
           widenBinding(translateInput(attr.attribute, this.tcb, this.scope), this.tcb);
       const sharedExprId = this.tcb.allocateId();
 
+      // Add the value span to the shared expression. This will be useful for completion
+      // purposes so that we can find a position for global completion in the context
+      // of assignment to a specific directive/component input. See `completion.ts`.
+      if (attr.attribute.valueSpan !== undefined) {
+        addParseSpanInfo(sharedExprId, attr.attribute.valueSpan);
+      }
+
       const inputTestStatements: ts.Statement[] = [
         // The expression itself is always added as test statement. In case no target
         // can be found (e.g. there is no class member for the input)- we'd still want to
@@ -722,7 +729,7 @@ class TcbDirectiveInputsOp extends TcbOp {
                     undefined,
                     exprForDiag,
                     )],
-                ts.NodeFlags.Const))
+                ts.NodeFlags.Const)),
       ];
 
       for (const {fieldName, required, transformType} of attr.inputs) {
@@ -805,8 +812,8 @@ class TcbDirectiveInputsOp extends TcbOp {
 
         // TODO(signals)
         if (this.dir.isSignal) {
-          const toWritableSignalExpr =
-              this.tcb.env.referenceExternalSymbol('@angular/core', 'ɵɵtoWritableSignal')
+          const toWritableSignalExpr = this.tcb.env.referenceExternalSymbol(
+              R3Identifiers.toWriteableSignal.moduleName, R3Identifiers.toWriteableSignal.name);
 
           inputTestStatements.push(
               ts.factory.createExpressionStatement(ts.factory.createCallExpression(

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

@@ -74,6 +74,8 @@ export class TypeCheckFile extends Environment {
     // is somehow more expensive than the initial parse.
     source += '\nexport const IS_A_MODULE = true;\n';
 
+    console.error(source);
+
     return source;
   }
 

packages/compiler/src/render3/r3_identifiers.ts

@@ -391,4 +391,10 @@ export class Identifiers {
       o.ExternalReference = {name: 'ɵɵtrustConstantResourceUrl', moduleName: CORE};
   static validateIframeAttribute:
       o.ExternalReference = {name: 'ɵɵvalidateIframeAttribute', moduleName: CORE};
+
+  // type-checking helpers
+  static toWriteableSignal = {
+    name: 'ɵɵtoWritableSignal',
+    moduleName: CORE,
+  };
 }

packages/language-service/src/completions.ts

@@ -203,6 +203,8 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
       if (this.isValidNodeContextCompletion(result)) {
         ngResults.push({
           ...result,
+          // Remove the leading/trailing quotes from the TS completion entries.
+          name: result.name.replace(/(^['"`]|['"~]$)/g, ''),
           replacementSpan,
         });
       }

packages/language-service/test/BUILD.bazel

@@ -24,6 +24,7 @@ jasmine_node_test(
         "//packages/compiler-cli/src/ngtsc/testing/fake_common:npm_package",
         "//packages/compiler-cli/src/ngtsc/testing/fake_core:npm_package",
     ],
+    shard_count = 4,
     deps = [
         ":test_lib",
     ],