Skip to content

Sort exports when organizeImports is run #24237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 18, 2018
Merged

Sort exports when organizeImports is run #24237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a327241
Sort exports when organizeImports is run
amcasey May 17, 2018
7fcf1fd
Delete redundant tests
amcasey May 18, 2018
43e1edf
Eliminate runtime type check
amcasey May 18, 2018
8f662a9
Extract sorting helper
amcasey May 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 244 additions & 7 deletions src/harness/unittests/organizeImports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,6 @@ namespace ts {
assert.isEmpty(OrganizeImports.coalesceImports([]));
});

it("Sort specifiers", () => {
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(actualCoalescedImports, expectedCoalescedImports);
});

it("Sort specifiers - case-insensitive", () => {
const sortedImports = parseImports(`import { default as M, a as n, B, y, Z as O } from "lib";`);
const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports);
Expand Down Expand Up @@ -181,6 +174,78 @@ namespace ts {
});
});

describe("Coalesce exports", () => {
it("No exports", () => {
assert.isEmpty(OrganizeImports.coalesceExports([]));
});

it("Sort specifiers - case-insensitive", () => {
const sortedExports = parseExports(`export { default as M, a as n, B, y, Z as O } from "lib";`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(`export { a as n, B, default as M, y, Z as O } from "lib";`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine namespace re-exports", () => {
const sortedExports = parseExports(
`export * from "lib";`,
`export * from "lib";`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(`export * from "lib";`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine property exports", () => {
const sortedExports = parseExports(
`export { x };`,
`export { y as z };`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(`export { x, y as z };`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine property re-exports", () => {
const sortedExports = parseExports(
`export { x } from "lib";`,
`export { y as z } from "lib";`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(`export { x, y as z } from "lib";`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine namespace re-export with property re-export", () => {
const sortedExports = parseExports(
`export * from "lib";`,
`export { y } from "lib";`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = sortedExports;
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine many exports", () => {
const sortedExports = parseExports(
`export { x };`,
`export { y as w, z as default };`,
`export { w as q };`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(
`export { w as q, x, y as w, z as default };`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});

it("Combine many re-exports", () => {
const sortedExports = parseExports(
`export { x as a, y } from "lib";`,
`export * from "lib";`,
`export { z as b } from "lib";`);
const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports);
const expectedCoalescedExports = parseExports(
`export * from "lib";`,
`export { x as a, y, z as b } from "lib";`);
assertListEqual(actualCoalescedExports, expectedCoalescedExports);
});
});

describe("Baselines", () => {

const libFile = {
Expand Down Expand Up @@ -471,6 +536,154 @@ import { React, Other } from "react";
},
reactLibFile);

describe("Exports", () => {

testOrganizeExports("MoveToTop",
{
path: "/test.ts",
content: `
export { F1, F2 } from "lib";
1;
export * from "lib";
2;
`,
},
libFile);

// tslint:disable no-invalid-template-strings
testOrganizeExports("MoveToTop_Invalid",
{
path: "/test.ts",
content: `
export { F1, F2 } from "lib";
1;
export * from "lib";
2;
export { b } from ${"`${'lib'}`"};
export { a } from ${"`${'lib'}`"};
export { D } from "lib";
3;
`,
},
libFile);
// tslint:enable no-invalid-template-strings

testOrganizeExports("MoveToTop_WithImportsFirst",
{
path: "/test.ts",
content: `
import { F1, F2 } from "lib";
1;
export { F1, F2 } from "lib";
2;
import * as NS from "lib";
3;
export * from "lib";
4;
F1(); F2(); NS.F1();
`,
},
libFile);

testOrganizeExports("MoveToTop_WithExportsFirst",
{
path: "/test.ts",
content: `
export { F1, F2 } from "lib";
1;
import { F1, F2 } from "lib";
2;
export * from "lib";
3;
import * as NS from "lib";
4;
F1(); F2(); NS.F1();
`,
},
libFile);

testOrganizeExports("CoalesceMultipleModules",
{
path: "/test.ts",
content: `
export { d } from "lib1";
export { b } from "lib1";
export { c } from "lib2";
export { a } from "lib2";
`,
},
{ path: "/lib1.ts", content: "export const b = 1, d = 2;" },
{ path: "/lib2.ts", content: "export const a = 3, c = 4;" });

testOrganizeExports("CoalesceTrivia",
{
path: "/test.ts",
content: `
/*A*/export /*B*/ { /*C*/ F2 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/;/*H*/ //I
/*J*/export /*K*/ { /*L*/ F1 /*M*/ } /*N*/ from /*O*/ "lib" /*P*/;/*Q*/ //R
`,
},
libFile);

testOrganizeExports("SortTrivia",
{
path: "/test.ts",
content: `
/*A*/export /*B*/ * /*C*/ from /*D*/ "lib2" /*E*/;/*F*/ //G
/*H*/export /*I*/ * /*J*/ from /*K*/ "lib1" /*L*/;/*M*/ //N
`,
},
{ path: "/lib1.ts", content: "" },
{ path: "/lib2.ts", content: "" });

testOrganizeExports("SortHeaderComment",
{
path: "/test.ts",
content: `
// Header
export * from "lib2";
export * from "lib1";
`,
},
{ path: "/lib1.ts", content: "" },
{ path: "/lib2.ts", content: "" });

testOrganizeExports("AmbientModule",
{
path: "/test.ts",
content: `
declare module "mod" {
export { F1 } from "lib";
export * from "lib";
export { F2 } from "lib";
}
`,
},
libFile);

testOrganizeExports("TopLevelAndAmbientModule",
{
path: "/test.ts",
content: `
export { D } from "lib";

declare module "mod" {
export { F1 } from "lib";
export * from "lib";
export { F2 } from "lib";
}

export { E } from "lib";
export * from "lib";
`,
},
libFile);
});

function testOrganizeExports(testName: string, testFile: TestFSWithWatch.File, ...otherFiles: TestFSWithWatch.File[]) {
testOrganizeImports(`${testName}.exports`, testFile, ...otherFiles);
}

function testOrganizeImports(testName: string, testFile: TestFSWithWatch.File, ...otherFiles: TestFSWithWatch.File[]) {
it(testName, () => runBaseline(`organizeImports/${testName}.ts`, testFile, ...otherFiles));
}
Expand Down Expand Up @@ -509,6 +722,13 @@ import { React, Other } from "react";
return imports;
}

function parseExports(...exportStrings: string[]): ReadonlyArray<ExportDeclaration> {
const sourceFile = createSourceFile("a.ts", exportStrings.join("\n"), ScriptTarget.ES2015, /*setParentNodes*/ true, ScriptKind.TS);
const exports = filter(sourceFile.statements, isExportDeclaration);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this isn't really a filter, this and parseImports could use a helper function:

function castEvery<T, U extends T>(inputs: ReadonlyArray<T>, pred: (t: T) => t is U): ReadonlyArray<U> {
    for (const input of inputs)
        Debug.assert(pred(input));
    return inputs as ReadonlyArray<U>;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what benefit this provides.

assert.equal(exports.length, exportStrings.length);
return exports;
}

function assertEqual(node1?: Node, node2?: Node) {
if (node1 === undefined) {
assert.isUndefined(node2);
Expand Down Expand Up @@ -550,6 +770,23 @@ import { React, Other } from "react";
assertEqual(is1.name, is2.name);
assertEqual(is1.propertyName, is2.propertyName);
break;
case SyntaxKind.ExportDeclaration:
const ed1 = node1 as ExportDeclaration;
const ed2 = node2 as ExportDeclaration;
assertEqual(ed1.exportClause, ed2.exportClause);
assertEqual(ed1.moduleSpecifier, ed2.moduleSpecifier);
break;
case SyntaxKind.NamedExports:
const ne1 = node1 as NamedExports;
const ne2 = node2 as NamedExports;
assertListEqual(ne1.elements, ne2.elements);
break;
case SyntaxKind.ExportSpecifier:
const es1 = node1 as ExportSpecifier;
const es2 = node2 as ExportSpecifier;
assertEqual(es1.name, es2.name);
assertEqual(es1.propertyName, es2.propertyName);
break;
case SyntaxKind.Identifier:
const id1 = node1 as Identifier;
const id2 = node2 as Identifier;
Expand Down
Loading