fix(core): improve input writes migration in best effort mode

Currently, signal migration schematics in best effort mode doesn't do a very good job migrating input writes when there is a nested property access in templates.

In event handlers, no attempt is made to migrate a nested access in the left-hand-side of assignments or anything in their right-hand-side. E.g., nothing will happen here:

`(ngModelChange)="inputD.prop = $event + inputF"`.

Additionally, when a migration attempt is made, parentheses are often incorrectly placed on the parent, both in event handlers and two-way bindings:

`(ngModelChange)="inputC = $event"` is migrated to `(ngModelChange)="inputC = $event()"`.

`[(ngModel)]="inputB.prop.prop"` is migrated to `[(ngModel)]="inputB.prop().prop"`.

packages/core/schematics/migrations/signal-migration/src/passes/7_migrate_template_references.ts

@@ -30,15 +30,7 @@ export function pass7__migrateTemplateReferences<D extends ClassFieldDescriptor>
       continue;
     }
 
-    const parent = reference.from.readAstPath.at(-2);
-    let readEndPos: number;
-
-    if (reference.from.isWrite && parent) {
-      readEndPos = parent.sourceSpan.end;
-    } else {
-      readEndPos = reference.from.read.sourceSpan.end;
-    }
-
+    const readEndPos = reference.from.read.sourceSpan.end;
     // Skip duplicate references. E.g. if a template is shared.
     const fileReferenceId = `${reference.from.templateFile.id}:${readEndPos}`;
     if (seenFileReferences.has(fileReferenceId)) {

packages/core/schematics/migrations/signal-migration/src/passes/8_migrate_host_bindings.ts

@@ -35,15 +35,7 @@ export function pass8__migrateHostBindings<D extends ClassFieldDescriptor>(
 
     const bindingField = reference.from.hostPropertyNode;
     const expressionOffset = bindingField.getStart() + 1; // account for quotes.
-    const parent = reference.from.readAstPath.at(-2);
-    let readEndPos: number;
-
-    if (reference.from.isWrite && parent) {
-      readEndPos = expressionOffset + parent.sourceSpan.end;
-    } else {
-      readEndPos = expressionOffset + reference.from.read.sourceSpan.end;
-    }
-
+    const readEndPos = expressionOffset + reference.from.read.sourceSpan.end;
     // Skip duplicate references. Can happen if the host object is shared.
     if (seenReferences.get(bindingField)?.has(readEndPos)) {
       continue;

packages/core/schematics/migrations/signal-migration/src/passes/reference_resolution/template_reference_visitor.ts

@@ -244,6 +244,7 @@ export class TemplateExpressionReferenceVisitor<
   private activeTmplAstNode: ExprContext | null = null;
   private detectedInputReferences: TmplInputExpressionReference<ExprContext, D>[] = [];
   private isInsideObjectShorthandExpression = false;
+  private isInsideAssignment = false;
   private insideConditionalExpressionsWithReads: AST[] = [];
 
   constructor(
@@ -285,17 +286,20 @@ export class TemplateExpressionReferenceVisitor<
   }
 
   override visitPropertyRead(ast: PropertyRead, context: AST[]) {
-    this._inspectPropertyAccess(ast, false, context);
+    this._inspectPropertyAccess(ast, context);
     super.visitPropertyRead(ast, context);
   }
   override visitSafePropertyRead(ast: SafePropertyRead, context: AST[]) {
-    this._inspectPropertyAccess(ast, false, context);
+    this._inspectPropertyAccess(ast, context);
     super.visitPropertyRead(ast, context);
   }
 
   override visitBinary(ast: Binary, context: AST[]) {
-    if (ast.operation === '=' && ast.left instanceof PropertyRead) {
-      this._inspectPropertyAccess(ast.left, true, [...context, ast, ast.left]);
+    if (ast.operation === '=') {
+      this.isInsideAssignment = true;
+      this.visit(ast.left, [...context, ast]);
+      this.isInsideAssignment = false;
+      this.visit(ast.right, [...context, ast]);
     } else {
       super.visitBinary(ast, context);
     }
@@ -313,7 +317,7 @@ export class TemplateExpressionReferenceVisitor<
    * Inspects the property access and attempts to resolve whether they access
    * a known field. If so, the result is captured.
    */
-  private _inspectPropertyAccess(ast: PropertyRead, isAssignment: boolean, astPath: AST[]) {
+  private _inspectPropertyAccess(ast: PropertyRead, astPath: AST[]) {
     if (
       this.fieldNamesToConsiderForReferenceLookup !== null &&
       !this.fieldNamesToConsiderForReferenceLookup.has(ast.name)
@@ -322,7 +326,7 @@ export class TemplateExpressionReferenceVisitor<
     }
 
     const isWrite = !!(
-      isAssignment ||
+      this.isInsideAssignment ||
       (this.activeTmplAstNode && isTwoWayBindingNode(this.activeTmplAstNode))
     );
 

packages/core/schematics/migrations/signal-migration/test/golden-test/template_writes.ts

@@ -5,15 +5,19 @@ import {Component, Input} from '@angular/core';
 @Component({
   template: `
     <input [(ngModel)]="inputA" />
-    <div (click)="inputB = false"></div>
+    <input [(ngModel)]="inputB.prop.prop" />
+    <input (ngModelChange)="inputC = $event" />
+    <input (ngModelChange)="inputD.prop = $event + inputF" />
   `,
   host: {
-    '(click)': 'inputC = true',
+    '(click)': 'inputE = true',
   },
 })
 class TwoWayBinding {
   @Input() inputA = '';
-  @Input() inputB = true;
-  @Input() inputC = false;
-  @Input() inputD = false;
+  @Input() inputB = {prop: {prop: ''}};
+  @Input() inputC = '';
+  @Input() inputD = {prop: ''};
+  @Input() inputE = false;
+  @Input() inputF = '';
 }

packages/core/schematics/migrations/signal-migration/test/golden.txt

@@ -1299,10 +1299,12 @@ import {Component, Input, input} from '@angular/core';
 @Component({
   template: `
     <input [(ngModel)]="inputA" />
-    <div (click)="inputB = false"></div>
+    <input [(ngModel)]="inputB.prop.prop" />
+    <input (ngModelChange)="inputC = $event" />
+    <input (ngModelChange)="inputD.prop = $event + inputF()" />
   `,
   host: {
-    '(click)': 'inputC = true',
+    '(click)': 'inputE = true',
   },
 })
 class TwoWayBinding {
@@ -1311,11 +1313,17 @@ class TwoWayBinding {
   @Input() inputA = '';
   // TODO: Skipped for migration because:
   //  Your application code writes to the input. This prevents migration.
-  @Input() inputB = true;
+  @Input() inputB = {prop: {prop: ''}};
   // TODO: Skipped for migration because:
   //  Your application code writes to the input. This prevents migration.
-  @Input() inputC = false;
-  readonly inputD = input(false);
+  @Input() inputC = '';
+  // TODO: Skipped for migration because:
+  //  Your application code writes to the input. This prevents migration.
+  @Input() inputD = {prop: ''};
+  // TODO: Skipped for migration because:
+  //  Your application code writes to the input. This prevents migration.
+  @Input() inputE = false;
+  readonly inputF = input('');
 }
 @@@@@@ temporary_variables.ts @@@@@@
 

packages/core/schematics/migrations/signal-migration/test/golden_best_effort.txt

@@ -1253,17 +1253,21 @@ import {Component, input} from '@angular/core';
 @Component({
   template: `
     <input [(ngModel)]="inputA()" />
-    <div (click)="inputB = false()"></div>
+    <input [(ngModel)]="inputB().prop.prop" />
+    <input (ngModelChange)="inputC() = $event" />
+    <input (ngModelChange)="inputD().prop = $event + inputF()" />
   `,
   host: {
-    '(click)': 'inputC = true()',
+    '(click)': 'inputE() = true',
   },
 })
 class TwoWayBinding {
   readonly inputA = input('');
-  readonly inputB = input(true);
-  readonly inputC = input(false);
-  readonly inputD = input(false);
+  readonly inputB = input({ prop: { prop: '' } });
+  readonly inputC = input('');
+  readonly inputD = input({ prop: '' });
+  readonly inputE = input(false);
+  readonly inputF = input('');
 }
 @@@@@@ temporary_variables.ts @@@@@@