Skip to content

Commit a910c8d

Browse files
JoshuaKGoldbergJesse Trinity
JoshuaKGoldberg
and
Jesse Trinity
authored
Added skipDestructiveCodeActions argument to organize imports server command (microsoft#43184)
* Stopped removing unused imports in files with syntactic errors * Added allowDestructiveCodeActions arg * Updated .d.ts baselines * Stop factoring syntax errors. Weird that no tests break... * Have args extend scope so it is not a breaking change * Update src/harness/harnessLanguageService.ts Co-authored-by: Jesse Trinity <jetrinit@microsoft.com> * Fixed API breaking change, and renamed to skip * Always with the baselines * One more .d.ts baseline to fix * Remove blank line in src/harness/harnessLanguageService.ts Co-authored-by: Jesse Trinity <jetrinit@microsoft.com>
1 parent f67ee44 commit a910c8d

14 files changed

Lines changed: 200 additions & 23 deletions

‎src/harness/client.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,7 @@ namespace ts.server {
710710
};
711711
}
712712

713-
organizeImports(_scope: OrganizeImportsScope, _formatOptions: FormatCodeSettings): readonly FileTextChanges[] {
713+
organizeImports(_args: OrganizeImportsArgs, _formatOptions: FormatCodeSettings): readonly FileTextChanges[] {
714714
return notImplemented();
715715
}
716716

‎src/harness/harnessLanguageService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ namespace Harness.LanguageService {
584584
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {
585585
throw new Error("Not supported on the shim.");
586586
}
587-
organizeImports(_scope: ts.OrganizeImportsScope, _formatOptions: ts.FormatCodeSettings): readonly ts.FileTextChanges[] {
587+
organizeImports(_args: ts.OrganizeImportsArgs, _formatOptions: ts.FormatCodeSettings): readonly ts.FileTextChanges[] {
588588
throw new Error("Not supported on the shim.");
589589
}
590590
getEditsForFileRename(): readonly ts.FileTextChanges[] {

‎src/server/protocol.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,7 @@ namespace ts.server.protocol {
681681

682682
export interface OrganizeImportsRequestArgs {
683683
scope: OrganizeImportsScope;
684+
skipDestructiveCodeActions?: boolean;
684685
}
685686

686687
export interface OrganizeImportsResponse extends Response {

‎src/server/session.ts‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,10 +2201,18 @@ namespace ts.server {
22012201
}
22022202
}
22032203

2204-
private organizeImports({ scope }: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): readonly protocol.FileCodeEdits[] | readonly FileTextChanges[] {
2205-
Debug.assert(scope.type === "file");
2206-
const { file, project } = this.getFileAndProject(scope.args);
2207-
const changes = project.getLanguageService().organizeImports({ type: "file", fileName: file }, this.getFormatOptions(file), this.getPreferences(file));
2204+
private organizeImports(args: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): readonly protocol.FileCodeEdits[] | readonly FileTextChanges[] {
2205+
Debug.assert(args.scope.type === "file");
2206+
const { file, project } = this.getFileAndProject(args.scope.args);
2207+
const changes = project.getLanguageService().organizeImports(
2208+
{
2209+
fileName: file,
2210+
skipDestructiveCodeActions: args.skipDestructiveCodeActions,
2211+
type: "file",
2212+
},
2213+
this.getFormatOptions(file),
2214+
this.getPreferences(file)
2215+
);
22082216
if (simplifiedResult) {
22092217
return this.mapTextChangesToCodeEdits(changes);
22102218
}

‎src/services/organizeImports.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@ namespace ts.OrganizeImports {
1313
host: LanguageServiceHost,
1414
program: Program,
1515
preferences: UserPreferences,
16+
skipDestructiveCodeActions?: boolean
1617
) {
1718

1819
const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext, preferences });
1920

2021
const coalesceAndOrganizeImports = (importGroup: readonly ImportDeclaration[]) => stableSort(
21-
coalesceImports(removeUnusedImports(importGroup, sourceFile, program)),
22+
coalesceImports(removeUnusedImports(importGroup, sourceFile, program, skipDestructiveCodeActions)),
2223
(s1, s2) => compareImportsOrRequireStatements(s1, s2));
2324

2425
// All of the old ImportDeclarations in the file, in syntactic order.
@@ -87,7 +88,12 @@ namespace ts.OrganizeImports {
8788
}
8889
}
8990

90-
function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program) {
91+
function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program, skipDestructiveCodeActions: boolean | undefined) {
92+
// As a precaution, consider unused import detection to be destructive (GH #43051)
93+
if (skipDestructiveCodeActions) {
94+
return oldImports;
95+
}
96+
9197
const typeChecker = program.getTypeChecker();
9298
const jsxNamespace = typeChecker.getJsxNamespace(sourceFile);
9399
const jsxFragmentFactory = typeChecker.getJsxFragmentFactory(sourceFile);

‎src/services/services.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2017,13 +2017,13 @@ namespace ts {
20172017
return codefix.getAllFixes({ fixId, sourceFile, program, host, cancellationToken, formatContext, preferences });
20182018
}
20192019

2020-
function organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, preferences: UserPreferences = emptyOptions): readonly FileTextChanges[] {
2020+
function organizeImports(args: OrganizeImportsArgs, formatOptions: FormatCodeSettings, preferences: UserPreferences = emptyOptions): readonly FileTextChanges[] {
20212021
synchronizeHostData();
2022-
Debug.assert(scope.type === "file");
2023-
const sourceFile = getValidSourceFile(scope.fileName);
2022+
Debug.assert(args.type === "file");
2023+
const sourceFile = getValidSourceFile(args.fileName);
20242024
const formatContext = formatting.getFormatContext(formatOptions, host);
20252025

2026-
return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences);
2026+
return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences, args.skipDestructiveCodeActions);
20272027
}
20282028

20292029
function getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences = emptyOptions): readonly FileTextChanges[] {

‎src/services/types.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ namespace ts {
528528

529529
getApplicableRefactors(fileName: string, positionOrRange: number | TextRange, preferences: UserPreferences | undefined, triggerReason?: RefactorTriggerReason, kind?: string): ApplicableRefactorInfo[];
530530
getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string, preferences: UserPreferences | undefined): RefactorEditInfo | undefined;
531-
organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[];
531+
organizeImports(args: OrganizeImportsArgs, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[];
532532
getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences | undefined): readonly FileTextChanges[];
533533

534534
getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean, forceDtsEmit?: boolean): EmitOutput;
@@ -552,7 +552,9 @@ namespace ts {
552552

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

555-
export type OrganizeImportsScope = CombinedCodeFixScope;
555+
export interface OrganizeImportsArgs extends CombinedCodeFixScope {
556+
skipDestructiveCodeActions?: boolean;
557+
}
556558

557559
export type CompletionsTriggerCharacter = "." | '"' | "'" | "`" | "/" | "@" | "<" | "#" | " ";
558560

0 commit comments

Comments
 (0)