From 60d0bb9b199e76974cdc65647663267e9fcf75b1 Mon Sep 17 00:00:00 2001
From: Andrew Branch <andrew@wheream.io>
Date: Wed, 22 May 2019 11:37:35 -0700
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20touch=20imports=20used=20for=20?=
 =?UTF-8?q?module=20augmentation=20in=20non-declaration=20files=20since=20?=
 =?UTF-8?q?it=20could=20change=20JS=20emit?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/services/organizeImports.ts               | 28 ++++++++++++-------
 .../unittests/services/organizeImports.ts     | 15 ++++++++++
 ...le_augmentation_in_non_declaration_file.ts | 22 +++++++++++++++
 3 files changed, 55 insertions(+), 10 deletions(-)
 create mode 100644 tests/baselines/reference/organizeImports/Unused_preserve_imports_for_module_augmentation_in_non_declaration_file.ts

diff --git a/src/services/organizeImports.ts b/src/services/organizeImports.ts
index b6be92fbf9c..81c9108749f 100644
--- a/src/services/organizeImports.ts
+++ b/src/services/organizeImports.ts
@@ -125,13 +125,22 @@ namespace ts.OrganizeImports {
             if (name || namedBindings) {
                 usedImports.push(updateImportDeclarationAndClause(importDecl, name, namedBindings));
             }
-            // If a module is imported to be augmented, keep the import declaration, but without an import clause
+            // If a module is imported to be augmented, it’s used
             else if (hasModuleDeclarationMatchingSpecifier(sourceFile, moduleSpecifier)) {
-                usedImports.push(createImportDeclaration(
-                    importDecl.decorators,
-                    importDecl.modifiers,
-                    /*importClause*/ undefined,
-                    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);
+                }
             }
         }
 
@@ -145,10 +154,9 @@ namespace ts.OrganizeImports {
 
     function hasModuleDeclarationMatchingSpecifier(sourceFile: SourceFile, moduleSpecifier: Expression) {
         const moduleSpecifierText = isStringLiteral(moduleSpecifier) && moduleSpecifier.text;
-        return isString(moduleSpecifierText) && some(sourceFile.statements, statement =>
-            isModuleDeclaration(statement)
-            && isStringLiteral(statement.name)
-            && statement.name.text === moduleSpecifierText);
+        return isString(moduleSpecifierText) && some(sourceFile.moduleAugmentations, moduleName =>
+            isStringLiteral(moduleName)
+            && moduleName.text === moduleSpecifierText);
     }
 
     function getExternalModuleName(specifier: Expression) {
diff --git a/src/testRunner/unittests/services/organizeImports.ts b/src/testRunner/unittests/services/organizeImports.ts
index 8003e1867cd..c119aadb34e 100644
--- a/src/testRunner/unittests/services/organizeImports.ts
+++ b/src/testRunner/unittests/services/organizeImports.ts
@@ -350,6 +350,21 @@ import { } from "lib";
 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 {
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
new file mode 100644
index 00000000000..6d28f89903a
--- /dev/null
+++ 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;
+    }
+}
\ No newline at end of file