Skip to content

Commit

Permalink
Add option to OrganizeImports for removal only (#50931)
Browse files Browse the repository at this point in the history
* Remove unused imports

* Lint

* Update baselines

* Make mode paramter required

* Clean up
  • Loading branch information
andrewbranch authored Sep 29, 2022
1 parent 42f9143 commit 0ce72ef
Show file tree
Hide file tree
Showing 11 changed files with 101 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,8 @@ namespace FourSlash {
}
}

public verifyOrganizeImports(newContent: string) {
const changes = this.languageService.organizeImports({ fileName: this.activeFile.fileName, type: "file" }, this.formatCodeSettings, ts.emptyOptions);
public verifyOrganizeImports(newContent: string, mode?: ts.OrganizeImportsMode) {
const changes = this.languageService.organizeImports({ fileName: this.activeFile.fileName, type: "file", mode }, this.formatCodeSettings, ts.emptyOptions);
this.applyChanges(changes);
this.verifyFileContent(this.activeFile.fileName, newContent);
}
Expand Down
4 changes: 2 additions & 2 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,8 @@ namespace FourSlashInterface {
this.state.noMoveToNewFile();
}

public organizeImports(newContent: string) {
this.state.verifyOrganizeImports(newContent);
public organizeImports(newContent: string, mode?: ts.OrganizeImportsMode): void {
this.state.verifyOrganizeImports(newContent, mode);
}
}

Expand Down
8 changes: 8 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,17 @@ namespace ts.server.protocol {

export type OrganizeImportsScope = GetCombinedCodeFixScope;

export const enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused",
}

export interface OrganizeImportsRequestArgs {
scope: OrganizeImportsScope;
/** @deprecated Use `mode` instead */
skipDestructiveCodeActions?: boolean;
mode?: OrganizeImportsMode;
}

export interface OrganizeImportsResponse extends Response {
Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ namespace ts.server {
const changes = project.getLanguageService().organizeImports(
{
fileName: file,
skipDestructiveCodeActions: args.skipDestructiveCodeActions,
mode: args.mode as OrganizeImportsMode | undefined ?? (args.skipDestructiveCodeActions ? OrganizeImportsMode.SortAndCombine : undefined),
type: "file",
},
this.getFormatOptions(file),
Expand Down
56 changes: 34 additions & 22 deletions src/services/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,51 @@ namespace ts.OrganizeImports {
host: LanguageServiceHost,
program: Program,
preferences: UserPreferences,
skipDestructiveCodeActions?: boolean
mode: OrganizeImportsMode,
) {
const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext, preferences });

const coalesceAndOrganizeImports = (importGroup: readonly ImportDeclaration[]) => stableSort(
coalesceImports(removeUnusedImports(importGroup, sourceFile, program, skipDestructiveCodeActions)),
(s1, s2) => compareImportsOrRequireStatements(s1, s2));
const shouldSort = mode === OrganizeImportsMode.SortAndCombine || mode === OrganizeImportsMode.All;
const shouldCombine = shouldSort; // These are currently inseparable, but I draw a distinction for clarity and in case we add modes in the future.
const shouldRemove = mode === OrganizeImportsMode.RemoveUnused || mode === OrganizeImportsMode.All;
const maybeRemove = shouldRemove ? removeUnusedImports : identity;
const maybeCoalesce = shouldCombine ? coalesceImports : identity;
const processImportsOfSameModuleSpecifier = (importGroup: readonly ImportDeclaration[]) => {
const processedDeclarations = maybeCoalesce(maybeRemove(importGroup, sourceFile, program));
return shouldSort
? stableSort(processedDeclarations, (s1, s2) => compareImportsOrRequireStatements(s1, s2))
: processedDeclarations;
};

// All of the old ImportDeclarations in the file, in syntactic order.
const topLevelImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, sourceFile.statements.filter(isImportDeclaration));
topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports));
topLevelImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, processImportsOfSameModuleSpecifier));

// All of the old ExportDeclarations in the file, in syntactic order.
const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration);
organizeImportsWorker(topLevelExportDecls, coalesceExports);
// Exports are always used
if (mode !== OrganizeImportsMode.RemoveUnused) {
// All of the old ExportDeclarations in the file, in syntactic order.
const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration);
organizeImportsWorker(topLevelExportDecls, coalesceExports);
}

for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) {
if (!ambientModule.body) continue;

const ambientModuleImportGroupDecls = groupImportsByNewlineContiguous(sourceFile, ambientModule.body.statements.filter(isImportDeclaration));
ambientModuleImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, coalesceAndOrganizeImports));
ambientModuleImportGroupDecls.forEach(importGroupDecl => organizeImportsWorker(importGroupDecl, processImportsOfSameModuleSpecifier));

const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration);
organizeImportsWorker(ambientModuleExportDecls, coalesceExports);
// Exports are always used
if (mode !== OrganizeImportsMode.RemoveUnused) {
const ambientModuleExportDecls = ambientModule.body.statements.filter(isExportDeclaration);
organizeImportsWorker(ambientModuleExportDecls, coalesceExports);
}
}

