Merge pull request microsoft#31482 from andrewbranch/bug/31338

andrewbranch · web-flow · commit b8dcf27c3e90 · 2019-05-30T14:01:55.000-07:00
Organize imports: don’t delete import declarations used for module augmentation
diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts
@@ -89,7 +89,7 @@ namespace ts.OrganizeImports {
         const usedImports: ImportDeclaration[] = [];
 
         for (const importDecl of oldImports) {
-            const {importClause} = importDecl;
+            const { importClause, moduleSpecifier } = importDecl;
 
             if (!importClause) {
                 // Imports without import clauses are assumed to be included for their side effects and are not removed.
@@ -125,6 +125,23 @@ namespace ts.OrganizeImports {
             if (name || namedBindings) {
                 usedImports.push(updateImportDeclarationAndClause(importDecl, name, namedBindings));
             }
+            // If a module is imported to be augmented, it’s used
+            else if (hasModuleDeclarationMatchingSpecifier(sourceFile, moduleSpecifier)) {
+                // If we’re in a declaration file, it’s safe to remove the import clause from it
+                if (sourceFile.isDeclarationFile) {
+                    usedImports.push(createImportDeclaration(
+                        importDecl.decorators,
+                        importDecl.modifiers,
+                        /*importClause*/ undefined,
+                        moduleSpecifier));
+                }
+                // If we’re not in a declaration file, we can’t remove the import clause even though
+                // the imported symbols are unused, because removing them makes it look like the import
+                // declaration has side effects, which will cause it to be preserved in the JS emit.
+                else {
+                    usedImports.push(importDecl);
+                }
+            }
         }
 
         return usedImports;
@@ -135,6 +152,13 @@ namespace ts.OrganizeImports {
         }
     }
 
+    function hasModuleDeclarationMatchingSpecifier(sourceFile: SourceFile, moduleSpecifier: Expression) {
+        const moduleSpecifierText = isStringLiteral(moduleSpecifier) && moduleSpecifier.text;
+        return isString(moduleSpecifierText) && some(sourceFile.moduleAugmentations, moduleName =>
+            isStringLiteral(moduleName)
+            && moduleName.text === moduleSpecifierText);
+    }
+
     function getExternalModuleName(specifier: Expression) {
         return specifier !== undefined && isStringLiteralLike(specifier)
             ? specifier.text
diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts
@@ -1,5 +1,5 @@
 namespace ts {
-    describe("unittests:: services:: Organize imports", () => {
+    describe("unittests:: services:: organizeImports", () => {
         describe("Sort imports", () => {
             it("Sort - non-relative vs non-relative", () => {
                 assertSortsBefore(
@@ -343,6 +343,36 @@ import { } from "lib";
                 },
                 libFile);
 
+            testOrganizeImports("Unused_false_positive_module_augmentation",
+            {
+                path: "/test.d.ts",
+                content: `
+import foo from 'foo';
+import { Caseless } from 'caseless';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}`
+            });
+
+            testOrganizeImports("Unused_preserve_imports_for_module_augmentation_in_non_declaration_file",
+            {
+                path: "/test.ts",
+                content: `
+import foo from 'foo';
+import { Caseless } from 'caseless';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}`
+            });
+
             testOrganizeImports("Unused_false_positive_shorthand_assignment",
             {
                     path: "/test.ts",
diff --git a/tests/baselines/reference/organizeImports/Unused_false_positive_module_augmentation.ts b/tests/baselines/reference/organizeImports/Unused_false_positive_module_augmentation.ts
@@ -0,0 +1,22 @@
+// ==ORIGINAL==
+
+import foo from 'foo';
+import { Caseless } from 'caseless';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}
+// ==ORGANIZED==
+
+import 'caseless';
+import 'foo';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}
diff --git a/tests/baselines/reference/organizeImports/Unused_preserve_imports_for_module_augmentation_in_non_declaration_file.ts b/tests/baselines/reference/organizeImports/Unused_preserve_imports_for_module_augmentation_in_non_declaration_file.ts
@@ -0,0 +1,22 @@
+// ==ORIGINAL==
+
+import foo from 'foo';
+import { Caseless } from 'caseless';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}
+// ==ORGANIZED==
+
+import { Caseless } from 'caseless';
+import foo from 'foo';
+
+declare module 'foo' {}
+declare module 'caseless' {
+    interface Caseless {
+        test(name: KeyType): boolean;
+    }
+}
