Merge pull request #22087 from amcasey/AmbientModuleImports

Organize imports in ambient modules

Author: amcasey

src/compiler/utilities.ts

@@ -425,8 +425,8 @@ namespace ts {
     }
 
     export function isAmbientModule(node: Node): boolean {
-        return node && node.kind === SyntaxKind.ModuleDeclaration &&
-            ((<ModuleDeclaration>node).name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(<ModuleDeclaration>node));
+        return node && isModuleDeclaration(node) &&
+            (node.name.kind === SyntaxKind.StringLiteral || isGlobalScopeAugmentation(node));
     }
 
     export function isModuleWithStringLiteralName(node: Node): node is ModuleDeclaration {

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

src/services/organizeImports.ts

@@ -13,46 +13,55 @@ 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 oldImportDecls = sourceFile.statements.filter(isImportDeclaration);
+        const topLevelImportDecls = sourceFile.statements.filter(isImportDeclaration);
+        organizeImportsWorker(topLevelImportDecls);
 
-        if (oldImportDecls.length === 0) {
-            return [];
+        for (const ambientModule of sourceFile.statements.filter(isAmbientModule)) {
+            const ambientModuleBody = getModuleBlock(ambientModule as ModuleDeclaration);
+            const ambientModuleImportDecls = ambientModuleBody.statements.filter(isImportDeclaration);
+            organizeImportsWorker(ambientModuleImportDecls);
         }
 
-        const oldImportGroups = group(oldImportDecls, importDecl => getExternalModuleName(importDecl.moduleSpecifier));
+        return changeTracker.getChanges();
 
-        const sortedImportGroups = stableSort(oldImportGroups, (group1, group2) =>
-            compareModuleSpecifiers(group1[0].moduleSpecifier, group2[0].moduleSpecifier));
+        function organizeImportsWorker(oldImportDecls: ReadonlyArray<ImportDeclaration>) {
+            if (length(oldImportDecls) === 0) {
+                return;
+            }
 
-        const newImportDecls = flatMap(sortedImportGroups, importGroup =>
-            getExternalModuleName(importGroup[0].moduleSpecifier)
-                ? coalesceImports(removeUnusedImports(importGroup, sourceFile, program))
-                : importGroup);
+            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 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 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]);
+            // Delete any subsequent imports.
+            for (let i = 1; i < oldImportDecls.length; i++) {
+                changeTracker.deleteNode(sourceFile, oldImportDecls[i]);
+            }
         }
+    }
 
-        return changeTracker.getChanges();
+    function getModuleBlock(moduleDecl: ModuleDeclaration): ModuleBlock | undefined {
+        const body = moduleDecl.body;
+        return body && !isIdentifier(body) && (isModuleBlock(body) ? body : getModuleBlock(body));
     }
 
     function removeUnusedImports(oldImports: ReadonlyArray<ImportDeclaration>, sourceFile: SourceFile, program: Program) {

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) {}
+}

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();