From 7102de77d3b62cb7bcf344bc7d5ae3dd4c25d603 Mon Sep 17 00:00:00 2001
From: Andrew Casey <andrew.casey@microsoft.com>
Date: Wed, 16 Jan 2019 15:54:08 -0800
Subject: [PATCH 1/3] Consider JSX namespace imports when moving statements
 between files

Each of the old and new files should end up with a JSX namespace import
iff it contains JSX.

Fixes #27939
---
 src/services/refactors/moveToNewFile.ts | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts
index 87973226b3c..6d2a4872842 100644
--- a/src/services/refactors/moveToNewFile.ts
+++ b/src/services/refactors/moveToNewFile.ts
@@ -460,6 +460,12 @@ namespace ts.refactor {
         const oldImportsNeededByNewFile = new SymbolSet();
         const newFileImportsFromOldFile = new SymbolSet();
 
+        const containsJsx = find(toMove, statement => !!(statement.transformFlags & TransformFlags.ContainsJsx));
+        const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx);
+        if (jsxNamespaceSymbol) { // Might not exist (e.g. in non-compiling code)
+            oldImportsNeededByNewFile.add(jsxNamespaceSymbol);
+        }
+
         for (const statement of toMove) {
             forEachTopLevelDeclaration(statement, decl => {
                 movedSymbols.add(Debug.assertDefined(isExpressionStatement(decl) ? checker.getSymbolAtLocation(decl.expression.left) : decl.symbol));
@@ -485,6 +491,11 @@ namespace ts.refactor {
         for (const statement of oldFile.statements) {
             if (contains(toMove, statement)) continue;
 
+            // jsxNamespaceSymbol will only be set iff it is in oldImportsNeededByNewFile.
+            if (jsxNamespaceSymbol && !!(statement.transformFlags & TransformFlags.ContainsJsx)) {
+                unusedImportsFromOldFile.delete(jsxNamespaceSymbol);
+            }
+
             forEachReference(statement, checker, symbol => {
                 if (movedSymbols.has(symbol)) oldFileImportsFromNewFile.add(symbol);
                 unusedImportsFromOldFile.delete(symbol);
@@ -492,6 +503,18 @@ namespace ts.refactor {
         }
 
         return { movedSymbols, newFileImportsFromOldFile, oldFileImportsFromNewFile, oldImportsNeededByNewFile, unusedImportsFromOldFile };
+
+        function getJsxNamespaceSymbol(containsJsx: Node | undefined) {
+            if (containsJsx === undefined) {
+                return undefined;
+            }
+
+            const jsxNamespace = checker.getJsxNamespace(containsJsx);
+            const jsxNamespaceSymbol = checker.resolveName(jsxNamespace, containsJsx, SymbolFlags.Namespace, /*excludeGlobals*/ true);
+            return !!jsxNamespaceSymbol && some(jsxNamespaceSymbol.declarations, isInImport)
+                ? jsxNamespaceSymbol
+                : undefined;
+        }
     }
 
     // Below should all be utilities
@@ -512,7 +535,7 @@ namespace ts.refactor {
     }
     function isVariableDeclarationInImport(decl: VariableDeclaration) {
         return isSourceFile(decl.parent.parent.parent) &&
-            decl.initializer && isRequireCall(decl.initializer, /*checkArgumentIsStringLiteralLike*/ true);
+            !!decl.initializer && isRequireCall(decl.initializer, /*checkArgumentIsStringLiteralLike*/ true);
     }
 
     function filterImport(i: SupportedImport, moduleSpecifier: StringLiteralLike, keep: (name: Identifier) => boolean): SupportedImportStatement | undefined {

From 3e256e14dcb35665b538722197b0aa3ffddc8d79 Mon Sep 17 00:00:00 2001
From: Andrew Casey <andrew.casey@microsoft.com>
Date: Wed, 16 Jan 2019 19:18:25 -0800
Subject: [PATCH 2/3] Add fourslash tests

---
 src/services/refactors/moveToNewFile.ts       |  5 +++++
 .../fourslash/moveToNewFile_moveJsxImport1.ts | 21 ++++++++++++++++++
 .../fourslash/moveToNewFile_moveJsxImport2.ts | 22 +++++++++++++++++++
 .../fourslash/moveToNewFile_moveJsxImport3.ts | 21 ++++++++++++++++++
 4 files changed, 69 insertions(+)
 create mode 100644 tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts
 create mode 100644 tests/cases/fourslash/moveToNewFile_moveJsxImport2.ts
 create mode 100644 tests/cases/fourslash/moveToNewFile_moveJsxImport3.ts

diff --git a/src/services/refactors/moveToNewFile.ts b/src/services/refactors/moveToNewFile.ts
index 6d2a4872842..ad60a6da5fc 100644
--- a/src/services/refactors/moveToNewFile.ts
+++ b/src/services/refactors/moveToNewFile.ts
@@ -510,7 +510,12 @@ namespace ts.refactor {
             }
 
             const jsxNamespace = checker.getJsxNamespace(containsJsx);
+
+            // Strictly speaking, this could resolve to a symbol other than the JSX namespace.
+            // This will produce erroneous output (probably, an incorrectly copied import) but
+            // is expected to be very rare and easily reversible.
             const jsxNamespaceSymbol = checker.resolveName(jsxNamespace, containsJsx, SymbolFlags.Namespace, /*excludeGlobals*/ true);
+
             return !!jsxNamespaceSymbol && some(jsxNamespaceSymbol.declarations, isInImport)
                 ? jsxNamespaceSymbol
                 : undefined;
diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts
new file mode 100644
index 00000000000..c63efb52dac
--- /dev/null
+++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport1.ts
@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @noLib: true
+// @libFiles: react.d.ts,lib.d.ts
+
+// @Filename: file.tsx
+//// import React = require('react');
+//// [|<div/>;|]
+//// 1;
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/tests/cases/fourslash/file.tsx":
+`1;`,
+        "/tests/cases/fourslash/newFile.tsx":
+`import React = require('react');
+<div />;
+`,
+    }
+});
diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport2.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport2.ts
new file mode 100644
index 00000000000..a73fcb2209c
--- /dev/null
+++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport2.ts
@@ -0,0 +1,22 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @noLib: true
+// @libFiles: react.d.ts,lib.d.ts
+
+// @Filename: file.tsx
+//// import React = require('react');
+//// [|<div/>;|]
+//// <div/>;
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/tests/cases/fourslash/file.tsx":
+`import React = require('react');
+<div/>;`,
+        "/tests/cases/fourslash/newFile.tsx":
+`import React = require('react');
+<div />;
+`,
+    }
+});
diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport3.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport3.ts
new file mode 100644
index 00000000000..9adfa9fd14a
--- /dev/null
+++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport3.ts
@@ -0,0 +1,21 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @noLib: true
+// @libFiles: react.d.ts,lib.d.ts
+
+// @Filename: file.tsx
+//// import React = require('react');
+//// [|1;|]
+//// <div/>;
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/tests/cases/fourslash/file.tsx":
+`import React = require('react');
+<div/>;`,
+        "/tests/cases/fourslash/newFile.tsx":
+`1;
+`,
+    }
+});

