From eafe9040a20fcf749dc2cb4c240fc2b69ccef666 Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Mon, 28 Oct 2024 15:21:26 +0100
Subject: [PATCH] refactor(migrations): inject migration internal mode
 incorrectly migration properties initialized to identifiers (#58393)

Fixes that when the `inject` migration in internal mode was starting to visit the nodes one level down from the root when considering whether an expression contains local references. This lead it to skip over top-level identifiers and migrate some code incorrectly.

PR Close #58393
---
 .../ng-generate/inject-migration/internal.ts  |  6 ++-
 .../schematics/test/inject_migration_spec.ts  | 39 +++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/packages/core/schematics/ng-generate/inject-migration/internal.ts b/packages/core/schematics/ng-generate/inject-migration/internal.ts
index ff09554fc52..dfbff4c4581 100644
--- a/packages/core/schematics/ng-generate/inject-migration/internal.ts
+++ b/packages/core/schematics/ng-generate/inject-migration/internal.ts
@@ -158,7 +158,7 @@ function hasLocalReferences(
   const sourceFile = root.getSourceFile();
   let hasLocalRefs = false;
 
-  root.forEachChild(function walk(node) {
+  const walk = (node: ts.Node) => {
     // Stop searching if we know that it has local references.
     if (hasLocalRefs) {
       return;
@@ -193,7 +193,9 @@ function hasLocalReferences(
     if (!hasLocalRefs) {
       node.forEachChild(walk);
     }
-  });
+  };
+
+  walk(root);
 
   return hasLocalRefs;
 }
diff --git a/packages/core/schematics/test/inject_migration_spec.ts b/packages/core/schematics/test/inject_migration_spec.ts
index 0526d9ea742..d75c0d3571a 100644
--- a/packages/core/schematics/test/inject_migration_spec.ts
+++ b/packages/core/schematics/test/inject_migration_spec.ts
@@ -1986,5 +1986,44 @@ describe('inject migration', () => {
         `}`,
       ]);
     });
+
+    it('should not inline properties initialized to identifiers referring to constructor parameters', async () => {
+      writeFile(
+        '/dir.ts',
+        [
+          `import { Injectable } from '@angular/core';`,
+          `import { OtherService } from './other-service';`,
+          ``,
+          `@Injectable()`,
+          `export class SomeService {`,
+          `  readonly otherService: OtherService;`,
+          ``,
+          `  constructor(readonly differentName: OtherService) {`,
+          `    this.otherService = differentName;`,
+          `  }`,
+          `}`,
+        ].join('\n'),
+      );
+
+      await runInternalMigration();
+
+      expect(tree.readContent('/dir.ts').split('\n')).toEqual([
+        `import { Injectable, inject } from '@angular/core';`,
+        `import { OtherService } from './other-service';`,
+        ``,
+        `@Injectable()`,
+        `export class SomeService {`,
+        `  readonly differentName = inject(OtherService);`,
+        ``,
+        `  readonly otherService: OtherService;`,
+        ``,
+        `  constructor() {`,
+        `    const differentName = this.differentName;`,
+        ``,
+        `    this.otherService = differentName;`,
+        `  }`,
+        `}`,
+      ]);
+    });
   });
 });