Skip to content

Commit

Permalink
Added skipDestructiveCodeActions argument to organize imports server …
Browse files Browse the repository at this point in the history
…command (#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 <[email protected]>

* 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 <[email protected]>
  • Loading branch information
JoshuaKGoldberg and Jesse Trinity authored Apr 20, 2021
1 parent f67ee44 commit a910c8d
Show file tree
Hide file tree
Showing 14 changed files with 200 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ namespace ts.server {
};
}

organizeImports(_scope: OrganizeImportsScope, _formatOptions: FormatCodeSettings): readonly FileTextChanges[] {
organizeImports(_args: OrganizeImportsArgs, _formatOptions: FormatCodeSettings): readonly FileTextChanges[] {
return notImplemented();
}

Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ namespace Harness.LanguageService {
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {
throw new Error("Not supported on the shim.");
}
organizeImports(_scope: ts.OrganizeImportsScope, _formatOptions: ts.FormatCodeSettings): readonly ts.FileTextChanges[] {
organizeImports(_args: ts.OrganizeImportsArgs, _formatOptions: ts.FormatCodeSettings): readonly ts.FileTextChanges[] {
throw new Error("Not supported on the shim.");
}
getEditsForFileRename(): readonly ts.FileTextChanges[] {
Expand Down
1 change: 1 addition & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ namespace ts.server.protocol {

export interface OrganizeImportsRequestArgs {
scope: OrganizeImportsScope;
skipDestructiveCodeActions?: boolean;
}

export interface OrganizeImportsResponse extends Response {
Expand Down
16 changes: 12 additions & 4 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2201,10 +2201,18 @@ namespace ts.server {
}
}

private organizeImports({ scope }: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): readonly protocol.FileCodeEdits[] | readonly FileTextChanges[] {
Debug.assert(scope.type === "file");
const { file, project } = this.getFileAndProject(scope.args);
const changes = project.getLanguageService().organizeImports({ type: "file", fileName: file }, this.getFormatOptions(file), this.getPreferences(file));
private organizeImports(args: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): readonly protocol.FileCodeEdits[] | readonly FileTextChanges[] {
Debug.assert(args.scope.type === "file");
const { file, project } = this.getFileAndProject(args.scope.args);
const changes = project.getLanguageService().organizeImports(
{
fileName: file,
skipDestructiveCodeActions: args.skipDestructiveCodeActions,
type: "file",
},
this.getFormatOptions(file),
this.getPreferences(file)
);
if (simplifiedResult) {
return this.mapTextChangesToCodeEdits(changes);
}
Expand Down
10 changes: 8 additions & 2 deletions src/services/organizeImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ namespace ts.OrganizeImports {
host: LanguageServiceHost,
program: Program,
preferences: UserPreferences,
skipDestructiveCodeActions?: boolean
) {

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

const coalesceAndOrganizeImports = (importGroup: readonly ImportDeclaration[]) => stableSort(
coalesceImports(removeUnusedImports(importGroup, sourceFile, program)),
coalesceImports(removeUnusedImports(importGroup, sourceFile, program, skipDestructiveCodeActions)),
(s1, s2) => compareImportsOrRequireStatements(s1, s2));

// All of the old ImportDeclarations in the file, in syntactic order.
Expand Down Expand Up @@ -87,7 +88,12 @@ namespace ts.OrganizeImports {
}
}

function removeUnusedImports(oldImports: readonly ImportDeclaration[], sourceFile: SourceFile, program: Program) {
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;
}

const typeChecker = program.getTypeChecker();
const jsxNamespace = typeChecker.getJsxNamespace(sourceFile);
const jsxFragmentFactory = typeChecker.getJsxFragmentFactory(sourceFile);
Expand Down
8 changes: 4 additions & 4 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2017,13 +2017,13 @@ namespace ts {
return codefix.getAllFixes({ fixId, sourceFile, program, host, cancellationToken, formatContext, preferences });
}

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

return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences);
return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences, args.skipDestructiveCodeActions);
}

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

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

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

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

export type OrganizeImportsScope = CombinedCodeFixScope;
export interface OrganizeImportsArgs extends CombinedCodeFixScope {
skipDestructiveCodeActions?: boolean;
}

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

Expand Down
Loading

0 comments on commit a910c8d

Please sign in to comment.