fix(core): handle shorthand assignment in the inject migration (#57134)

cexbrayat · dylhunn · commit ca89ef914119 · 2024-07-25T18:43:45.000-07:00
Currently the migration updates: ```ts constructor(@Inject(LOCALE_ID) locale: string) { console.log({ locale }); } ``` to: ```ts constructor() { console.log({ locale }); } ``` This fixes the migration, and results in: ``` constructor() { const locale = inject(LOCALE_ID); console.log({ locale }); } ``` PR Close #57134
diff --git a/packages/core/schematics/ng-generate/inject-migration/analysis.ts b/packages/core/schematics/ng-generate/inject-migration/analysis.ts
@@ -64,11 +64,11 @@ export function detectClassesUsingDI(sourceFile: ts.SourceFile, localTypeChecker
 export function getConstructorUnusedParameters(
   declaration: ts.ConstructorDeclaration,
   localTypeChecker: ts.TypeChecker,
-): Set<ts.ParameterDeclaration> {
-  const accessedTopLevelParameters = new Set<ts.ParameterDeclaration>();
-  const topLevelParameters = new Set<ts.ParameterDeclaration>();
+): Set<ts.Declaration> {
+  const accessedTopLevelParameters = new Set<ts.Declaration>();
+  const topLevelParameters = new Set<ts.Declaration>();
   const topLevelParameterNames = new Set<string>();
-  const unusedParams = new Set<ts.ParameterDeclaration>();
+  const unusedParams = new Set<ts.Declaration>();
 
   // Prepare the parameters for quicker checks further down.
   for (const param of declaration.parameters) {
@@ -102,6 +102,12 @@ export function getConstructorUnusedParameters(
       if (ts.isParameter(decl) && topLevelParameters.has(decl)) {
         accessedTopLevelParameters.add(decl);
       }
+      if (ts.isShorthandPropertyAssignment(decl)) {
+        const symbol = localTypeChecker.getShorthandAssignmentValueSymbol(decl);
+        if (symbol && symbol.valueDeclaration && ts.isParameter(symbol.valueDeclaration)) {
+          accessedTopLevelParameters.add(symbol.valueDeclaration);
+        }
+      }
     });
   });
 
@@ -110,7 +116,6 @@ export function getConstructorUnusedParameters(
       unusedParams.add(param);
     }
   }
-
   return unusedParams;
 }
 
diff --git a/packages/core/schematics/test/inject_migration_spec.ts b/packages/core/schematics/test/inject_migration_spec.ts
@@ -503,6 +503,37 @@ describe('inject migration', () => {
     ]);
   });
 
+  it('should declare a variable if an injected parameter with modifiers is referenced in the constructor via shorthand assignment', async () => {
+    writeFile(
+      '/dir.ts',
+      [
+        `import { Directive, Inject, LOCALE_ID } from '@angular/core';`,
+        ``,
+        `@Directive()`,
+        `class MyDir {`,
+        `  constructor(@Inject(LOCALE_ID) locale: string) {`,
+        `    console.log({ locale });`,
+        `  }`,
+        `}`,
+      ].join('\n'),
+    );
+
+    await runMigration();
+
+    expect(tree.readContent('/dir.ts').split('\n')).toEqual([
+      `import { Directive, Inject, LOCALE_ID, inject } from '@angular/core';`,
+      ``,
+      `@Directive()`,
+      `class MyDir {`,
+      `  constructor() {`,
+      `    const locale = inject(LOCALE_ID);`,
+      ``,
+      `    console.log({ locale });`,
+      `  }`,
+      `}`,
+    ]);
+  });
+
   it('should not declare a variable in the constructor if the only references to the parameter are shadowed', async () => {
     writeFile(
       '/dir.ts',
@@ -824,7 +855,7 @@ describe('inject migration', () => {
     ]);
   });
 
-  it('should be able to opt into generating backwads-compatible constructors for a class with existing members', async () => {
+  it('should be able to opt into generating backwards-compatible constructors for a class with existing members', async () => {
     writeFile(
       '/dir.ts',
       [
@@ -870,7 +901,7 @@ describe('inject migration', () => {
     ]);
   });
 
-  it('should be able to opt into generating backwads-compatible constructors for a class that only has a constructor', async () => {
+  it('should be able to opt into generating backwards-compatible constructors for a class that only has a constructor', async () => {
     writeFile(
       '/dir.ts',
       [