From 4029e70c97070a5b8adfe3cc154d3fc35e9dda39 Mon Sep 17 00:00:00 2001
From: Andrew Casey <andrew.casey@microsoft.com>
Date: Wed, 16 Jan 2019 19:24:11 -0800
Subject: [PATCH 3/3] Illustrate a case that isn't handled correctly

---
 .../fourslash/moveToNewFile_moveJsxImport4.ts | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 tests/cases/fourslash/moveToNewFile_moveJsxImport4.ts

diff --git a/tests/cases/fourslash/moveToNewFile_moveJsxImport4.ts b/tests/cases/fourslash/moveToNewFile_moveJsxImport4.ts
new file mode 100644
index 00000000000..b5179f66a45
--- /dev/null
+++ b/tests/cases/fourslash/moveToNewFile_moveJsxImport4.ts
@@ -0,0 +1,29 @@
+/// <reference path='fourslash.ts' />
+
+// @jsx: preserve
+// @noLib: true
+// @libFiles: react.d.ts,lib.d.ts,leftpad.d.ts
+
+// @Filename: file.tsx
+//// import React = require('leftpad');
+//// [|function F() {
+////     const React = import("react");
+////     <div/>;
+//// }|]
+//// React;
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/tests/cases/fourslash/file.tsx":
+`import React = require('leftpad');
+React;`,
+        // NB: A perfect implementation would not copy over the import
+        "/tests/cases/fourslash/F.tsx":
+`import React = require('leftpad');
+function F() {
+    const React = import("react");
+    <div />;
+}
+`,
+    }
+});