Skip to content

Commit

Permalink
Merge pull request #21909 from amcasey/OrganizeImports
Browse files Browse the repository at this point in the history
Introduce an organizeImports command
  • Loading branch information
amcasey authored Feb 16, 2018
2 parents b70aa22 + 5c278ce commit 70e9a5e
Show file tree
Hide file tree
Showing 20 changed files with 843 additions and 0 deletions.
1 change: 1 addition & 0 deletions Jakefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ var harnessSources = harnessCoreSources.concat([
"typingsInstaller.ts",
"projectErrors.ts",
"matchFiles.ts",
"organizeImports.ts",
"initializeTSConfig.ts",
"extractConstants.ts",
"extractFunctions.ts",
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1905,6 +1905,11 @@ namespace ts {
Comparison.EqualTo;
}

/** True is greater than false. */
export function compareBooleans(a: boolean, b: boolean): Comparison {
return compareValues(a ? 1 : 0, b ? 1 : 0);
}

function compareMessageText(text1: string | DiagnosticMessageChain, text2: string | DiagnosticMessageChain): Comparison {
while (text1 && text2) {
// We still have both chains.
Expand Down
3 changes: 3 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,9 @@ namespace Harness.LanguageService {
getApplicableRefactors(): ts.ApplicableRefactorInfo[] {
throw new Error("Not supported on the shim.");
}
organizeImports(_scope: ts.OrganizeImportsScope, _formatOptions: ts.FormatCodeSettings): ReadonlyArray<ts.FileTextChanges> {
throw new Error("Not supported on the shim.");
}
getEmitOutput(fileName: string): ts.EmitOutput {
return unwrapJSONCallResult(this.shim.getEmitOutput(fileName));
}
Expand Down
1 change: 1 addition & 0 deletions src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@
"./unittests/tsserverProjectSystem.ts",
"./unittests/tscWatchMode.ts",
"./unittests/matchFiles.ts",
"./unittests/organizeImports.ts",
"./unittests/initializeTSConfig.ts",
"./unittests/compileOnSave.ts",
"./unittests/typingsInstaller.ts",
Expand Down
426 changes: 426 additions & 0 deletions src/harness/unittests/organizeImports.ts

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/harness/unittests/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ namespace ts.server {
CommandNames.GetApplicableRefactors,
CommandNames.GetEditsForRefactor,
CommandNames.GetEditsForRefactorFull,
CommandNames.OrganizeImports,
CommandNames.OrganizeImportsFull,
];

it("should not throw when commands are executed with invalid arguments", () => {
Expand Down
4 changes: 4 additions & 0 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,10 @@ namespace ts.server {
};
}

organizeImports(_scope: OrganizeImportsScope, _formatOptions: FormatCodeSettings): ReadonlyArray<FileTextChanges> {
return notImplemented();
}

private convertCodeEditsToTextChanges(edits: protocol.FileCodeEdits[]): FileTextChanges[] {
return edits.map(edit => {
const fileName = edit.fileName;
Expand Down
25 changes: 25 additions & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ namespace ts.server.protocol {
/* @internal */
GetEditsForRefactorFull = "getEditsForRefactor-full",

OrganizeImports = "organizeImports",
/* @internal */
OrganizeImportsFull = "organizeImports-full",

// NOTE: If updating this, be sure to also update `allCommandNames` in `harness/unittests/session.ts`.
}

Expand Down Expand Up @@ -547,6 +551,27 @@ namespace ts.server.protocol {
renameFilename?: string;
}

/**
* Organize imports by:
* 1) Removing unused imports
* 2) Coalescing imports from the same module
* 3) Sorting imports
*/
export interface OrganizeImportsRequest extends Request {
command: CommandTypes.OrganizeImports;
arguments: OrganizeImportsRequestArgs;
}

export type OrganizeImportsScope = GetCombinedCodeFixScope;

export interface OrganizeImportsRequestArgs {
scope: OrganizeImportsScope;
}

export interface OrganizeImportsResponse extends Response {
edits: ReadonlyArray<FileCodeEdits>;
}

/**
* Request for the available codefixes at a specific position.
*/
Expand Down
19 changes: 19 additions & 0 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,19 @@ namespace ts.server {
}
}

private organizeImports({ scope }: protocol.OrganizeImportsRequestArgs, simplifiedResult: boolean): ReadonlyArray<protocol.FileCodeEdits> | ReadonlyArray<FileTextChanges> {
Debug.assert(scope.type === "file");
const { file, project } = this.getFileAndProject(scope.args);
const formatOptions = this.projectService.getFormatCodeOptions(file);
const changes = project.getLanguageService().organizeImports({ type: "file", fileName: file }, formatOptions);
if (simplifiedResult) {
return this.mapTextChangesToCodeEdits(project, changes);
}
else {
return changes;
}
}

private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): ReadonlyArray<protocol.CodeAction> | ReadonlyArray<CodeAction> {
if (args.errorCodes.length === 0) {
return undefined;
Expand Down Expand Up @@ -2041,6 +2054,12 @@ namespace ts.server {
},
[CommandNames.GetEditsForRefactorFull]: (request: protocol.GetEditsForRefactorRequest) => {
return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ false));
},
[CommandNames.OrganizeImports]: (request: protocol.OrganizeImportsRequest) => {
return this.requiredResponse(this.organizeImports(request.arguments, /*simplifiedResult*/ true));
},
[CommandNames.OrganizeImportsFull]: (request: protocol.OrganizeImportsRequest) => {
return this.requiredResponse(this.organizeImports(request.arguments, /*simplifiedResult*/ false));
}
});

Expand Down
234 changes: 234 additions & 0 deletions src/services/organizeImports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
/* @internal */
namespace ts.OrganizeImports {
export function organizeImports(
sourceFile: SourceFile,
formatContext: formatting.FormatContext,
host: LanguageServiceHost) {

// TODO (https://github.com/Microsoft/TypeScript/issues/10020): sort *within* ambient modules (find using isAmbientModule)

// All of the old ImportDeclarations in the file, in syntactic order.
const oldImportDecls = sourceFile.statements.filter(isImportDeclaration);

if (oldImportDecls.length === 0) {
return [];
}

const oldValidImportDecls = oldImportDecls.filter(importDecl => getExternalModuleName(importDecl.moduleSpecifier));
const oldInvalidImportDecls = oldImportDecls.filter(importDecl => !getExternalModuleName(importDecl.moduleSpecifier));

// All of the new ImportDeclarations in the file, in sorted order.
const newImportDecls = coalesceImports(sortImports(removeUnusedImports(oldValidImportDecls))).concat(oldInvalidImportDecls);

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

// Delete or replace the first import.
if (newImportDecls.length === 0) {
changeTracker.deleteNode(sourceFile, oldImportDecls[0]);
}
else {
// Note: Delete the surrounding trivia because it will have been retained in newImportDecls.
changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, {
useNonAdjustedStartPosition: false,
useNonAdjustedEndPosition: false,
suffix: getNewLineOrDefaultFromHost(host, formatContext.options),
});
}

// Delete any subsequent imports.
for (let i = 1; i < oldImportDecls.length; i++) {
changeTracker.deleteNode(sourceFile, oldImportDecls[i]);
}

return changeTracker.getChanges();
}

function removeUnusedImports(oldImports: ReadonlyArray<ImportDeclaration>) {
return oldImports; // TODO (https://github.com/Microsoft/TypeScript/issues/10020)
}

/* @internal */ // Internal for testing
export function sortImports(oldImports: ReadonlyArray<ImportDeclaration>) {
return stableSort(oldImports, (import1, import2) => {
const name1 = getExternalModuleName(import1.moduleSpecifier);
const name2 = getExternalModuleName(import2.moduleSpecifier);
Debug.assert(name1 !== undefined);
Debug.assert(name2 !== undefined);
return compareBooleans(isExternalModuleNameRelative(name1), isExternalModuleNameRelative(name2)) ||
compareStringsCaseSensitive(name1, name2);
});
}

function getExternalModuleName(specifier: Expression) {
return isStringLiteral(specifier) || isNoSubstitutionTemplateLiteral(specifier)
? specifier.text
: undefined;
}

/**
* @param sortedImports a non-empty list of ImportDeclarations, sorted by module name.
*/
function groupSortedImports(sortedImports: ReadonlyArray<ImportDeclaration>): ReadonlyArray<ReadonlyArray<ImportDeclaration>> {
Debug.assert(length(sortedImports) > 0);

const groups: ImportDeclaration[][] = [];

let groupName: string | undefined = getExternalModuleName(sortedImports[0].moduleSpecifier);
Debug.assert(groupName !== undefined);
let group: ImportDeclaration[] = [];

for (const importDeclaration of sortedImports) {
const moduleName = getExternalModuleName(importDeclaration.moduleSpecifier);
Debug.assert(moduleName !== undefined);
if (moduleName === groupName) {
group.push(importDeclaration);
}
else if (group.length) {
groups.push(group);

groupName = moduleName;
group = [importDeclaration];
}
}

if (group.length) {
groups.push(group);
}

return groups;
}

/* @internal */ // Internal for testing
/**
* @param sortedImports a list of ImportDeclarations, sorted by module name.
*/
export function coalesceImports(sortedImports: ReadonlyArray<ImportDeclaration>) {
if (sortedImports.length === 0) {
return sortedImports;
}

const coalescedImports: ImportDeclaration[] = [];

const groupedImports = groupSortedImports(sortedImports);
for (const importGroup of groupedImports) {

const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getImportParts(importGroup);

if (importWithoutClause) {
coalescedImports.push(importWithoutClause);
}

// Normally, we don't combine default and namespace imports, but it would be silly to
// produce two import declarations in this special case.
if (defaultImports.length === 1 && namespaceImports.length === 1 && namedImports.length === 0) {
// Add the namespace import to the existing default ImportDeclaration.
const defaultImportClause = defaultImports[0].parent as ImportClause;
coalescedImports.push(
updateImportDeclarationAndClause(defaultImportClause, defaultImportClause.name, namespaceImports[0]));

continue;
}

const sortedNamespaceImports = stableSort(namespaceImports, (n1, n2) => compareIdentifiers(n1.name, n2.name));

for (const namespaceImport of sortedNamespaceImports) {
// Drop the name, if any
coalescedImports.push(
updateImportDeclarationAndClause(namespaceImport.parent, /*name*/ undefined, namespaceImport));
}

if (defaultImports.length === 0 && namedImports.length === 0) {
continue;
}

let newDefaultImport: Identifier | undefined;
const newImportSpecifiers: ImportSpecifier[] = [];
if (defaultImports.length === 1) {
newDefaultImport = defaultImports[0];
}
else {
for (const defaultImport of defaultImports) {
newImportSpecifiers.push(
createImportSpecifier(createIdentifier("default"), defaultImport));
}
}

newImportSpecifiers.push(...flatMap(namedImports, n => n.elements));

const sortedImportSpecifiers = stableSort(newImportSpecifiers, (s1, s2) =>
compareIdentifiers(s1.propertyName || s1.name, s2.propertyName || s2.name) ||
compareIdentifiers(s1.name, s2.name));

const importClause = defaultImports.length > 0
? defaultImports[0].parent as ImportClause
: namedImports[0].parent;

const newNamedImports = sortedImportSpecifiers.length === 0
? undefined
: namedImports.length === 0
? createNamedImports(sortedImportSpecifiers)
: updateNamedImports(namedImports[0], sortedImportSpecifiers);

coalescedImports.push(
updateImportDeclarationAndClause(importClause, newDefaultImport, newNamedImports));
}

return coalescedImports;

function getImportParts(importGroup: ReadonlyArray<ImportDeclaration>) {
let importWithoutClause: ImportDeclaration | undefined;
const defaultImports: Identifier[] = [];
const namespaceImports: NamespaceImport[] = [];
const namedImports: NamedImports[] = [];

for (const importDeclaration of importGroup) {
if (importDeclaration.importClause === undefined) {
// Only the first such import is interesting - the others are redundant.
// Note: Unfortunately, we will lose trivia that was on this node.
importWithoutClause = importWithoutClause || importDeclaration;
continue;
}

const { name, namedBindings } = importDeclaration.importClause;

if (name) {
defaultImports.push(name);
}

if (namedBindings) {
if (isNamespaceImport(namedBindings)) {
namespaceImports.push(namedBindings);
}
else {
namedImports.push(namedBindings);
}
}
}

return {
importWithoutClause,
defaultImports,
namespaceImports,
namedImports,
};
}

function compareIdentifiers(s1: Identifier, s2: Identifier) {
return compareStringsCaseSensitive(s1.text, s2.text);
}

function updateImportDeclarationAndClause(
importClause: ImportClause,
name: Identifier | undefined,
namedBindings: NamedImportBindings | undefined) {

const importDeclaration = importClause.parent;
return updateImportDeclaration(
importDeclaration,
importDeclaration.decorators,
importDeclaration.modifiers,
updateImportClause(importClause, name, namedBindings),
importDeclaration.moduleSpecifier);
}
}
}
11 changes: 11 additions & 0 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
/// <reference path='jsTyping.ts' />
/// <reference path='navigateTo.ts' />
/// <reference path='navigationBar.ts' />
/// <reference path='organizeImports.ts' />
/// <reference path='outliningElementsCollector.ts' />
/// <reference path='patternMatcher.ts' />
/// <reference path='preProcess.ts' />
Expand Down Expand Up @@ -1848,6 +1849,15 @@ namespace ts {
return codefix.getAllFixes({ fixId, sourceFile, program, host, cancellationToken, formatContext });
}

function organizeImports(scope: OrganizeImportsScope, formatOptions: FormatCodeSettings): ReadonlyArray<FileTextChanges> {
synchronizeHostData();
Debug.assert(scope.type === "file");
const sourceFile = getValidSourceFile(scope.fileName);
const formatContext = formatting.getFormatContext(formatOptions);

return OrganizeImports.organizeImports(sourceFile, formatContext, host);
}

function applyCodeActionCommand(action: CodeActionCommand): Promise<ApplyCodeActionCommandResult>;
function applyCodeActionCommand(action: CodeActionCommand[]): Promise<ApplyCodeActionCommandResult[]>;
function applyCodeActionCommand(action: CodeActionCommand | CodeActionCommand[]): Promise<ApplyCodeActionCommandResult | ApplyCodeActionCommandResult[]>;
Expand Down Expand Up @@ -2143,6 +2153,7 @@ namespace ts {
getCodeFixesAtPosition,
getCombinedCodeFix,
applyCodeActionCommand,
organizeImports,
getEmitOutput,
getNonBoundSourceFile,
getSourceFile,
Expand Down
Loading

0 comments on commit 70e9a5e

Please sign in to comment.