diff --git a/src/harness/unittests/organizeImports.ts b/src/harness/unittests/organizeImports.ts index d8f15ce66bd77..94f99856777e7 100644 --- a/src/harness/unittests/organizeImports.ts +++ b/src/harness/unittests/organizeImports.ts @@ -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); @@ -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 = { @@ -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)); } @@ -509,6 +722,13 @@ import { React, Other } from "react"; return imports; } + function parseExports(...exportStrings: string[]): ReadonlyArray { + const sourceFile = createSourceFile("a.ts", exportStrings.join("\n"), ScriptTarget.ES2015, /*setParentNodes*/ true, ScriptKind.TS); + const exports = filter(sourceFile.statements, isExportDeclaration); + assert.equal(exports.length, exportStrings.length); + return exports; + } + function assertEqual(node1?: Node, node2?: Node) { if (node1 === undefined) { assert.isUndefined(node2); @@ -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; diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index fa30b5cd30e6c..99bac64973f80 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -17,19 +17,32 @@ namespace ts.OrganizeImports { const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext }); + const coalesceAndOrganizeImports = (importGroup: ReadonlyArray) => coalesceImports(removeUnusedImports(importGroup, sourceFile, program)); + // All of the old ImportDeclarations in the file, in syntactic order. const topLevelImportDecls = sourceFile.statements.filter(isImportDeclaration); - organizeImportsWorker(topLevelImportDecls); + organizeImportsWorker(topLevelImportDecls, coalesceAndOrganizeImports); + + // All of the old ExportDeclarations in the file, in syntactic order. + const topLevelExportDecls = sourceFile.statements.filter(isExportDeclaration); + organizeImportsWorker(topLevelExportDecls, coalesceExports); for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { const ambientModuleBody = getModuleBlock(ambientModule as ModuleDeclaration); + const ambientModuleImportDecls = ambientModuleBody.statements.filter(isImportDeclaration); - organizeImportsWorker(ambientModuleImportDecls); + organizeImportsWorker(ambientModuleImportDecls, coalesceAndOrganizeImports); + + const ambientModuleExportDecls = ambientModuleBody.statements.filter(isExportDeclaration); + organizeImportsWorker(ambientModuleExportDecls, coalesceExports); } return changeTracker.getChanges(); - function organizeImportsWorker(oldImportDecls: ReadonlyArray) { + function organizeImportsWorker( + oldImportDecls: ReadonlyArray, + coalesce: (group: ReadonlyArray) => ReadonlyArray) { + if (length(oldImportDecls) === 0) { return; } @@ -45,7 +58,7 @@ namespace ts.OrganizeImports { 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, sourceFile, program)) + ? coalesce(importGroup) : importGroup); // Delete or replace the first import. @@ -131,7 +144,9 @@ namespace ts.OrganizeImports { } function getExternalModuleName(specifier: Expression) { - return isStringLiteralLike(specifier) ? specifier.text : undefined; + return specifier !== undefined && isStringLiteralLike(specifier) + ? specifier.text + : undefined; } /* @internal */ // Internal for testing @@ -189,9 +204,7 @@ namespace ts.OrganizeImports { newImportSpecifiers.push(...flatMap(namedImports, i => (i.importClause.namedBindings as NamedImports).elements)); - const sortedImportSpecifiers = stableSort(newImportSpecifiers, (s1, s2) => - compareIdentifiers(s1.propertyName || s1.name, s2.propertyName || s2.name) || - compareIdentifiers(s1.name, s2.name)); + const sortedImportSpecifiers = sortSpecifiers(newImportSpecifiers); const importDecl = defaultImports.length > 0 ? defaultImports[0] @@ -254,9 +267,69 @@ namespace ts.OrganizeImports { namedImports, }; } + } + + /* @internal */ // Internal for testing + /** + * @param exportGroup a list of ExportDeclarations, all with the same module name. + */ + export function coalesceExports(exportGroup: ReadonlyArray) { + if (exportGroup.length === 0) { + return exportGroup; + } + + const { exportWithoutClause, namedExports } = getCategorizedExports(exportGroup); + + const coalescedExports: ExportDeclaration[] = []; + + if (exportWithoutClause) { + coalescedExports.push(exportWithoutClause); + } + + if (namedExports.length === 0) { + return coalescedExports; + } + + const newExportSpecifiers: ExportSpecifier[] = []; + newExportSpecifiers.push(...flatMap(namedExports, i => (i.exportClause).elements)); + + const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers); - function compareIdentifiers(s1: Identifier, s2: Identifier) { - return compareStringsCaseInsensitive(s1.text, s2.text); + const exportDecl = namedExports[0]; + coalescedExports.push( + updateExportDeclaration( + exportDecl, + exportDecl.decorators, + exportDecl.modifiers, + updateNamedExports(exportDecl.exportClause, sortedExportSpecifiers), + exportDecl.moduleSpecifier)); + + return coalescedExports; + + /* + * Returns entire export declarations because they may already have been rewritten and + * may lack parent pointers. The desired parts can easily be recovered based on the + * categorization. + */ + function getCategorizedExports(exportGroup: ReadonlyArray) { + let exportWithoutClause: ExportDeclaration | undefined; + const namedExports: ExportDeclaration[] = []; + + for (const exportDeclaration of exportGroup) { + if (exportDeclaration.exportClause === undefined) { + // Only the first such export is interesting - the others are redundant. + // Note: Unfortunately, we will lose trivia that was on this node. + exportWithoutClause = exportWithoutClause || exportDeclaration; + } + else { + namedExports.push(exportDeclaration); + } + } + + return { + exportWithoutClause, + namedExports, + }; } } @@ -273,6 +346,12 @@ namespace ts.OrganizeImports { importDeclaration.moduleSpecifier); } + function sortSpecifiers(specifiers: ReadonlyArray) { + return stableSort(specifiers, (s1, s2) => + compareIdentifiers(s1.propertyName || s1.name, s2.propertyName || s2.name) || + compareIdentifiers(s1.name, s2.name)); + } + /* internal */ // Exported for testing export function compareModuleSpecifiers(m1: Expression, m2: Expression) { const name1 = getExternalModuleName(m1); @@ -281,4 +360,8 @@ namespace ts.OrganizeImports { compareBooleans(isExternalModuleNameRelative(name1), isExternalModuleNameRelative(name2)) || compareStringsCaseInsensitive(name1, name2); } -} \ No newline at end of file + + function compareIdentifiers(s1: Identifier, s2: Identifier) { + return compareStringsCaseInsensitive(s1.text, s2.text); + } +} diff --git a/tests/baselines/reference/organizeImports/AmbientModule.exports.ts b/tests/baselines/reference/organizeImports/AmbientModule.exports.ts new file mode 100644 index 0000000000000..80403e42f4eca --- /dev/null +++ b/tests/baselines/reference/organizeImports/AmbientModule.exports.ts @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +declare module "mod" { + export { F1 } from "lib"; + export * from "lib"; + export { F2 } from "lib"; +} + +// ==ORGANIZED== + +declare module "mod" { + export * from "lib"; + export { F1, F2 } from "lib"; +} + \ No newline at end of file diff --git a/tests/baselines/reference/organizeImports/CoalesceMultipleModules.exports.ts b/tests/baselines/reference/organizeImports/CoalesceMultipleModules.exports.ts new file mode 100644 index 0000000000000..cb5114b0c25c0 --- /dev/null +++ b/tests/baselines/reference/organizeImports/CoalesceMultipleModules.exports.ts @@ -0,0 +1,11 @@ +// ==ORIGINAL== + +export { d } from "lib1"; +export { b } from "lib1"; +export { c } from "lib2"; +export { a } from "lib2"; + +// ==ORGANIZED== + +export { b, d } from "lib1"; +export { a, c } from "lib2"; diff --git a/tests/baselines/reference/organizeImports/CoalesceTrivia.exports.ts b/tests/baselines/reference/organizeImports/CoalesceTrivia.exports.ts new file mode 100644 index 0000000000000..f0ff1f550cd79 --- /dev/null +++ b/tests/baselines/reference/organizeImports/CoalesceTrivia.exports.ts @@ -0,0 +1,8 @@ +// ==ORIGINAL== + +/*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 + +// ==ORGANIZED== + +/*A*/export /*B*/ { /*L*/ F1 /*M*/, /*C*/ F2 /*D*/ } /*E*/ from /*F*/ "lib" /*G*/; /*H*/ //I diff --git a/tests/baselines/reference/organizeImports/MoveToTop.exports.ts b/tests/baselines/reference/organizeImports/MoveToTop.exports.ts new file mode 100644 index 0000000000000..85016084b4875 --- /dev/null +++ b/tests/baselines/reference/organizeImports/MoveToTop.exports.ts @@ -0,0 +1,13 @@ +// ==ORIGINAL== + +export { F1, F2 } from "lib"; +1; +export * from "lib"; +2; + +// ==ORGANIZED== + +export * from "lib"; +export { F1, F2 } from "lib"; +1; +2; diff --git a/tests/baselines/reference/organizeImports/MoveToTop_Invalid.exports.ts b/tests/baselines/reference/organizeImports/MoveToTop_Invalid.exports.ts new file mode 100644 index 0000000000000..6479e78c698da --- /dev/null +++ b/tests/baselines/reference/organizeImports/MoveToTop_Invalid.exports.ts @@ -0,0 +1,20 @@ +// ==ORIGINAL== + +export { F1, F2 } from "lib"; +1; +export * from "lib"; +2; +export { b } from `${'lib'}`; +export { a } from `${'lib'}`; +export { D } from "lib"; +3; + +// ==ORGANIZED== + +export * from "lib"; +export { D, F1, F2 } from "lib"; +export { b } from `${'lib'}`; +export { a } from `${'lib'}`; +1; +2; +3; diff --git a/tests/baselines/reference/organizeImports/MoveToTop_WithExportsFirst.exports.ts b/tests/baselines/reference/organizeImports/MoveToTop_WithExportsFirst.exports.ts new file mode 100644 index 0000000000000..c52681b17a981 --- /dev/null +++ b/tests/baselines/reference/organizeImports/MoveToTop_WithExportsFirst.exports.ts @@ -0,0 +1,23 @@ +// ==ORIGINAL== + +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(); + +// ==ORGANIZED== + +export * from "lib"; +export { F1, F2 } from "lib"; +1; +import * as NS from "lib"; +import { F1, F2 } from "lib"; +2; +3; +4; +F1(); F2(); NS.F1(); diff --git a/tests/baselines/reference/organizeImports/MoveToTop_WithImportsFirst.exports.ts b/tests/baselines/reference/organizeImports/MoveToTop_WithImportsFirst.exports.ts new file mode 100644 index 0000000000000..5dc908f63a189 --- /dev/null +++ b/tests/baselines/reference/organizeImports/MoveToTop_WithImportsFirst.exports.ts @@ -0,0 +1,23 @@ +// ==ORIGINAL== + +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(); + +// ==ORGANIZED== + +import * as NS from "lib"; +import { F1, F2 } from "lib"; +1; +export * from "lib"; +export { F1, F2 } from "lib"; +2; +3; +4; +F1(); F2(); NS.F1(); diff --git a/tests/baselines/reference/organizeImports/SortHeaderComment.exports.ts b/tests/baselines/reference/organizeImports/SortHeaderComment.exports.ts new file mode 100644 index 0000000000000..e3c20f3b14090 --- /dev/null +++ b/tests/baselines/reference/organizeImports/SortHeaderComment.exports.ts @@ -0,0 +1,11 @@ +// ==ORIGINAL== + +// Header +export * from "lib2"; +export * from "lib1"; + +// ==ORGANIZED== + +// Header +export * from "lib1"; +export * from "lib2"; diff --git a/tests/baselines/reference/organizeImports/SortTrivia.exports.ts b/tests/baselines/reference/organizeImports/SortTrivia.exports.ts new file mode 100644 index 0000000000000..459b3ebbdfa3c --- /dev/null +++ b/tests/baselines/reference/organizeImports/SortTrivia.exports.ts @@ -0,0 +1,9 @@ +// ==ORIGINAL== + +/*A*/export /*B*/ * /*C*/ from /*D*/ "lib2" /*E*/;/*F*/ //G +/*H*/export /*I*/ * /*J*/ from /*K*/ "lib1" /*L*/;/*M*/ //N + +// ==ORGANIZED== + +/*A*//*H*/ export /*I*/ * /*J*/ from /*K*/ "lib1" /*L*/; /*M*/ //N +export /*B*/ * /*C*/ from /*D*/ "lib2" /*E*/; /*F*/ //G diff --git a/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.exports.ts b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.exports.ts new file mode 100644 index 0000000000000..53124c103b54a --- /dev/null +++ b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.exports.ts @@ -0,0 +1,23 @@ +// ==ORIGINAL== + +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"; + +// ==ORGANIZED== + +export * from "lib"; +export { D, E } from "lib"; + +declare module "mod" { + export * from "lib"; + export { F1, F2 } from "lib"; +} +