From ae81add0834c6cfb1efd242f8095e8baa561f402 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Mon, 2 Nov 2020 14:18:50 -0800 Subject: [PATCH] Separate delete-all-imports from other delete-all (#41105) This fixes the first part of #32196 --- src/compiler/diagnosticMessages.json | 4 + src/services/codefixes/fixUnusedIdentifier.ts | 30 ++++- .../codeFixUnusedIdentifier_all_delete.ts | 6 +- ...deFixUnusedIdentifier_all_delete_import.ts | 115 ++++++++++++++++++ 4 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/codeFixUnusedIdentifier_all_delete_import.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 95d40754f37..6fc7ed1c994 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -5959,6 +5959,10 @@ "category": "Message", "code": 95144 }, + "Delete all unused imports": { + "category": "Message", + "code": 95145 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index 72ab6a957b5..4bb8282e6e4 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -3,6 +3,7 @@ namespace ts.codefix { const fixName = "unusedIdentifier"; const fixIdPrefix = "unusedIdentifier_prefix"; const fixIdDelete = "unusedIdentifier_delete"; + const fixIdDeleteImports = "unusedIdentifier_deleteImports"; const fixIdInfer = "unusedIdentifier_infer"; const errorCodes = [ Diagnostics._0_is_declared_but_its_value_is_never_read.code, @@ -32,7 +33,13 @@ namespace ts.codefix { const importDecl = tryGetFullImport(token); if (importDecl) { const changes = textChanges.ChangeTracker.with(context, t => t.delete(sourceFile, importDecl)); - return [createDeleteFix(changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)])]; + return [createCodeFixAction(fixName, changes, [Diagnostics.Remove_import_from_0, showModuleSpecifier(importDecl)], fixIdDeleteImports, Diagnostics.Delete_all_unused_imports)]; + } + else if (isImport(token)) { + const deletion = textChanges.ChangeTracker.with(context, t => tryDeleteDeclaration(sourceFile, token, t, checker, sourceFiles, /*isFixAll*/ false)); + if (deletion.length) { + return [createCodeFixAction(fixName, deletion, [Diagnostics.Remove_unused_declaration_for_Colon_0, token.getText(sourceFile)], fixIdDeleteImports, Diagnostics.Delete_all_unused_imports)]; + } } if (isObjectBindingPattern(token.parent)) { @@ -82,7 +89,7 @@ namespace ts.codefix { return result; }, - fixIds: [fixIdPrefix, fixIdDelete, fixIdInfer], + fixIds: [fixIdPrefix, fixIdDelete, fixIdDeleteImports, fixIdInfer], getAllCodeActions: context => { const { sourceFile, program } = context; const checker = program.getTypeChecker(); @@ -93,14 +100,20 @@ namespace ts.codefix { case fixIdPrefix: tryPrefixDeclaration(changes, diag.code, sourceFile, token); break; - case fixIdDelete: { - if (token.kind === SyntaxKind.InferKeyword) { - break; // Can't delete - } + case fixIdDeleteImports: { const importDecl = tryGetFullImport(token); if (importDecl) { changes.delete(sourceFile, importDecl); } + else if (isImport(token)) { + tryDeleteDeclaration(sourceFile, token, changes, checker, sourceFiles, /*isFixAll*/ true); + } + break; + } + case fixIdDelete: { + if (token.kind === SyntaxKind.InferKeyword || isImport(token)) { + break; // Can't delete + } else if (isJSDocTemplateTag(token)) { changes.delete(sourceFile, token); } @@ -147,6 +160,11 @@ namespace ts.codefix { changes.delete(sourceFile, Debug.checkDefined(cast(token.parent, isDeclarationWithTypeParameterChildren).typeParameters, "The type parameter to delete should exist")); } + function isImport(token: Node) { + return token.kind === SyntaxKind.ImportKeyword + || token.kind === SyntaxKind.Identifier && (token.parent.kind === SyntaxKind.ImportSpecifier || token.parent.kind === SyntaxKind.ImportClause); + } + // Sometimes the diagnostic span is an entire ImportDeclaration, so we should remove the whole thing. function tryGetFullImport(token: Node): ImportDeclaration | undefined { return token.kind === SyntaxKind.ImportKeyword ? tryCast(token.parent, isImportDeclaration) : undefined; diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts index ecdf9a98609..f4c57083066 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts @@ -61,8 +61,10 @@ verify.codeFixAll({ fixId: "unusedIdentifier_delete", fixAllDescription: ts.Diagnostics.Delete_all_unused_declarations.message, newFileContent: -`import { used1 } from "foo"; -import { used2 } from "foo"; +`import d from "foo"; +import d2, { used1 } from "foo"; +import { x } from "foo"; +import { x2, used2 } from "foo"; used1; used2; function f() { diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete_import.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete_import.ts new file mode 100644 index 00000000000..e0a528e1211 --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete_import.ts @@ -0,0 +1,115 @@ +/// + +// @noUnusedLocals: true +// @noUnusedParameters: true + +////import d from "foo"; +////import d2, { used1 } from "foo"; +////import { x } from "foo"; +////import { x2, used2 } from "foo"; +////used1; used2; +//// +////function f(a, b) { +//// const x = 0; +////} +////function g(a, b, c) { return a; } +////f; g; +//// +////interface I { +//// m(x: number): void; +////} +//// +////class C implements I { +//// m(x: number): void {} // Does not remove 'x', which is inherited +//// n(x: number): void {} +//// private ["o"](): void {} +////} +////C; +//// +////declare function takesCb(cb: (x: number, y: string) => void): void; +////takesCb((x, y) => {}); +////takesCb((x, y) => { x; }); +////takesCb((x, y) => { y; }); +//// +////function fn1(x: number, y: string): void {} +////takesCb(fn1); +//// +////function fn2(x: number, y: string): void { x; } +////takesCb(fn2); +//// +////function fn3(x: number, y: string): void { y; } +////takesCb(fn3); +//// +////x => { +//// const y = 0; +////}; +//// +////{ +//// let a, b; +////} +////for (let i = 0, j = 0; ;) {} +////for (const x of []) {} +////for (const y in {}) {} +//// +////export type First = T; +////export interface ISecond { u: U; } +////export const cls = class { u: U; }; +////export class Ctu {} +////export type Length = T extends ArrayLike ? number : never; // Not affected, can't delete + +verify.codeFixAll({ + fixId: "unusedIdentifier_deleteImports", + fixAllDescription: ts.Diagnostics.Delete_all_unused_imports.message, + newFileContent: +`import { used1 } from "foo"; +import { used2 } from "foo"; +used1; used2; + +function f(a, b) { + const x = 0; +} +function g(a, b, c) { return a; } +f; g; + +interface I { + m(x: number): void; +} + +class C implements I { + m(x: number): void {} // Does not remove 'x', which is inherited + n(x: number): void {} + private ["o"](): void {} +} +C; + +declare function takesCb(cb: (x: number, y: string) => void): void; +takesCb((x, y) => {}); +takesCb((x, y) => { x; }); +takesCb((x, y) => { y; }); + +function fn1(x: number, y: string): void {} +takesCb(fn1); + +function fn2(x: number, y: string): void { x; } +takesCb(fn2); + +function fn3(x: number, y: string): void { y; } +takesCb(fn3); + +x => { + const y = 0; +}; + +{ + let a, b; +} +for (let i = 0, j = 0; ;) {} +for (const x of []) {} +for (const y in {}) {} + +export type First = T; +export interface ISecond { u: U; } +export const cls = class { u: U; }; +export class Ctu {} +export type Length = T extends ArrayLike ? number : never; // Not affected, can't delete`, +});