refactor(migrations): fix unique name generation not marking generate…

…d identifiers (angular#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 angular#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