refactor(migrations): fix unique name generation not marking generated identifiers (#58126)

The unique name generator did not properly work to avoid collisions with
previously generated unique names. This commit fixes this and also
improves type safety of the logic.

PR Close #58126

packages/core/schematics/migrations/signal-migration/src/utils/is_identifier_free_in_scope.ts

@@ -10,9 +10,12 @@ import assert from 'assert';
 import ts from 'typescript';
 import {isNodeDescendantOf} from './is_descendant_of';
 
+/** Symbol that can be used to mark a variable as reserved, synthetically. */
+export const ReservedMarker = Symbol();
+
 // typescript/stable/src/compiler/types.ts;l=967;rcl=651008033
 export interface LocalsContainer extends ts.Node {
-  locals?: Map<string, ts.Symbol>;
+  locals?: Map<string, ts.Symbol | typeof ReservedMarker>;
   nextContainer?: LocalsContainer;
 }
 
@@ -71,9 +74,9 @@ function isIdentifierFreeInContainer(name: string, container: LocalsContainer):
   // Note: This check is similar to the check by the TypeScript emitter.
   // typescript/stable/src/compiler/emitter.ts;l=5436;rcl=651008033
   const local = container.locals.get(name)!;
-  return !(
-    local.flags &
-    (ts.SymbolFlags.Value | ts.SymbolFlags.ExportValue | ts.SymbolFlags.Alias)
+  return (
+    local !== ReservedMarker &&
+    !(local.flags & (ts.SymbolFlags.Value | ts.SymbolFlags.ExportValue | ts.SymbolFlags.Alias))
   );
 }
 

packages/core/schematics/migrations/signal-migration/src/utils/unique_names.ts

@@ -7,7 +7,7 @@
  */
 
 import ts from 'typescript';
-import {isIdentifierFreeInScope} from './is_identifier_free_in_scope';
+import {isIdentifierFreeInScope, ReservedMarker} from './is_identifier_free_in_scope';
 
 /**
  * Helper that can generate unique identifier names at a
@@ -27,7 +27,8 @@ export class UniqueNamesGenerator {
       }
 
       // Claim the locals to avoid conflicts with future generations.
-      freeInfo.container.locals?.set(name, null! as ts.Symbol);
+      freeInfo.container.locals ??= new Map();
+      freeInfo.container.locals.set(name, ReservedMarker);
       return true;
     };
 

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

@@ -0,0 +1,23 @@
+// tslint:disable
+
+import {Directive, Input} from '@angular/core';
+
+export class OtherCmp {
+  @Input() name = false;
+}
+
+@Directive()
+export class MyComp {
+  @Input() name = '';
+  other: OtherCmp = null!;
+
+  click() {
+    if (this.name) {
+      console.error(this.name);
+    }
+
+    if (this.other.name) {
+      console.error(this.other.name);
+    }
+  }
+}

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

@@ -602,12 +602,12 @@ export class AppComponent {
         if (isAudi(car)) {
           console.log(car.__audi);
         }
-        const narrowableMultipleTimes = ctx.narrowableMultipleTimes();
-        if (!isCar(narrowableMultipleTimes!) || !isAudi(narrowableMultipleTimes)) {
+        const narrowableMultipleTimesValue = ctx.narrowableMultipleTimes();
+        if (!isCar(narrowableMultipleTimesValue!) || !isAudi(narrowableMultipleTimesValue)) {
           return;
         }
 
-        narrowableMultipleTimes.__audi;
+        narrowableMultipleTimesValue.__audi;
       }
 
       // iife
@@ -1249,6 +1249,33 @@ class TwoWayBinding {
   @Input() inputC = false;
   readonly inputD = input(false);
 }
+@@@@@@ temporary_variables.ts @@@@@@
+
+// tslint:disable
+
+import {Directive, input} from '@angular/core';
+
+export class OtherCmp {
+  readonly name = input(false);
+}
+
+@Directive()
+export class MyComp {
+  readonly name = input('');
+  other: OtherCmp = null!;
+
+  click() {
+    const name = this.name();
+    if (name) {
+      console.error(name);
+    }
+
+    const nameValue = this.other.name();
+    if (nameValue) {
+      console.error(nameValue);
+    }
+  }
+}
 @@@@@@ transform_functions.ts @@@@@@
 
 // tslint:disable

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

@@ -576,12 +576,12 @@ export class AppComponent {
         if (isAudi(car)) {
           console.log(car.__audi);
         }
-        const narrowableMultipleTimes = ctx.narrowableMultipleTimes();
-        if (!isCar(narrowableMultipleTimes!) || !isAudi(narrowableMultipleTimes)) {
+        const narrowableMultipleTimesValue = ctx.narrowableMultipleTimes();
+        if (!isCar(narrowableMultipleTimesValue!) || !isAudi(narrowableMultipleTimesValue)) {
           return;
         }
 
-        narrowableMultipleTimes.__audi;
+        narrowableMultipleTimesValue.__audi;
       }
 
       // iife
@@ -1201,6 +1201,33 @@ class TwoWayBinding {
   readonly inputC = input(false);
   readonly inputD = input(false);
 }
+@@@@@@ temporary_variables.ts @@@@@@
+
+// tslint:disable
+
+import {Directive, input} from '@angular/core';
+
+export class OtherCmp {
+  readonly name = input(false);
+}
+
+@Directive()
+export class MyComp {
+  readonly name = input('');
+  other: OtherCmp = null!;
+
+  click() {
+    const name = this.name();
+    if (name) {
+      console.error(name);
+    }
+
+    const nameValue = this.other.name();
+    if (nameValue) {
+      console.error(nameValue);
+    }
+  }
+}
 @@@@@@ transform_functions.ts @@@@@@
 
 // tslint:disable