return changeTracker.getChanges();

function organizeImportsWorker<T extends ImportDeclaration | ExportDeclaration>(
oldImportDecls: readonly T[],
coalesce: (group: readonly T[]) => readonly T[]) {

coalesce: (group: readonly T[]) => readonly T[],
) {
if (length(oldImportDecls) === 0) {
return;
}
Expand All @@ -56,8 +69,12 @@ namespace ts.OrganizeImports {
// but the consequences of being wrong are very minor.
suppressLeadingTrivia(oldImportDecls[0]);

const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier!)!);
const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier));
const oldImportGroups = shouldCombine
? group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier!)!)
: [oldImportDecls];
const sortedImportGroups = shouldSort
? stableSort(oldImportGroups, (group1, group2) => compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier))
: oldImportGroups;
const newImportDecls = flatMap(sortedImportGroups, importGroup =>
getExternalModuleName(importGroup[0].moduleSpecifier!)
? coalesce(importGroup)
Expand Down Expand Up @@ -129,12 +146,7 @@ namespace ts.OrganizeImports {
return false;
}

function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program, skipDestructiveCodeActions: boolean | undefined) {
// As a precaution, consider unused import detection to be destructive (GH #43051)
if (skipDestructiveCodeActions) {
return oldImports;
}

function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program) {
const typeChecker = program.getTypeChecker();
const compilerOptions = program.getCompilerOptions();
const jsxNamespace = typeChecker.getJsxNamespace(sourceFile);
Expand Down
3 changes: 2 additions & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2076,7 +2076,8 @@ namespace ts {
const sourceFile = getValidSourceFile(args.fileName);
const formatContext = formatting.getFormatContext(formatOptions, host);

return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences, args.skipDestructiveCodeActions);
const mode = args.mode ?? (args.skipDestructiveCodeActions ? OrganizeImportsMode.SortAndCombine : OrganizeImportsMode.All);
return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences, mode);
}

function getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences = emptyOptions): readonly FileTextChanges[] {
Expand Down
8 changes: 8 additions & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -575,8 +575,16 @@ namespace ts {

export interface CombinedCodeFixScope { type: "file"; fileName: string; }

export const enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused",
}

export interface OrganizeImportsArgs extends CombinedCodeFixScope {
/** @deprecated Use `mode` instead */
skipDestructiveCodeActions?: boolean;
mode?: OrganizeImportsMode;
}

export type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#" | " ";
Expand Down
14 changes: 14 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6032,8 +6032,15 @@ declare namespace ts {
type: "file";
fileName: string;
}
enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused"
}
interface OrganizeImportsArgs extends CombinedCodeFixScope {
/** @deprecated Use `mode` instead */
skipDestructiveCodeActions?: boolean;
mode?: OrganizeImportsMode;
}
type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#" | " ";
enum CompletionTriggerKind {
Expand Down Expand Up @@ -7616,9 +7623,16 @@ declare namespace ts.server.protocol {
arguments: OrganizeImportsRequestArgs;
}
type OrganizeImportsScope = GetCombinedCodeFixScope;
enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused"
}
interface OrganizeImportsRequestArgs {
scope: OrganizeImportsScope;
/** @deprecated Use `mode` instead */
skipDestructiveCodeActions?: boolean;
mode?: OrganizeImportsMode;
}
interface OrganizeImportsResponse extends Response {
body: readonly FileCodeEdits[];
Expand Down
7 changes: 7 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6032,8 +6032,15 @@ declare namespace ts {
type: "file";
fileName: string;
}
enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused"
}
interface OrganizeImportsArgs extends CombinedCodeFixScope {
/** @deprecated Use `mode` instead */
skipDestructiveCodeActions?: boolean;
mode?: OrganizeImportsMode;
}
type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#" | " ";
enum CompletionTriggerKind {
Expand Down
8 changes: 7 additions & 1 deletion tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ declare module ts {
Message
}

enum OrganizeImportsMode {
All = "All",
SortAndCombine = "SortAndCombine",
RemoveUnused = "RemoveUnused",
}

interface DiagnosticMessage {
key: string;
category: DiagnosticCategory;
Expand Down Expand Up @@ -442,7 +448,7 @@ declare namespace FourSlashInterface {

generateTypes(...options: GenerateTypesOptions[]): void;

organizeImports(newContent: string): void;
organizeImports(newContent: string, mode?: ts.OrganizeImportsMode): void;

toggleLineComment(newFileContent: string): void;
toggleMultilineComment(newFileContent: string): void;
Expand Down
16 changes: 16 additions & 0 deletions tests/cases/fourslash/organizeImports_removeOnly.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/// <reference path="fourslash.ts" />

//// import { c, b, a } from "foo";
//// import d, { e } from "bar";
//// import * as f from "baz";
//// import { g } from "foo";
////
//// export { g, e, b, c };

verify.organizeImports(
`import { c, b } from "foo";
import { e } from "bar";
import { g } from "foo";
export { g, e, b, c };`,
ts.OrganizeImportsMode.RemoveUnused);

0 comments on commit 0ce72ef

Please sign in to comment.