perf(compiler-cli): fix performance of "interpolated signal not invoked" check (#64410)

This fixes a performance regression from #63754, which is almost a revert of the
prior performance fix in #57291; the latter was provided as quick fix to address
the severe performance overhead this extended diagnostic used to have, with #57337
as follow-up change to address the false negatives that were introduced in #57291.
That follow-up never landed, though, so this commit is re-applying the changes
from #57337 to fix the performance regression.

Fixes #64403

PR Close #64410

Author: JoostK

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

@@ -312,6 +312,14 @@ export interface TemplateTypeChecker {
    */
   getPipeMetadata(pipe: ts.ClassDeclaration): PipeMeta | null;
 
+  /**
+   * Gets the directives that apply to the given template node in a component's template.
+   */
+  getDirectivesOfNode(
+    component: ts.ClassDeclaration,
+    node: TmplAstElement | TmplAstTemplate,
+  ): TypeCheckableDirectiveMeta[] | null;
+
   /**
    * Gets the directives that have been used in a component's template.
    */

packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts

@@ -14,14 +14,16 @@ import {
   PrefixNot,
   PropertyRead,
   TmplAstBoundAttribute,
+  TmplAstElement,
   TmplAstIfBlock,
   TmplAstNode,
   TmplAstSwitchBlock,
+  TmplAstTemplate,
 } from '@angular/compiler';
 import ts from 'typescript';
 
 import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics';
-import {NgTemplateDiagnostic, SymbolKind} from '../../../api';
+import {NgTemplateDiagnostic, SymbolKind, TypeCheckableDirectiveMeta} from '../../../api';
 import {isSignalReference} from '../../../src/symbol_util';
 import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api';
 
@@ -52,37 +54,24 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
         .filter((item): item is PropertyRead => item instanceof PropertyRead)
         .flatMap((item) => buildDiagnosticForSignal(ctx, item, component));
     }
-    // bound properties like `[prop]="mySignal"`
-    else if (node instanceof TmplAstBoundAttribute) {
-      const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component);
-
-      if (
-        symbol?.kind === SymbolKind.Input &&
-        symbol.bindings.length > 0 &&
-        symbol.bindings.some((binding) => binding.target.kind === SymbolKind.Directive)
-      ) {
-        return [];
-      }
-
-      // otherwise, we check if the node is
-      const nodeAst = isPropertyReadNodeAst(node);
-      if (
-        // a bound property like `[prop]="mySignal"`
-        (node.type === BindingType.Property ||
-          // or a class binding like `[class.myClass]="mySignal"`
-          node.type === BindingType.Class ||
-          // or a style binding like `[style.width]="mySignal"`
-          node.type === BindingType.Style ||
-          // or an attribute binding like `[attr.role]="mySignal"`
-          node.type === BindingType.Attribute ||
-          // or an animation binding like `[animate.enter]="mySignal"`
-          node.type === BindingType.Animation ||
-          // or an animation binding like `[@myAnimation]="mySignal"`
-          node.type === BindingType.LegacyAnimation) &&
-        nodeAst
-      ) {
-        return buildDiagnosticForSignal(ctx, nodeAst, component);
-      }
+    // check bound inputs like `[prop]="mySignal"` on an element or inline template
+    else if (node instanceof TmplAstElement && node.inputs.length > 0) {
+      const directivesOfElement = ctx.templateTypeChecker.getDirectivesOfNode(component, node);
+      return node.inputs.flatMap((input) =>
+        checkBoundAttribute(ctx, component, directivesOfElement, input),
+      );
+    } else if (node instanceof TmplAstTemplate && node.tagName === 'ng-template') {
+      const directivesOfElement = ctx.templateTypeChecker.getDirectivesOfNode(component, node);
+      const inputDiagnostics = node.inputs.flatMap((input) => {
+        return checkBoundAttribute(ctx, component, directivesOfElement, input);
+      });
+      const templateAttrDiagnostics = node.templateAttrs.flatMap((attr) => {
+        if (!(attr instanceof TmplAstBoundAttribute)) {
+          return [];
+        }
+        return checkBoundAttribute(ctx, component, directivesOfElement, attr);
+      });
+      return inputDiagnostics.concat(templateAttrDiagnostics);
     }
     // if blocks like `@if(mySignal) { ... }`
     else if (node instanceof TmplAstIfBlock) {
@@ -111,6 +100,43 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
   }
 }
 
