From 427e6ed3e6e71154bcb01fa6c89b788751a4abfe Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 20 Feb 2018 16:46:03 -0800 Subject: [PATCH 1/3] Tidy isAmbientModule --- src/compiler/utilities.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 863e8107f33..7fb4f0bfcba 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -425,8 +425,8 @@ namespace ts { } export function isAmbientModule(node: Node): boolean { - return node && node.kind === SyntaxKind.ModuleDeclaration && - ((node).name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(node)); + return node && isModuleDeclaration(node) && + (node.name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(node)); } export function isModuleWithStringLiteralName(node: Node): node is ModuleDeclaration { From 189eb505b93473f583ef38260cc64ef39678cbb1 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 20 Feb 2018 16:42:15 -0800 Subject: [PATCH 2/3] Factor worker method out of ts.OrganizeImports.organizeImports --- src/services/organizeImports.ts | 67 ++++++++++++++++----------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 082dfc4130d..45fa6bddfa6 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -16,43 +16,42 @@ namespace ts.OrganizeImports { // TODO (https://github.com/Microsoft/TypeScript/issues/10020): sort *within* ambient modules (find using isAmbientModule) // All of the old ImportDeclarations in the file, in syntactic order. - const oldImportDecls = sourceFile.statements.filter(isImportDeclaration); - - if (oldImportDecls.length === 0) { - return []; - } - - const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier)); - - 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)) - : importGroup); + const topLevelImportDecls = sourceFile.statements.filter(isImportDeclaration); const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext }); - - // Delete or replace the first import. - if (newImportDecls.length === 0) { - changeTracker.deleteNode(sourceFile, oldImportDecls[0]); - } - else { - // Note: Delete the surrounding trivia because it will have been retained in newImportDecls. - changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, { - useNonAdjustedStartPosition: false, - useNonAdjustedEndPosition: false, - suffix: getNewLineOrDefaultFromHost(host, formatContext.options), - }); - } - - // Delete any subsequent imports. - for (let i = 1; i < oldImportDecls.length; i++) { - changeTracker.deleteNode(sourceFile, oldImportDecls[i]); - } - + organizeImportsWorker(topLevelImportDecls); return changeTracker.getChanges(); + + function organizeImportsWorker(oldImportDecls: ReadonlyArray) { + if (length(oldImportDecls) === 0) { + return; + } + + const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier)); + 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)) + : importGroup); + + // Delete or replace the first import. + if (newImportDecls.length === 0) { + changeTracker.deleteNode(sourceFile, oldImportDecls[0]); + } + else { + // Note: Delete the surrounding trivia because it will have been retained in newImportDecls. + changeTracker.replaceNodeWithNodes(sourceFile, oldImportDecls[0], newImportDecls, { + useNonAdjustedStartPosition: false, + useNonAdjustedEndPosition: false, + suffix: getNewLineOrDefaultFromHost(host, formatContext.options), + }); + } + + // Delete any subsequent imports. + for (let i = 1; i < oldImportDecls.length; i++) { + changeTracker.deleteNode(sourceFile, oldImportDecls[i]); + } + } } function removeUnusedImports(oldImports: ReadonlyArray, sourceFile: SourceFile, program: Program) { From 8ead7ab29c2f7e5dc6b05f7087076b3e9f6657d7 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 20 Feb 2018 17:41:44 -0800 Subject: [PATCH 3/3] Organize imports within ambient module declarations --- src/harness/unittests/organizeImports.ts | 37 +++++++++++++++++++ src/services/organizeImports.ts | 16 ++++++-- .../organizeImports/AmbientModule.ts | 17 +++++++++ .../TopLevelAndAmbientModule.ts | 30 +++++++++++++++ 4 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/baselines/reference/organizeImports/AmbientModule.ts create mode 100644 tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts diff --git a/src/harness/unittests/organizeImports.ts b/src/harness/unittests/organizeImports.ts index 0b9781462a4..bb5c9cbee18 100644 --- a/src/harness/unittests/organizeImports.ts +++ b/src/harness/unittests/organizeImports.ts @@ -325,6 +325,43 @@ F2(); /*A*/import /*B*/ { /*C*/ F1 /*D*/, /*E*/ F2 /*F*/ } /*G*/ from /*H*/ "lib" /*I*/;/*J*/ //K F1(); +`, + }, + libFile); + + testOrganizeImports("AmbientModule", + { + path: "/test.ts", + content: ` +declare module "mod" { + import { F1 } from "lib"; + import * as NS from "lib"; + import { F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} +`, + }, + libFile); + + testOrganizeImports("TopLevelAndAmbientModule", + { + path: "/test.ts", + content: ` +import D from "lib"; + +declare module "mod" { + import { F1 } from "lib"; + import * as NS from "lib"; + import { F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} + +import E from "lib"; +import "lib"; + +D(); `, }, libFile); diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts index 45fa6bddfa6..511fa821fd2 100644 --- a/src/services/organizeImports.ts +++ b/src/services/organizeImports.ts @@ -13,13 +13,18 @@ namespace ts.OrganizeImports { host: LanguageServiceHost, program: Program) { - // TODO (https://github.com/Microsoft/TypeScript/issues/10020): sort *within* ambient modules (find using isAmbientModule) + const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext }); // All of the old ImportDeclarations in the file, in syntactic order. const topLevelImportDecls = sourceFile.statements.filter(isImportDeclaration); - - const changeTracker = textChanges.ChangeTracker.fromContext({ host, formatContext }); organizeImportsWorker(topLevelImportDecls); + + for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) { + const ambientModuleBody = getModuleBlock(ambientModule as ModuleDeclaration); + const ambientModuleImportDecls = ambientModuleBody.statements.filter(isImportDeclaration); + organizeImportsWorker(ambientModuleImportDecls); + } + return changeTracker.getChanges(); function organizeImportsWorker(oldImportDecls: ReadonlyArray) { @@ -54,6 +59,11 @@ namespace ts.OrganizeImports { } } + function getModuleBlock(moduleDecl: ModuleDeclaration): ModuleBlock | undefined { + const body = moduleDecl.body; + return body && !isIdentifier(body) && (isModuleBlock(body) ? body : getModuleBlock(body)); + } + function removeUnusedImports(oldImports: ReadonlyArray, sourceFile: SourceFile, program: Program) { const typeChecker = program.getTypeChecker(); const jsxNamespace = typeChecker.getJsxNamespace(); diff --git a/tests/baselines/reference/organizeImports/AmbientModule.ts b/tests/baselines/reference/organizeImports/AmbientModule.ts new file mode 100644 index 00000000000..4a286610742 --- /dev/null +++ b/tests/baselines/reference/organizeImports/AmbientModule.ts @@ -0,0 +1,17 @@ +// ==ORIGINAL== + +declare module "mod" { + import { F1 } from "lib"; + import * as NS from "lib"; + import { F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} + +// ==ORGANIZED== + +declare module "mod" { + import { F1, F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} diff --git a/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts new file mode 100644 index 00000000000..d6f3158efe8 --- /dev/null +++ b/tests/baselines/reference/organizeImports/TopLevelAndAmbientModule.ts @@ -0,0 +1,30 @@ +// ==ORIGINAL== + +import D from "lib"; + +declare module "mod" { + import { F1 } from "lib"; + import * as NS from "lib"; + import { F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} + +import E from "lib"; +import "lib"; + +D(); + +// ==ORGANIZED== + +import "lib"; +import D from "lib"; + +declare module "mod" { + import { F1, F2 } from "lib"; + + function F(f1: {} = F1, f2: {} = F2) {} +} + + +D();