microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-05-15 12:51:30 -05:00
Code Issues Packages Projects Releases 212 Wiki Activity

Merge pull request #22001 from amcasey/OrganizeImportsMissedCommits

Browse Source 
Group imports before sorting and coalescing
This commit is contained in:
Andrew Casey
2018-02-16 15:28:15 -08:00
committed by GitHub
parent 4b34c428a7 1faefc7703
commit d2cbd8de06
3 changed files with 133 additions and 200 deletions
Download Patch File Download Diff File Expand all files Collapse all files

128
src/harness/unittests/organizeImports.ts
View File

@@ -5,43 +5,6 @@
namespace ts {
describe("Organize imports", () => {
describe("Sort imports", () => {
it("No imports", () => {
assert.isEmpty(OrganizeImports.sortImports([]));
});
it("One import", () => {
const unsortedImports = parseImports(`import "lib";`);
const actualSortedImports = OrganizeImports.sortImports(unsortedImports);
const expectedSortedImports = unsortedImports;
assertListEqual(expectedSortedImports, actualSortedImports);
});
it("Stable - import kind", () => {
assertUnaffectedBySort(
`import "lib";`,
`import * as x from "lib";`,
`import x from "lib";`,
`import {x} from "lib";`);
});
it("Stable - default property alias", () => {
assertUnaffectedBySort(
`import x from "lib";`,
`import y from "lib";`);
});
it("Stable - module alias", () => {
assertUnaffectedBySort(
`import * as x from "lib";`,
`import * as y from "lib";`);
});
it("Stable - symbol", () => {
assertUnaffectedBySort(
`import {x} from "lib";`,
`import {y} from "lib";`);
});
it("Sort - non-relative vs non-relative", () => {
assertSortsBefore(
`import y from "lib1";`,
@@ -60,18 +23,10 @@ namespace ts {
`import x from "./lib";`);
});
function assertUnaffectedBySort(...importStrings: string[]) {
const unsortedImports1 = parseImports(...importStrings);
assertListEqual(unsortedImports1, OrganizeImports.sortImports(unsortedImports1));
const unsortedImports2 = reverse(unsortedImports1);
assertListEqual(unsortedImports2, OrganizeImports.sortImports(unsortedImports2));
}
function assertSortsBefore(importString1: string, importString2: string) {
const imports = parseImports(importString1, importString2);
assertListEqual(imports, OrganizeImports.sortImports(imports));
assertListEqual(imports, OrganizeImports.sortImports(reverse(imports)));
const [{moduleSpecifier: moduleSpecifier1}, {moduleSpecifier: moduleSpecifier2}] = parseImports(importString1, importString2);
assert.equal(OrganizeImports.compareModuleSpecifiers(moduleSpecifier1, moduleSpecifier2), Comparison.LessThan);
assert.equal(OrganizeImports.compareModuleSpecifiers(moduleSpecifier2, moduleSpecifier1), Comparison.GreaterThan);
}
});
@@ -84,7 +39,7 @@ namespace ts {
const sortedImports = parseImports(`import { default as m, a as n, b, y, z as o } from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(`import { a as n, b, default as m, y, z as o } from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine side-effect-only imports", () => {
@@ -93,7 +48,7 @@ namespace ts {
`import "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(`import "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine namespace imports", () => {
@@ -102,7 +57,7 @@ namespace ts {
`import * as y from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine default imports", () => {
@@ -111,7 +66,7 @@ namespace ts {
`import y from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(`import { default as x, default as y } from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine property imports", () => {
@@ -120,7 +75,7 @@ namespace ts {
`import { y as z } from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(`import { x, y as z } from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine side-effect-only import with namespace import", () => {
@@ -129,7 +84,7 @@ namespace ts {
`import * as x from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine side-effect-only import with default import", () => {
@@ -138,7 +93,7 @@ namespace ts {
`import x from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine side-effect-only import with property import", () => {
@@ -147,7 +102,7 @@ namespace ts {
`import { x } from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine namespace import with default import", () => {
@@ -157,7 +112,7 @@ namespace ts {
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(
`import y, * as x from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine namespace import with property import", () => {
@@ -166,7 +121,7 @@ namespace ts {
`import { y } from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine default import with property import", () => {
@@ -176,7 +131,7 @@ namespace ts {
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(
`import x, { y } from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
it("Combine many imports", () => {
@@ -195,20 +150,7 @@ namespace ts {
`import * as x from "lib";`,
`import * as y from "lib";`,
`import { a, b, default as w, default as z } from "lib";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
});
it("Combine imports from different modules", () => {
const sortedImports = parseImports(
`import { d } from "lib1";`,
`import { b } from "lib1";`,
`import { c } from "lib2";`,
`import { a } from "lib2";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = parseImports(
`import { b, d } from "lib1";`,
`import { a, c } from "lib2";`);
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
// This is descriptive, rather than normative
@@ -219,7 +161,7 @@ namespace ts {
`import z from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
const expectedCoalescedImports = sortedImports;
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
});
});
@@ -233,6 +175,17 @@ export default function F2();
`,
};
// Don't bother to actually emit a baseline for this.
it("NoImports", () => {
const testFile = {
path: "/a.ts",
content: "function F() { }",
};
const languageService = makeLanguageService(testFile);
const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatOptions);
assert.isEmpty(changes);
});
testOrganizeImports("Simple",
{
path: "/test.ts",
@@ -283,6 +236,19 @@ D();
libFile);
// tslint:enable no-invalid-template-strings
testOrganizeImports("CoalesceMultipleModules",
{
path: "/test.ts",
content: `
import { d } from "lib1";
import { b } from "lib1";
import { c } from "lib2";
import { a } from "lib2";
`,
},
{ path: "/lib1.ts", content: "" },
{ path: "/lib2.ts", content: "" });
testOrganizeImports("CoalesceTrivia",
{
path: "/test.ts",
@@ -315,8 +281,8 @@ F2();
const { path: testPath, content: testContent } = testFile;
const languageService = makeLanguageService(testFile, ...otherFiles);
const changes = languageService.organizeImports({ type: "file", fileName: testPath }, testFormatOptions);
assert.equal(1, changes.length);
assert.equal(testPath, changes[0].fileName);
assert.equal(changes.length, 1);
assert.equal(changes[0].fileName, testPath);
Harness.Baseline.runBaseline(baselinePath, () => {
const newText = textChanges.applyChanges(testContent, changes[0].textChanges);
@@ -340,7 +306,7 @@ F2();
function parseImports(...importStrings: string[]): ReadonlyArray<ImportDeclaration> {
const sourceFile = createSourceFile("a.ts", importStrings.join("\n"), ScriptTarget.ES2015, /*setParentNodes*/ true, ScriptKind.TS);
const imports = filter(sourceFile.statements, isImportDeclaration);
assert.equal(importStrings.length, imports.length);
assert.equal(imports.length, importStrings.length);
return imports;
}
@@ -414,13 +380,5 @@ F2();
assertEqual(list1[i], list2[i]);
}
}
function reverse<T>(list: ReadonlyArray<T>) {
const result = [];
for (let i = list.length - 1; i >= 0; i--) {
result.push(list[i]);
}
return result;
}
});
}

194
src/services/organizeImports.ts
View File

@@ -14,11 +14,15 @@ namespace ts.OrganizeImports {
return [];
}
const oldValidImportDecls = oldImportDecls.filter(importDecl => getExternalModuleName(importDecl.moduleSpecifier));
const oldInvalidImportDecls = oldImportDecls.filter(importDecl => !getExternalModuleName(importDecl.moduleSpecifier));
const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier));
// All of the new ImportDeclarations in the file, in sorted order.
const newImportDecls = coalesceImports(sortImports(removeUnusedImports(oldValidImportDecls))).concat(oldInvalidImportDecls);
const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) =>
compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier));
const newImportDecls = flatMap(sortedImportGroups, importGroup =>
getExternalModuleName(importGroup[0].moduleSpecifier)
? coalesceImports(removeUnusedImports(importGroup))
: importGroup);
const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext });
@@ -47,132 +51,83 @@ namespace ts.OrganizeImports {
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.
* @param importGroup a list of ImportDeclarations, all with the same module name.
*/
export function coalesceImports(sortedImports: ReadonlyArray<ImportDeclaration>) {
if (sortedImports.length === 0) {
return sortedImports;
export function coalesceImports(importGroup: ReadonlyArray<ImportDeclaration>) {
if (importGroup.length === 0) {
return importGroup;
}
const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getImportParts(importGroup);
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));
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]));
return coalescedImports;
}
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) {
return coalescedImports;
}
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>) {
@@ -231,4 +186,13 @@ namespace ts.OrganizeImports {
importDeclaration.moduleSpecifier);
}
}
/* internal */ // Exported for testing
export function compareModuleSpecifiers(m1: Expression, m2: Expression) {
const name1 = getExternalModuleName(m1);
const name2 = getExternalModuleName(m2);
return compareBooleans(name1 === undefined, name2 === undefined) ||
compareBooleans(isExternalModuleNameRelative(name1), isExternalModuleNameRelative(name2)) ||
compareStringsCaseSensitive(name1, name2);
}
}

11
tests/baselines/reference/organizeImports/CoalesceMultipleModules.ts Normal file
View File

@@ -0,0 +1,11 @@
// ==ORIGINAL==
import { d } from "lib1";
import { b } from "lib1";
import { c } from "lib2";
import { a } from "lib2";
// ==ORGANIZED==
import { b, d } from "lib1";
import { a, c } from "lib2";