fix(language-service): add fix for individual unused imports (#58719)

crisbeto · AndrewKushnir · commit a983865bffa8 · 2024-11-19T12:19:15.000-08:00
Fixes that `getCodeActions` wasn't implemented for the unused imports fixer which meant that it wouldn't show up in the most common cases. PR Close #58719
diff --git a/packages/language-service/src/codefixes/fix_unused_standalone_imports.ts b/packages/language-service/src/codefixes/fix_unused_standalone_imports.ts
@@ -17,7 +17,78 @@ import {findFirstMatchingNode} from '../utils/ts_utils';
  */
 export const fixUnusedStandaloneImportsMeta: CodeActionMeta = {
   errorCodes: [ngErrorCode(ErrorCode.UNUSED_STANDALONE_IMPORTS)],
-  getCodeActions: () => [],
+  getCodeActions: ({start, fileName, compiler}) => {
+    const file = compiler.programDriver.getProgram().getSourceFile(fileName) || null;
+
+    if (file === null) {
+      return [];
+    }
+
+    const node = findFirstMatchingNode(file, {
+      filter: (n): n is tss.Identifier =>
+        tss.isIdentifier(n) && start >= n.getStart() && start <= n.getEnd(),
+    });
+    const parent = node?.parent || null;
+
+    if (node === null || parent === null) {
+      return [];
+    }
+
+    if (isFullyUnusedArray(node, parent)) {
+      return [
+        {
+          fixName: FixIdForCodeFixesAll.FIX_UNUSED_STANDALONE_IMPORTS,
+          fixId: FixIdForCodeFixesAll.FIX_UNUSED_STANDALONE_IMPORTS,
+          fixAllDescription: `Remove all unused imports`,
+          description: `Remove all unused imports`,
+          changes: [
+            {
+              fileName,
+              textChanges: [
+                {
+                  span: {
+                    start: parent.initializer.getStart(),
+                    length: parent.initializer.getWidth(),
+                  },
+                  newText: '[]',
+                },
+              ],
+            },
+          ],
+        },
+      ];
+    } else if (tss.isArrayLiteralExpression(parent)) {
+      const newArray = tss.factory.updateArrayLiteralExpression(
+        parent,
+        parent.elements.filter((el) => el !== node),
+      );
+
+      return [
+        {
+          fixName: FixIdForCodeFixesAll.FIX_UNUSED_STANDALONE_IMPORTS,
+          fixId: FixIdForCodeFixesAll.FIX_UNUSED_STANDALONE_IMPORTS,
+          fixAllDescription: `Remove all unused imports`,
+          description: `Remove unused import ${node.text}`,
+          changes: [
+            {
+              fileName,
+              textChanges: [
+                {
+                  span: {
+                    start: parent.getStart(),
+                    length: parent.getWidth(),
+                  },
+                  newText: tss.createPrinter().printNode(tss.EmitHint.Unspecified, newArray, file),
+                },
+              ],
+            },
+          ],
+        },
+      ];
+    }
+
+    return [];
+  },
   fixIds: [FixIdForCodeFixesAll.FIX_UNUSED_STANDALONE_IMPORTS],
   getAllCodeActions: ({diagnostics}) => {
     const arrayUpdates = new Map<tss.ArrayLiteralExpression, Set<tss.Expression>>();
@@ -42,11 +113,7 @@ export const fixUnusedStandaloneImportsMeta: CodeActionMeta = {
       // If the diagnostic is reported on the name of the `imports` array initializer, it means
       // that all imports are unused so we can clear the entire array. Otherwise if it's reported
       // on a single element, we only have to remove that element.
-      if (
-        tss.isPropertyAssignment(parent) &&
-        parent.name === node &&
-        tss.isArrayLiteralExpression(parent.initializer)
-      ) {
+      if (isFullyUnusedArray(node, parent)) {
         arraysToClear.add(parent.initializer);
       } else if (tss.isArrayLiteralExpression(parent)) {
         if (!arrayUpdates.has(parent)) {
@@ -93,3 +160,15 @@ export const fixUnusedStandaloneImportsMeta: CodeActionMeta = {
     return {changes};
   },
 };
+
+/** Checks whether a diagnostic was reported on a node where all imports are unused. */
+function isFullyUnusedArray(
+  node: tss.Node,
+  parent: tss.Node,
+): parent is tss.PropertyAssignment & {initializer: tss.ArrayLiteralExpression} {
+  return (
+    tss.isPropertyAssignment(parent) &&
+    parent.name === node &&
+    tss.isArrayLiteralExpression(parent.initializer)
+  );
+}
diff --git a/packages/language-service/test/code_fixes_spec.ts b/packages/language-service/test/code_fixes_spec.ts
@@ -7,7 +7,6 @@
  */
 
 import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
-import {spawn} from 'child_process';
 import ts from 'typescript';
 
 import {FixIdForCodeFixesAll} from '../src/codefixes/utils';
@@ -357,7 +356,7 @@ describe('code fixes', () => {
       const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
       actionChangesMatch(actionChanges, `Import BarComponent from './bar' on FooModule`, [
         [``, `import { BarComponent } from "./bar";`],
-        [`imp`, `imports: [BarComponent]`],
+        [`imports: []`, `imports: [BarComponent]`],
       ]);
     });
 
@@ -527,7 +526,7 @@ describe('code fixes', () => {
       ]);
       const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
       actionChangesMatch(actionChanges, `Import BarComponent from './bar' on FooComponent`, [
-        [`{te`, `BarComponent, { test }`],
+        [`{test}`, `BarComponent, { test }`],
         [``, `, imports: [BarComponent]`],
       ]);
     });
@@ -573,7 +572,77 @@ describe('code fixes', () => {
   });
 
   describe('unused standalone imports', () => {
-    it('should fix imports array where some imports are not used', () => {
+    it('should fix single diagnostic about individual imports that are not used', () => {
+      const files = {
+        'app.ts': `
+         import {Component, Directive} from '@angular/core';
+
+         @Directive({selector: '[used]', standalone: true})
+         export class UsedDirective {}
+
+         @Directive({selector: '[unused]', standalone: true})
+         export class UnusedDirective {}
+
+         @Component({
+           template: '<span used></span>',
+           standalone: true,
+           imports: [UnusedDirective, UsedDirective],
+         })
+         export class AppComponent {}
+       `,
+      };
+
+      const project = createModuleAndProjectWithDeclarations(env, 'test', files);
+      const fixFile = project.openFile('app.ts');
+      fixFile.moveCursorToText('Unused¦Directive,');
+
+      const diags = project.getDiagnosticsForFile('app.ts');
+      const codeActions = project.getCodeFixesAtPosition('app.ts', fixFile.cursor, fixFile.cursor, [
+        diags[0].code,
+      ]);
+      const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
+
+      actionChangesMatch(actionChanges, 'Remove unused import UnusedDirective', [
+        ['[UnusedDirective, UsedDirective]', '[UsedDirective]'],
+      ]);
+    });
+
+    it('should fix single diagnostic about all imports that are not used', () => {
+      const files = {
+        'app.ts': `
+         import {Component, Directive, Pipe} from '@angular/core';
+
+         @Directive({selector: '[unused]', standalone: true})
+         export class UnusedDirective {}
+
+         @Pipe({name: 'unused', standalone: true})
+         export class UnusedPipe {}
+
+         @Component({
+           template: '',
+           standalone: true,
+           imports: [UnusedDirective, UnusedPipe],
+         })
+         export class AppComponent {}
+       `,
+      };
+
+      const project = createModuleAndProjectWithDeclarations(env, 'test', files);
+      const fixFile = project.openFile('app.ts');
+      fixFile.moveCursorToText('impo¦rts:');
+
+      const diags = project.getDiagnosticsForFile('app.ts');
+      const codeActions = project.getCodeFixesAtPosition('app.ts', fixFile.cursor, fixFile.cursor, [
+        diags[0].code,
+      ]);
+      const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
+
+      actionChangesMatch(actionChanges, 'Remove all unused imports', [
+        ['[UnusedDirective, UnusedPipe]', '[]'],
+      ]);
+    });
+
+    it('should fix all imports arrays where some imports are not used', () => {
       const files = {
         'app.ts': `
          import {Component, Directive, Pipe} from '@angular/core';
@@ -627,7 +696,7 @@ describe('code fixes', () => {
       });
     });
 
-    it('should fix imports array where all imports are not used', () => {
+    it('should fix all imports arrays where all imports are not used', () => {
       const files = {
         'app.ts': `
          import {Component, Directive, Pipe} from '@angular/core';
@@ -697,7 +766,7 @@ function allChangesForCodeActions(
   for (const action of codeActions) {
     const actionChanges = action.changes.flatMap((change) => {
       return change.textChanges.map((tc) => {
-        const oldText = collapse(fileContents.slice(tc.span.start, tc.span.start + spawn.length));
+        const oldText = collapse(fileContents.slice(tc.span.start, tc.span.start + tc.span.length));
         const newText = collapse(tc.newText);
         return [oldText, newText] as const;
       });
