diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index e7731a21f72..be0392a3eba 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -174,7 +174,7 @@ namespace ts.OrganizeImports { return importGroup; } - const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getCategorizedImports(importGroup); + const { importWithoutClause, typeOnlyImports, regularImports } = getCategorizedImports(importGroup); const coalescedImports: ImportDeclaration[] = []; @@ -182,107 +182,126 @@ namespace ts.OrganizeImports { 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 defaultImport = defaultImports[0]; - coalescedImports.push( - updateImportDeclarationAndClause(defaultImport, defaultImport.importClause!.name, namespaceImports[0].importClause!.namedBindings)); // TODO: GH#18217 + for (const group of [regularImports, typeOnlyImports]) { + const isTypeOnly = group === typeOnlyImports; + const { defaultImports, namespaceImports, namedImports } = group; + // Normally, we don't combine default and namespace imports, but it would be silly to + // produce two import declarations in this special case. + if (!isTypeOnly && defaultImports.length === 1 && namespaceImports.length === 1 && namedImports.length === 0) { + // Add the namespace import to the existing default ImportDeclaration. + const defaultImport = defaultImports[0]; + coalescedImports.push( + updateImportDeclarationAndClause(defaultImport, defaultImport.importClause!.name, namespaceImports[0].importClause!.namedBindings)); // TODO: GH#18217 - return coalescedImports; - } + continue; + } - const sortedNamespaceImports = stableSort(namespaceImports, (i1, i2) => - compareIdentifiers((i1.importClause!.namedBindings as NamespaceImport).name, (i2.importClause!.namedBindings as NamespaceImport).name)); // TODO: GH#18217 + const sortedNamespaceImports = stableSort(namespaceImports, (i1, i2) => + compareIdentifiers((i1.importClause!.namedBindings as NamespaceImport).name, (i2.importClause!.namedBindings as NamespaceImport).name)); // TODO: GH#18217 - for (const namespaceImport of sortedNamespaceImports) { - // Drop the name, if any - coalescedImports.push( - updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause!.namedBindings)); // TODO: GH#18217 - } + for (const namespaceImport of sortedNamespaceImports) { + // Drop the name, if any + coalescedImports.push( + updateImportDeclarationAndClause(namespaceImport, /*name*/ undefined, namespaceImport.importClause!.namedBindings)); // TODO: GH#18217 + } - if (defaultImports.length === 0 && namedImports.length === 0) { - return coalescedImports; - } + if (defaultImports.length === 0 && namedImports.length === 0) { + continue; + } - let newDefaultImport: Identifier | undefined; - const newImportSpecifiers: ImportSpecifier[] = []; - if (defaultImports.length === 1) { - newDefaultImport = defaultImports[0].importClause!.name; - } - else { - for (const defaultImport of defaultImports) { - newImportSpecifiers.push( - createImportSpecifier(createIdentifier("default"), defaultImport.importClause!.name!)); // TODO: GH#18217 + let newDefaultImport: Identifier | undefined; + const newImportSpecifiers: ImportSpecifier[] = []; + if (defaultImports.length === 1) { + newDefaultImport = defaultImports[0].importClause!.name; + } + else { + for (const defaultImport of defaultImports) { + newImportSpecifiers.push( + createImportSpecifier(createIdentifier("default"), defaultImport.importClause!.name!)); // TODO: GH#18217 + } + } + + newImportSpecifiers.push(...flatMap(namedImports, i => (i.importClause!.namedBindings as NamedImports).elements)); // TODO: GH#18217 + + const sortedImportSpecifiers = sortSpecifiers(newImportSpecifiers); + + const importDecl = defaultImports.length > 0 + ? defaultImports[0] + : namedImports[0]; + + const newNamedImports = sortedImportSpecifiers.length === 0 + ? newDefaultImport + ? undefined + : createNamedImports(emptyArray) + : namedImports.length === 0 + ? createNamedImports(sortedImportSpecifiers) + : updateNamedImports(namedImports[0].importClause!.namedBindings as NamedImports, sortedImportSpecifiers); // TODO: GH#18217 + + // Type-only imports are not allowed to combine + if (isTypeOnly && newDefaultImport && newNamedImports) { + coalescedImports.push( + updateImportDeclarationAndClause(importDecl, newDefaultImport, /*namedBindings*/ undefined)); + coalescedImports.push( + updateImportDeclarationAndClause(namedImports[0] ?? importDecl, /*name*/ undefined, newNamedImports)); + } + else { + coalescedImports.push( + updateImportDeclarationAndClause(importDecl, newDefaultImport, newNamedImports)); } } - newImportSpecifiers.push(...flatMap(namedImports, i => (i.importClause!.namedBindings as NamedImports).elements)); // TODO: GH#18217 - - const sortedImportSpecifiers = sortSpecifiers(newImportSpecifiers); - - const importDecl = defaultImports.length > 0 - ? defaultImports[0] - : namedImports[0]; - - const newNamedImports = sortedImportSpecifiers.length === 0 - ? newDefaultImport - ? undefined - : createNamedImports(emptyArray) - : namedImports.length === 0 - ? createNamedImports(sortedImportSpecifiers) - : updateNamedImports(namedImports[0].importClause!.namedBindings as NamedImports, sortedImportSpecifiers); // TODO: GH#18217 - - coalescedImports.push( - updateImportDeclarationAndClause(importDecl, newDefaultImport, newNamedImports)); - return coalescedImports; - /* - * Returns entire import declarations because they may already have been rewritten and - * may lack parent pointers. The desired parts can easily be recovered based on the - * categorization. - * - * NB: There may be overlap between `defaultImports` and `namespaceImports`/`namedImports`. - */ - function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { - let importWithoutClause: ImportDeclaration | undefined; - const defaultImports: ImportDeclaration[] = []; - const namespaceImports: ImportDeclaration[] = []; - const namedImports: ImportDeclaration[] = []; + } - 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; - } + interface ImportGroup { + defaultImports: ImportDeclaration[]; + namespaceImports: ImportDeclaration[]; + namedImports: ImportDeclaration[]; + } - const { name, namedBindings } = importDeclaration.importClause; + /* + * Returns entire import declarations because they may already have been rewritten and + * may lack parent pointers. The desired parts can easily be recovered based on the + * categorization. + * + * NB: There may be overlap between `defaultImports` and `namespaceImports`/`namedImports`. + */ + function getCategorizedImports(importGroup: readonly ImportDeclaration[]) { + let importWithoutClause: ImportDeclaration | undefined; + const typeOnlyImports: ImportGroup = { defaultImports: [], namespaceImports: [], namedImports: [] }; + const regularImports: ImportGroup = { defaultImports: [], namespaceImports: [], namedImports: [] }; - if (name) { - defaultImports.push(importDeclaration); - } - - if (namedBindings) { - if (isNamespaceImport(namedBindings)) { - namespaceImports.push(importDeclaration); - } - else { - namedImports.push(importDeclaration); - } - } + 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; } - return { - importWithoutClause, - defaultImports, - namespaceImports, - namedImports, - }; + const group = importDeclaration.importClause.isTypeOnly ? typeOnlyImports : regularImports; + const { name, namedBindings } = importDeclaration.importClause; + + if (name) { + group.defaultImports.push(importDeclaration); + } + + if (namedBindings) { + if (isNamespaceImport(namedBindings)) { + group.namespaceImports.push(importDeclaration); + } + else { + group.namedImports.push(importDeclaration); + } + } } + + return { + importWithoutClause, + typeOnlyImports, + regularImports, + }; } // Internal for testing @@ -294,7 +313,7 @@ namespace ts.OrganizeImports { return exportGroup; } - const { exportWithoutClause, namedExports } = getCategorizedExports(exportGroup); + const { exportWithoutClause, namedExports, typeOnlyExports } = getCategorizedExports(exportGroup); const coalescedExports: ExportDeclaration[] = []; @@ -302,29 +321,30 @@ namespace ts.OrganizeImports { coalescedExports.push(exportWithoutClause); } - if (namedExports.length === 0) { - return coalescedExports; + for (const exportGroup of [namedExports, typeOnlyExports]) { + if (exportGroup.length === 0) { + continue; + } + const newExportSpecifiers: ExportSpecifier[] = []; + newExportSpecifiers.push(...flatMap(exportGroup, i => i.exportClause && isNamedExports(i.exportClause) ? i.exportClause.elements : emptyArray)); + + const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers); + + const exportDecl = exportGroup[0]; + coalescedExports.push( + updateExportDeclaration( + exportDecl, + exportDecl.decorators, + exportDecl.modifiers, + exportDecl.exportClause && ( + isNamedExports(exportDecl.exportClause) ? + updateNamedExports(exportDecl.exportClause, sortedExportSpecifiers) : + updateNamespaceExport(exportDecl.exportClause, exportDecl.exportClause.name) + ), + exportDecl.moduleSpecifier, + exportDecl.isTypeOnly)); } - const newExportSpecifiers: ExportSpecifier[] = []; - newExportSpecifiers.push(...flatMap(namedExports, i => i.exportClause && isNamedExports(i.exportClause) ? i.exportClause.elements : emptyArray)); - - const sortedExportSpecifiers = sortSpecifiers(newExportSpecifiers); - - const exportDecl = namedExports[0]; - coalescedExports.push( - updateExportDeclaration( - exportDecl, - exportDecl.decorators, - exportDecl.modifiers, - exportDecl.exportClause && ( - isNamedExports(exportDecl.exportClause) ? - updateNamedExports(exportDecl.exportClause, sortedExportSpecifiers) : - updateNamespaceExport(exportDecl.exportClause, exportDecl.exportClause.name) - ), - exportDecl.moduleSpecifier, - exportDecl.isTypeOnly)); - return coalescedExports; /* @@ -335,6 +355,7 @@ namespace ts.OrganizeImports { function getCategorizedExports(exportGroup: readonly ExportDeclaration[]) { let exportWithoutClause: ExportDeclaration | undefined; const namedExports: ExportDeclaration[] = []; + const typeOnlyExports: ExportDeclaration[] = []; for (const exportDeclaration of exportGroup) { if (exportDeclaration.exportClause === undefined) { @@ -342,6 +363,9 @@ namespace ts.OrganizeImports { // Note: Unfortunately, we will lose trivia that was on this node. exportWithoutClause = exportWithoutClause || exportDeclaration; } + else if (exportDeclaration.isTypeOnly) { + typeOnlyExports.push(exportDeclaration); + } else { namedExports.push(exportDeclaration); } @@ -350,6 +374,7 @@ namespace ts.OrganizeImports { return { exportWithoutClause, namedExports, + typeOnlyExports, }; } } diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts index 984091a2aa1..149ed1d04b2 100644 --- a/src/testRunner/unittests/services/organizeImports.ts +++ b/src/testRunner/unittests/services/organizeImports.ts @@ -168,6 +168,28 @@ namespace ts { const expectedCoalescedImports = sortedImports; assertListEqual(actualCoalescedImports, expectedCoalescedImports); }); + + it("Combine type-only imports separately from other imports", () => { + const sortedImports = parseImports( + `import type { x } from "lib";`, + `import type { y } from "lib";`, + `import { z } from "lib";`); + const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports); + const expectedCoalescedImports = parseImports( + `import { z } from "lib";`, + `import type { x, y } from "lib";`); + assertListEqual(actualCoalescedImports, expectedCoalescedImports); + }); + + it("Do not combine type-only default, namespace, or named imports with each other", () => { + const sortedImports = parseImports( + `import type { x } from "lib";`, + `import type * as y from "lib";`, + `import type z from "lib";`); + const actualCoalescedImports = OrganizeImports.coalesceImports(sortedImports); + const expectedCoalescedImports = actualCoalescedImports; + assertListEqual(actualCoalescedImports, expectedCoalescedImports); + }); }); describe("Coalesce exports", () => { @@ -240,6 +262,25 @@ namespace ts { `export { x as a, y, z as b } from "lib";`); assertListEqual(actualCoalescedExports, expectedCoalescedExports); }); + + it("Keep type-only exports separate", () => { + const sortedExports = parseExports( + `export { x };`, + `export type { y };`); + const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports); + const expectedCoalescedExports = sortedExports; + assertListEqual(actualCoalescedExports, expectedCoalescedExports); + }); + + it("Combine type-only exports", () => { + const sortedExports = parseExports( + `export type { x };`, + `export type { y };`); + const actualCoalescedExports = OrganizeImports.coalesceExports(sortedExports); + const expectedCoalescedExports = parseExports( + `export type { x, y };`); + assertListEqual(actualCoalescedExports, expectedCoalescedExports); + }); }); describe("Baselines", () => { @@ -425,6 +466,18 @@ D(); libFile); /* eslint-enable no-template-curly-in-string */ + testOrganizeImports("TypeOnly", + { + path: "/test.ts", + content: ` +import { X } from "lib"; +import type Y from "lib"; +import { Z } from "lib"; +import type { A, B } from "lib"; + +export { A, B, X, Y, Z };` + }); + testOrganizeImports("CoalesceMultipleModules", { path: "/test.ts", diff --git a/tests/baselines/reference/organizeImports/TypeOnly.ts b/tests/baselines/reference/organizeImports/TypeOnly.ts new file mode 100644 index 00000000000..dcd18e7ef86 --- /dev/null +++ b/tests/baselines/reference/organizeImports/TypeOnly.ts @@ -0,0 +1,15 @@ +// ==ORIGINAL== + +import { X } from "lib"; +import type Y from "lib"; +import { Z } from "lib"; +import type { A, B } from "lib"; + +export { A, B, X, Y, Z }; +// ==ORGANIZED== + +import { X, Z } from "lib"; +import type Y from "lib"; +import type { A, B } from "lib"; + +export { A, B, X, Y, Z };