Add 'move to new file' refactor

src/compiler/diagnosticMessages.json

@@ -4175,7 +4175,7 @@
         "category": "Message",
         "code": 95046
     },
-    "Move to new file": {
+    "Move to a new file": {
         "category": "Message",
         "code": 95047
     }

src/compiler/factory.ts

@@ -1504,6 +1504,13 @@ namespace ts {
         return block;
     }
 
+    /* @internal */
+    export function createExpressionStatement(expression: Expression): ExpressionStatement {
+        const node = <ExpressionStatement>createSynthesizedNode(SyntaxKind.ExpressionStatement);
+        node.expression = expression;
+        return node;
+    }
+
     export function updateBlock(node: Block, statements: ReadonlyArray<Statement>) {
         return node.statements !== statements
             ? updateNode(createBlock(statements, node.multiLine), node)
@@ -1530,9 +1537,8 @@ namespace ts {
     }
 
     export function createStatement(expression: Expression) {
-        const node = <ExpressionStatement>createSynthesizedNode(SyntaxKind.ExpressionStatement);
-        node.expression = parenthesizeExpressionForExpressionStatement(expression);
-        return node;
+        return createExpressionStatement(parenthesizeExpressionForExpressionStatement(expression));
+
     }
 
     export function updateStatement(node: ExpressionStatement, expression: Expression) {

src/harness/fourslash.ts

@@ -3059,10 +3059,10 @@ Actual: ${stringify(fullActual)}`);
         public moveToNewFile(options: FourSlashInterface.MoveToNewFileOptions): void {
             assert(this.getRanges().length === 1);
             const range = this.getRanges()[0];
-            const refactor = ts.find(this.getApplicableRefactors(range), r => r.name === "Move to new file");
+            const refactor = ts.find(this.getApplicableRefactors(range), r => r.name === "Move to a new file");
             assert(refactor.actions.length === 1);
             const action = ts.first(refactor.actions);
-            assert(action.name === "Move to new file" && action.description === "Move to new file");
+            assert(action.name === "Move to a new file" && action.description === "Move to a new file");
 
             const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, this.formatCodeSettings, range, refactor.name, action.name, ts.defaultPreferences);
             for (const edit of editInfo.edits) {

src/services/findAllReferences.ts

@@ -612,27 +612,19 @@ namespace ts.FindAllReferences.Core {
             checker.getPropertySymbolOfDestructuringAssignment(<Identifier>location);
     }
 
-    function getObjectBindingElementWithoutPropertyName(symbol: Symbol): BindingElement | undefined {
+    function getObjectBindingElementWithoutPropertyName(symbol: Symbol): BindingElement & { name: Identifier } | undefined {
         const bindingElement = getDeclarationOfKind<BindingElement>(symbol, SyntaxKind.BindingElement);
         if (bindingElement &&
             bindingElement.parent.kind === SyntaxKind.ObjectBindingPattern &&
+            isIdentifier(bindingElement.name) &&
             !bindingElement.propertyName) {
-            return bindingElement;
+            return bindingElement as BindingElement & { name: Identifier };
         }
     }
 
     function getPropertySymbolOfObjectBindingPatternWithoutPropertyName(symbol: Symbol, checker: TypeChecker): Symbol | undefined {
         const bindingElement = getObjectBindingElementWithoutPropertyName(symbol);
-        if (!bindingElement) return undefined;
-
-        const typeOfPattern = checker.getTypeAtLocation(bindingElement.parent);
-        const propSymbol = typeOfPattern && checker.getPropertyOfType(typeOfPattern, (<Identifier>bindingElement.name).text);
-        if (propSymbol && propSymbol.flags & SymbolFlags.Accessor) {
-            // See GH#16922
-            Debug.assert(!!(propSymbol.flags & SymbolFlags.Transient));
-            return (propSymbol as TransientSymbol).target;
-        }
-        return propSymbol;
+        return bindingElement && getPropertySymbolFromBindingElement(checker, bindingElement);
     }
 
     /**

src/services/textChanges.ts

@@ -458,16 +458,25 @@ namespace ts.textChanges {
             this.insertNodeAt(sourceFile, cls.members.pos, newElement, { indentation, prefix, suffix });
         }
 
-        public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node): this {
+        public insertNodeAfter(sourceFile: SourceFile, after: Node, newNode: Node): void {
+            const endPosition = this.insertNodeAfterWorker(sourceFile, after, newNode);
+            this.insertNodeAt(sourceFile, endPosition, newNode, this.getInsertNodeAfterOptions(after));
+        }
+
+        public insertNodesAfter(sourceFile: SourceFile, after: Node, newNodes: ReadonlyArray<Node>): void {
+            const endPosition = this.insertNodeAfterWorker(sourceFile, after, first(newNodes));
+            this.insertNodesAt(sourceFile, endPosition, newNodes, this.getInsertNodeAfterOptions(after));
+        }
+
+        private insertNodeAfterWorker(sourceFile: SourceFile, after: Node, newNode: Node): number {
             if (needSemicolonBetween(after, newNode)) {
                 // check if previous statement ends with semicolon
                 // if not - insert semicolon to preserve the code from changing the meaning due to ASI
                 if (sourceFile.text.charCodeAt(after.end - 1) !== CharacterCodes.semicolon) {
                     this.replaceRange(sourceFile, createTextRange(after.end), createToken(SyntaxKind.SemicolonToken));
                 }
             }
-            const endPosition = getAdjustedEndPosition(sourceFile, after, {});
-            return this.replaceRange(sourceFile, createTextRange(endPosition), newNode, this.getInsertNodeAfterOptions(after));
+            return getAdjustedEndPosition(sourceFile, after, {});
         }
 
         private getInsertNodeAfterOptions(node: Node): InsertNodeOptions {

src/services/utilities.ts

@@ -1262,6 +1262,17 @@ namespace ts {
             return name && name.kind === SyntaxKind.Identifier ? name.escapedText : undefined;
         });
     }
+
+    export function getPropertySymbolFromBindingElement(checker: TypeChecker, bindingElement: BindingElement & { name: Identifier }) {
+        const typeOfPattern = checker.getTypeAtLocation(bindingElement.parent);
+        const propSymbol = typeOfPattern && checker.getPropertyOfType(typeOfPattern, bindingElement.name.text);
+        if (propSymbol && propSymbol.flags & SymbolFlags.Accessor) {
+            // See GH#16922
+            Debug.assert(!!(propSymbol.flags & SymbolFlags.Transient));
+            return (propSymbol as TransientSymbol).target;
+        }
+        return propSymbol;
+    }
 }
 
 // Display-part writer helpers

tests/cases/fourslash/moveToNewFile.ts

@@ -4,7 +4,7 @@
 ////import { a, b } from "./other";
 ////const p = 0;
 ////[|const y = p + b;|]
-////y;
+////a; y;
 
 verify.moveToNewFile({
     newFileContents: {
@@ -13,7 +13,7 @@ verify.moveToNewFile({
 
 import { a } from "./other";
 export const p = 0;
-y;`,
+a; y;`,
 
         "/y.ts":
 `import { b } from "./other";

tests/cases/fourslash/moveToNewFile_importEquals.ts

@@ -0,0 +1,18 @@
+/// <reference path='fourslash.ts' />
+
+// @Filename: /a.ts
+////import i = require("./i");
+////import j = require("./j");
+////[|const y = i;|]
+////j;
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/a.ts":
+`import j = require("./j");
+j;`,
+        "/y.ts":
+`import i = require("./i");
+const y = i;`,
+    },
+});