+function checkBoundAttribute(
+  ctx: TemplateContext<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>,
+  component: ts.ClassDeclaration,
+  directivesOfElement: Array<TypeCheckableDirectiveMeta> | null,
+  node: TmplAstBoundAttribute,
+): Array<NgTemplateDiagnostic<ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED>> {
+  // we skip the check if the node is an input binding
+  if (
+    directivesOfElement !== null &&
+    directivesOfElement.some((dir) => dir.inputs.getByBindingPropertyName(node.name) !== null)
+  ) {
+    return [];
+  }
+
+  // otherwise, we check if the node is
+  const nodeAst = isPropertyReadNodeAst(node);
+  if (
+    // a bound property like `[prop]="mySignal"`
+    (node.type === BindingType.Property ||
+      // or a class binding like `[class.myClass]="mySignal"`
+      node.type === BindingType.Class ||
+      // or a style binding like `[style.width]="mySignal"`
+      node.type === BindingType.Style ||
+      // or an attribute binding like `[attr.role]="mySignal"`
+      node.type === BindingType.Attribute ||
+      // or an animation binding like `[animate.enter]="mySignal"`
+      node.type === BindingType.Animation ||
+      // or an animation binding like `[@myAnimation]="mySignal"`
+      node.type === BindingType.LegacyAnimation) &&
+    nodeAst
+  ) {
+    return buildDiagnosticForSignal(ctx, nodeAst, component);
+  }
+
+  return [];
+}
+
 function isPropertyReadNodeAst(node: TmplAstBoundAttribute): PropertyRead | undefined {
   if (node.value instanceof ASTWithSource === false) {
     return undefined;
@@ -153,9 +179,11 @@ function buildDiagnosticForSignal(
   // error.
   // We also check for `{{ mySignal.set }}` or `{{ mySignal.update }}` or
   // `{{ mySignal.asReadonly }}` as these are the names of instance properties of Signal
+  if (!isFunctionInstanceProperty(node.name) && !isSignalInstanceProperty(node.name)) {
+    return [];
+  }
   const symbolOfReceiver = ctx.templateTypeChecker.getSymbolOfNode(node.receiver, component);
   if (
-    (isFunctionInstanceProperty(node.name) || isSignalInstanceProperty(node.name)) &&
     symbolOfReceiver !== null &&
     symbolOfReceiver.kind === SymbolKind.Expression &&
     isSignalReference(symbolOfReceiver)

packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts

@@ -634,29 +634,39 @@ runInEachFileSystem(() => {
       {
         fileName,
         templates: {
-          'TestCmp': `<child /> <div [id]="title"></div>`,
+          'TestCmp': `
+          <!-- The below "myInput" binding should be ignored, as it corresponds with TestDir -->
+          <div dir [myInput]="dirSignal"></div>
+
+          <!-- The below "myInput" binding should be reported, as it does not correspond with TestDir -->
+          <div [myInput]="divSignal"></div>
+
+          <!-- The below "myInput" binding applies to the "div" element so it should be reported -->
+          <div *dir [myInput]="structuralSignal"></div>
+          `,
         },
         source: `
           import {signal, input} from '@angular/core';
 
-          export class Child {
-            id = input(0);
+          export class TestDir {
+            myInput = input.required();
           }
-
           export class TestCmp {
-            title = signal('');
+            dirSignal = signal(0);
+            divSignal = signal(0);
+            structuralSignal = signal(0);
           }`,
         declarations: [
           {
             type: 'directive',
-            name: 'Child',
-            selector: 'child',
+            name: 'TestDir',
+            selector: '[dir]',
             inputs: {
-              id: {
+              myInput: {
                 isSignal: true,
-                bindingPropertyName: 'id',
-                classPropertyName: 'id',
-                required: false,
+                bindingPropertyName: 'myInput',
+                classPropertyName: 'myInput',
+                required: true,
                 transform: null,
               },
             },
@@ -670,11 +680,17 @@ runInEachFileSystem(() => {
       templateTypeChecker,
       program.getTypeChecker(),
       [interpolatedSignalFactory],
-      {} /* options */,
+      {},
+      /* options */
     );
     const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
-    expect(diags.length).toBe(1);
-    expect(diags[0].messageText).toBe('title is a function and should be invoked: title()');
+    expect(diags.length).toBe(2);
+    expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
+    expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
+    expect(getSourceCodeForDiagnostic(diags[0])).toBe(`divSignal`);
+    expect(diags[1].category).toBe(ts.DiagnosticCategory.Warning);
+    expect(diags[1].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
+    expect(getSourceCodeForDiagnostic(diags[1])).toBe(`structuralSignal`);
   });
 
   [

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

@@ -186,6 +186,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
     return data?.hostElement ?? null;
   }
 
+  getDirectivesOfNode(
+    component: ts.ClassDeclaration,
+    node: TmplAstElement | TmplAstTemplate,
+  ): TypeCheckableDirectiveMeta[] | null {
+    return (
+      this.getLatestComponentState(component).data?.boundTarget.getDirectivesOfNode(node) ?? null
+    );
+  }
+
   getUsedDirectives(component: ts.ClassDeclaration): TypeCheckableDirectiveMeta[] | null {
     return this.getLatestComponentState(component).data?.boundTarget.getUsedDirectives() ?? null;
   }