Skip to content

🤖 Cherry-pick PR #36807 into release-3.8 #36810

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 1 commit into from
Feb 14, 2020
Merged

🤖 Cherry-pick PR #36807 into release-3.8 #36810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
233 changes: 129 additions & 104 deletions src/services/organizeImports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,115 +174,134 @@ namespace ts.OrganizeImports {
return importGroup;
}

const { importWithoutClause, defaultImports, namespaceImports, namedImports } = getCategorizedImports(importGroup);
const { importWithoutClause, typeOnlyImports, regularImports } = getCategorizedImports(importGroup);

const coalescedImports: ImportDeclaration[] = [];

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 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
newImportSpecifiers.push(...flatMap(namedImports, i => (i.importClause!.namedBindings as NamedImports).elements)); // TODO: GH#18217

const sortedImportSpecifiers = sortSpecifiers(newImportSpecifiers);
const sortedImportSpecifiers = sortSpecifiers(newImportSpecifiers);

const importDecl = defaultImports.length > 0
? defaultImports[0]
: namedImports[0];
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
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));
// 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));
}
}

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;
}
}

const { name, namedBindings } = importDeclaration.importClause;
interface ImportGroup {
defaultImports: ImportDeclaration[];
namespaceImports: ImportDeclaration[];
namedImports: ImportDeclaration[];
}

if (name) {
defaultImports.push(importDeclaration);
}
/*
* 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: [] };

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;
}

if (namedBindings) {
if (isNamespaceImport(namedBindings)) {
namespaceImports.push(importDeclaration);
}
else {
namedImports.push(importDeclaration);
}
}
const group = importDeclaration.importClause.isTypeOnly ? typeOnlyImports : regularImports;
const { name, namedBindings } = importDeclaration.importClause;

if (name) {
group.defaultImports.push(importDeclaration);
}

return {
importWithoutClause,
defaultImports,
namespaceImports,
namedImports,
};
if (namedBindings) {
if (isNamespaceImport(namedBindings)) {
group.namespaceImports.push(importDeclaration);
}
else {
group.namedImports.push(importDeclaration);
}
}
}

return {
importWithoutClause,
typeOnlyImports,
regularImports,
};
}

// Internal for testing
Expand All @@ -294,37 +313,38 @@ namespace ts.OrganizeImports {
return exportGroup;
}

const { exportWithoutClause, namedExports } = getCategorizedExports(exportGroup);
const { exportWithoutClause, namedExports, typeOnlyExports } = getCategorizedExports(exportGroup);

const coalescedExports: ExportDeclaration[] = [];

if (exportWithoutClause) {
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;

/*
Expand All @@ -335,13 +355,17 @@ 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) {
// 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 if (exportDeclaration.isTypeOnly) {
typeOnlyExports.push(exportDeclaration);
}
else {
namedExports.push(exportDeclaration);
}
Expand All @@ -350,6 +374,7 @@ namespace ts.OrganizeImports {
return {
exportWithoutClause,
namedExports,
typeOnlyExports,
};
}
}
Expand Down
53 changes: 53 additions & 0 deletions src/testRunner/unittests/services/organizeImports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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",
Expand Down
15 changes: 15 additions & 0 deletions tests/baselines/reference/organizeImports/TypeOnly.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 };