Skip to content

Commit 7498043

Browse files
amcaseyamcasey
committed
Group imports before sorting and coalescing
1 parent f459953 commit 7498043

File tree

3 files changed

+120
-189
lines changed

3 files changed

+120
-189
lines changed

Diff for: ‎src/harness/unittests/organizeImports.ts

+43-85
Original file line numberDiff line numberDiff line change
@@ -5,43 +5,6 @@
55
namespace ts {
66
describe("Organize imports", () => {
77
describe("Sort imports", () => {
8-
it("No imports", () => {
9-
assert.isEmpty(OrganizeImports.sortImports([]));
10-
});
11-
12-
it("One import", () => {
13-
const unsortedImports = parseImports(`import "lib";`);
14-
const actualSortedImports = OrganizeImports.sortImports(unsortedImports);
15-
const expectedSortedImports = unsortedImports;
16-
assertListEqual(expectedSortedImports, actualSortedImports);
17-
});
18-
19-
it("Stable - import kind", () => {
20-
assertUnaffectedBySort(
21-
`import "lib";`,
22-
`import * as x from "lib";`,
23-
`import x from "lib";`,
24-
`import {x} from "lib";`);
25-
});
26-
27-
it("Stable - default property alias", () => {
28-
assertUnaffectedBySort(
29-
`import x from "lib";`,
30-
`import y from "lib";`);
31-
});
32-
33-
it("Stable - module alias", () => {
34-
assertUnaffectedBySort(
35-
`import * as x from "lib";`,
36-
`import * as y from "lib";`);
37-
});
38-
39-
it("Stable - symbol", () => {
40-
assertUnaffectedBySort(
41-
`import {x} from "lib";`,
42-
`import {y} from "lib";`);
43-
});
44-
458
it("Sort - non-relative vs non-relative", () => {
469
assertSortsBefore(
4710
`import y from "lib1";`,
@@ -60,18 +23,10 @@ namespace ts {
6023
`import x from "./lib";`);
6124
});
6225

63-
function assertUnaffectedBySort(...importStrings: string[]) {
64-
const unsortedImports1 = parseImports(...importStrings);
65-
assertListEqual(unsortedImports1, OrganizeImports.sortImports(unsortedImports1));
66-
67-
const unsortedImports2 = reverse(unsortedImports1);
68-
assertListEqual(unsortedImports2, OrganizeImports.sortImports(unsortedImports2));
69-
}
70-
7126
function assertSortsBefore(importString1: string, importString2: string) {
72-
const imports = parseImports(importString1, importString2);
73-
assertListEqual(imports, OrganizeImports.sortImports(imports));
74-
assertListEqual(imports, OrganizeImports.sortImports(reverse(imports)));
27+
const [{moduleSpecifier: moduleSpecifier1}, {moduleSpecifier: moduleSpecifier2}] = parseImports(importString1, importString2);
28+
assert.equal(OrganizeImports.compareModuleSpecifiers(moduleSpecifier1, moduleSpecifier2), Comparison.LessThan);
29+
assert.equal(OrganizeImports.compareModuleSpecifiers(moduleSpecifier2, moduleSpecifier1), Comparison.GreaterThan);
7530
}
7631
});
7732

@@ -84,7 +39,7 @@ namespace ts {
8439
const sortedImports = parseImports(`import { default as m, a as n, b, y, z as o } from "lib";`);
8540
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
8641
const expectedCoalescedImports = parseImports(`import { a as n, b, default as m, y, z as o } from "lib";`);
87-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
42+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
8843
});
8944

9045
it("Combine side-effect-only imports", () => {
@@ -93,7 +48,7 @@ namespace ts {
9348
`import "lib";`);
9449
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
9550
const expectedCoalescedImports = parseImports(`import "lib";`);
96-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
51+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
9752
});
9853

9954
it("Combine namespace imports", () => {
@@ -102,7 +57,7 @@ namespace ts {
10257
`import * as y from "lib";`);
10358
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
10459
const expectedCoalescedImports = sortedImports;
105-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
60+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
10661
});
10762

10863
it("Combine default imports", () => {
@@ -111,7 +66,7 @@ namespace ts {
11166
`import y from "lib";`);
11267
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
11368
const expectedCoalescedImports = parseImports(`import { default as x, default as y } from "lib";`);
114-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
69+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
11570
});
11671

11772
it("Combine property imports", () => {
@@ -120,7 +75,7 @@ namespace ts {
12075
`import { y as z } from "lib";`);
12176
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
12277
const expectedCoalescedImports = parseImports(`import { x, y as z } from "lib";`);
123-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
78+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
12479
});
12580

12681
it("Combine side-effect-only import with namespace import", () => {
@@ -129,7 +84,7 @@ namespace ts {
12984
`import * as x from "lib";`);
13085
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
13186
const expectedCoalescedImports = sortedImports;
132-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
87+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
13388
});
13489

13590
it("Combine side-effect-only import with default import", () => {
@@ -138,7 +93,7 @@ namespace ts {
13893
`import x from "lib";`);
13994
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
14095
const expectedCoalescedImports = sortedImports;
141-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
96+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
14297
});
14398

14499
it("Combine side-effect-only import with property import", () => {
@@ -147,7 +102,7 @@ namespace ts {
147102
`import { x } from "lib";`);
148103
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
149104
const expectedCoalescedImports = sortedImports;
150-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
105+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
151106
});
152107

153108
it("Combine namespace import with default import", () => {
@@ -157,7 +112,7 @@ namespace ts {
157112
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
158113
const expectedCoalescedImports = parseImports(
159114
`import y, * as x from "lib";`);
160-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
115+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
161116
});
162117

163118
it("Combine namespace import with property import", () => {
@@ -166,7 +121,7 @@ namespace ts {
166121
`import { y } from "lib";`);
167122
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
168123
const expectedCoalescedImports = sortedImports;
169-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
124+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
170125
});
171126

172127
it("Combine default import with property import", () => {
@@ -176,7 +131,7 @@ namespace ts {
176131
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
177132
const expectedCoalescedImports = parseImports(
178133
`import x, { y } from "lib";`);
179-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
134+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
180135
});
181136

182137
it("Combine many imports", () => {
@@ -195,20 +150,7 @@ namespace ts {
195150
`import * as x from "lib";`,
196151
`import * as y from "lib";`,
197152
`import { a, b, default as w, default as z } from "lib";`);
198-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
199-
});
200-
201-
it("Combine imports from different modules", () => {
202-
const sortedImports = parseImports(
203-
`import { d } from "lib1";`,
204-
`import { b } from "lib1";`,
205-
`import { c } from "lib2";`,
206-
`import { a } from "lib2";`);
207-
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
208-
const expectedCoalescedImports = parseImports(
209-
`import { b, d } from "lib1";`,
210-
`import { a, c } from "lib2";`);
211-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
153+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
212154
});
213155

214156
// This is descriptive, rather than normative
@@ -219,7 +161,7 @@ namespace ts {
219161
`import z from "lib";`);
220162
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
221163
const expectedCoalescedImports = sortedImports;
222-
assertListEqual(expectedCoalescedImports, actualCoalescedImports);
164+
assertListEqual(actualCoalescedImports, expectedCoalescedImports);
223165
});
224166
});
225167

@@ -233,6 +175,17 @@ export default function F2();
233175
`,
234176
};
235177

178+
// Don't bother to actually emit a baseline for this.
179+
it("NoImports", () => {
180+
const testFile = {
181+
path: "/a.ts",
182+
content: "function F() { }",
183+
};
184+
const languageService = makeLanguageService(testFile);
185+
const changes = languageService.organizeImports({ type: "file", fileName: testFile.path }, testFormatOptions);
186+
assert.isEmpty(changes);
187+
});
188+
236189
testOrganizeImports("Simple",
237190
{
238191
path: "/test.ts",
@@ -283,6 +236,19 @@ D();
283236
libFile);
284237
// tslint:enable no-invalid-template-strings
285238

239+
testOrganizeImports("CoalesceMultipleModules",
240+
{
241+
path: "/test.ts",
242+
content: `
243+
import { d } from "lib1";
244+
import { b } from "lib1";
245+
import { c } from "lib2";
246+
import { a } from "lib2";
247+
`,
248+
},
249+
{ path: "/lib1.ts", content: "" },
250+
{ path: "/lib2.ts", content: "" });
251+
286252
testOrganizeImports("CoalesceTrivia",
287253
{
288254
path: "/test.ts",
@@ -315,8 +281,8 @@ F2();
315281
const { path: testPath, content: testContent } = testFile;
316282
const languageService = makeLanguageService(testFile, ...otherFiles);
317283
const changes = languageService.organizeImports({ type: "file", fileName: testPath }, testFormatOptions);
318-
assert.equal(1, changes.length);
319-
assert.equal(testPath, changes[0].fileName);
284+
assert.equal(changes.length, 1);
285+
assert.equal(changes[0].fileName, testPath);
320286

321287
Harness.Baseline.runBaseline(baselinePath, () => {
322288
const newText = textChanges.applyChanges(testContent, changes[0].textChanges);
@@ -340,7 +306,7 @@ F2();
340306
function parseImports(...importStrings: string[]): ReadonlyArray<ImportDeclaration> {
341307
const sourceFile = createSourceFile("a.ts", importStrings.join("\n"), ScriptTarget.ES2015, /*setParentNodes*/ true, ScriptKind.TS);
342308
const imports = filter(sourceFile.statements, isImportDeclaration);
343-
assert.equal(importStrings.length, imports.length);
309+
assert.equal(imports.length, importStrings.length);
344310
return imports;
345311
}
346312

@@ -414,13 +380,5 @@ F2();
414380
assertEqual(list1[i], list2[i]);
415381
}
416382
}
417-
418-
function reverse<T>(list: ReadonlyArray<T>) {
419-
const result = [];
420-
for (let i = list.length - 1; i >= 0; i--) {
421-
result.push(list[i]);
422-
}
423-
return result;
424-
}
425383
});
426384
}

0 commit comments

Comments
 (0)