tests/cases/fourslash/moveToNewFile_js.ts

@@ -0,0 +1,39 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+
+// @Filename: /a.js
+////const { a, b } = require("./other");
+////const p = 0;
+////
+////[|const y = p + b;
+////const z = 0;
+////exports.z = 0;|]
+////a; y; z;
+
+// @Filename: /user.ts
+////const { x, y } = require("./a");
+
+// TODO: GH#23793 This will crash if the blank line between `const p` and `const y` is removed
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/a.js":
+// TODO: GH#22330
+`const { y, z } = require("./y");
+
+const { a, } = require("./other");
+const p = 0;
+exports.p = p;
+
+a; y; z;`,
+
+        "/y.js":
+`const { b } = require("./other");
+const { p } = require("./a");
+const y = p + b;
+exports.y = y;
+const z = 0;
+exports.z = 0;`,
+    },
+});

tests/cases/fourslash/moveToNewFile_newModuleNameUnique.ts

@@ -6,15 +6,15 @@
 // @Filename: /x.ts
 ////
 
-// @Filename: /x0.ts
+// @Filename: /x.1.ts
 ////
 
 verify.moveToNewFile({
     newFileContents: {
         "/a.ts":
 ``,
 
-        "/x00.ts":
+        "/x.2.ts":
 `export const x = 0;`,
     },
 });

tests/cases/fourslash/moveToNewFile_rangeInvalid.ts

@@ -9,5 +9,5 @@
 
 for (const range of test.ranges()) {
     goTo.selectRange(range);
-    verify.not.refactorAvailable("Move to new file")
+    verify.not.refactorAvailable("Move to a new file")
 }

tests/cases/fourslash/moveToNewFile_updateUses_js.ts

@@ -0,0 +1,30 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+
+// @Filename: /a.js
+////exports.x = 0;
+////[|exports.y = 0;|]
+
+// @Filename: /user.js
+////const { x, y } = require("./a");
+////
+
+// TODO: GH#23728 Shouldn't need `////` above
+
+verify.moveToNewFile({
+    newFileContents: {
+        "/a.js":
+`exports.x = 0;
+`,
+
+        "/y.js":
+`exports.y = 0;`,
+
+        "/user.js":
+// TODO: GH#22330
+`const { x, } = require("./a");
+const { y } = require("./y");
+`,
+    },
+});