Skip to content

Commit

Permalink
fix(language-service): add fix for individual unused imports (angular…
Browse files Browse the repository at this point in the history 
…#58719)

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 angular#58719
  • Loading branch information
@crisbeto @AndrewKushnir
crisbeto authored and AndrewKushnir committed Nov 19, 2024
1 parent f5b2b58 commit dcd27d7
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 12 deletions.
91 changes: 85 additions & 6 deletions packages/language-service/src/codefixes/fix_unused_standalone_imports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>>();
Expand All @@ -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)) {
Expand Down Expand Up @@ -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)
);
}
81 changes: 75 additions & 6 deletions packages/language-service/test/code_fixes_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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]`],
]);
});

Expand Down Expand Up @@ -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]`],
]);
});
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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;
});
Expand Down

0 comments on commit dcd27d7

Please sign in to comment.