moveToNewFile: Don't move imports (#24177)

Author: Andy

src/compiler/core.ts

@@ -696,6 +696,23 @@ namespace ts {
         return false;
     }
 
+    /** Calls the callback with (start, afterEnd) index pairs for each range where 'pred' is true. */
+    export function getRangesWhere<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: number, afterEnd: number) => void): void {
+        let start: number | undefined;
+        for (let i = 0; i < arr.length; i++) {
+            if (pred(arr[i])) {
+                start = start === undefined ? i : start;
+            }
+            else {
+                if (start !== undefined) {
+                    cb(start, i);
+                    start = undefined;
+                }
+            }
+        }
+        if (start !== undefined) cb(start, arr.length);
+    }
+
     export function concatenate<T>(array1: T[], array2: T[]): T[];
     export function concatenate<T>(array1: ReadonlyArray<T>, array2: ReadonlyArray<T>): ReadonlyArray<T>;
     export function concatenate<T>(array1: T[], array2: T[]): T[] {

src/services/codefixes/fixUnreachableCode.ts

@@ -71,19 +71,6 @@ namespace ts.codefix {
 
     // Calls 'cb' with the start and end of each range where 'pred' is true.
     function split<T>(arr: ReadonlyArray<T>, pred: (t: T) => boolean, cb: (start: T, end: T) => void): void {
-        let start: T | undefined;
-        for (let i = 0; i < arr.length; i++) {
-            const value = arr[i];
-            if (pred(value)) {
-                start = start || value;
-            }
-            else {
-                if (start) {
-                    cb(start, arr[i - 1]);
-                    start = undefined;
-                }
-            }
-        }
-        if (start) cb(start, arr[arr.length - 1]);
+        getRangesWhere(arr, pred, (start, afterEnd) => cb(arr[start], arr[afterEnd - 1]));
     }
 }

src/services/refactors/moveToNewFile.ts

@@ -3,7 +3,7 @@ namespace ts.refactor {
     const refactorName = "Move to a new file";
     registerRefactor(refactorName, {
         getAvailableActions(context): ApplicableRefactorInfo[] {
-            if (!context.preferences.allowTextChangesInNewFiles || getStatementsToMove(context) === undefined) return undefined;
+            if (!context.preferences.allowTextChangesInNewFiles || getFirstAndLastStatementToMove(context) === undefined) return undefined;
             const description = getLocaleSpecificMessage(Diagnostics.Move_to_a_new_file);
             return [{ name: refactorName, description, actions: [{ name: refactorName, description }] }];
         },
@@ -15,7 +15,7 @@ namespace ts.refactor {
         }
     });
 
-    function getStatementsToMove(context: RefactorContext): ReadonlyArray<Statement> | undefined {
+    function getFirstAndLastStatementToMove(context: RefactorContext): { readonly first: number, readonly afterLast: number } | undefined {
         const { file } = context;
         const range = createTextRangeFromSpan(getRefactorContextSpan(context));
         const { statements } = file;
@@ -28,12 +28,12 @@ namespace ts.refactor {
         // Can't be partially into the next node
         if (afterEndNodeIndex !== -1 && (afterEndNodeIndex === 0 || statements[afterEndNodeIndex].getStart(file) < range.end)) return undefined;
 
-        return statements.slice(startNodeIndex, afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex);
+        return { first: startNodeIndex, afterLast: afterEndNodeIndex === -1 ? statements.length : afterEndNodeIndex };
     }
 
-    function doChange(oldFile: SourceFile, program: Program, toMove: ReadonlyArray<Statement>, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void {
+    function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost): void {
         const checker = program.getTypeChecker();
-        const usage = getUsageInfo(oldFile, toMove, checker);
+        const usage = getUsageInfo(oldFile, toMove.all, checker);
 
         const currentDirectory = getDirectoryPath(oldFile.fileName);
         const extension = extensionFromPath(oldFile.fileName);
@@ -46,6 +46,42 @@ namespace ts.refactor {
         addNewFileToTsconfig(program, changes, oldFile.fileName, newFileNameWithExtension, hostGetCanonicalFileName(host));
     }
 
+    interface StatementRange {
+        readonly first: Statement;
+        readonly last: Statement;
+    }
+    interface ToMove {
+        readonly all: ReadonlyArray<Statement>;
+        readonly ranges: ReadonlyArray<StatementRange>;
+    }
+
+    // Filters imports out of the range of statements to move. Imports will be copied to the new file anyway, and may still be needed in the old file.
+    function getStatementsToMove(context: RefactorContext): ToMove | undefined {
+        const { statements } = context.file;
+        const { first, afterLast } = getFirstAndLastStatementToMove(context)!;
+        const all: Statement[] = [];
+        const ranges: StatementRange[] = [];
+        const rangeToMove = statements.slice(first, afterLast);
+        getRangesWhere(rangeToMove, s => !isPureImport(s), (start, afterEnd) => {
+            for (let i = start; i < afterEnd; i++) all.push(rangeToMove[i]);
+            ranges.push({ first: rangeToMove[start], last: rangeToMove[afterEnd - 1] });
+        });
+        return { all, ranges };
+    }
+
+    function isPureImport(node: Node): boolean {
+        switch (node.kind) {
+            case SyntaxKind.ImportDeclaration:
+                return true;
+            case SyntaxKind.ImportEqualsDeclaration:
+                return !hasModifier(node, ModifierFlags.Export);
+            case SyntaxKind.VariableStatement:
+                return (node as VariableStatement).declarationList.declarations.every(d => isRequireCall(d.initializer, /*checkArgumentIsStringLiteralLike*/ true));
+            default:
+                return false;
+        }
+    }
+
     function addNewFileToTsconfig(program: Program, changes: textChanges.ChangeTracker, oldFileName: string, newFileNameWithExtension: string, getCanonicalFileName: GetCanonicalFileName): void {
         const cfg = program.getCompilerOptions().configFile;
         if (!cfg) return;
@@ -62,13 +98,13 @@ namespace ts.refactor {
     }
 
     function getNewStatements(
-        oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ReadonlyArray<Statement>, program: Program, newModuleName: string,
+        oldFile: SourceFile, usage: UsageInfo, changes: textChanges.ChangeTracker, toMove: ToMove, program: Program, newModuleName: string,
     ): ReadonlyArray<Statement> {
         const checker = program.getTypeChecker();
 
         if (!oldFile.externalModuleIndicator && !oldFile.commonJsModuleIndicator) {
-            changes.deleteNodeRange(oldFile, first(toMove), last(toMove));
-            return toMove;
+            deleteMovedStatements(oldFile, toMove.ranges, changes);
+            return toMove.all;
         }
 
         const useEs6ModuleSyntax = !!oldFile.externalModuleIndicator;
@@ -77,17 +113,23 @@ namespace ts.refactor {
             changes.insertNodeBefore(oldFile, oldFile.statements[0], importsFromNewFile, /*blankLineBetween*/ true);
         }
 
-        deleteUnusedOldImports(oldFile, toMove, changes, usage.unusedImportsFromOldFile, checker);
-        changes.deleteNodeRange(oldFile, first(toMove), last(toMove));
+        deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);
+        deleteMovedStatements(oldFile, toMove.ranges, changes);
 
         updateImportsInOtherFiles(changes, program, oldFile, usage.movedSymbols, newModuleName);
 
         return [
             ...getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByNewFile, usage.newFileImportsFromOldFile, changes, checker, useEs6ModuleSyntax),
-            ...addExports(oldFile, toMove, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax),
+            ...addExports(oldFile, toMove.all, usage.oldFileImportsFromNewFile, useEs6ModuleSyntax),
         ];
     }
 
+    function deleteMovedStatements(sourceFile: SourceFile, moved: ReadonlyArray<StatementRange>, changes: textChanges.ChangeTracker) {
+        for (const { first, last } of moved) {
+            changes.deleteNodeRange(sourceFile, first, last);
+        }
+    }
+
     function deleteUnusedOldImports(oldFile: SourceFile, toMove: ReadonlyArray<Statement>, changes: textChanges.ChangeTracker, toDelete: ReadonlySymbolSet, checker: TypeChecker) {
         for (const statement of oldFile.statements) {
             if (contains(toMove, statement)) continue;

tests/cases/fourslash/moveToNewFile_exportImport.ts

@@ -9,14 +9,12 @@
 verify.moveToNewFile({
     newFileContents: {
         "/a.ts":
-`import { M } from "./M";
-
-export namespace N { export const x = 0; }
+`export namespace N { export const x = 0; }
+import M = N;
 M;`,
 
-        "/M.ts":
+        "/O.ts":
 `import { N } from "./a";
-export import M = N;
 export import O = N;`,
     },
 });

tests/cases/fourslash/moveToNewFile_moveImport.ts

@@ -5,14 +5,13 @@
 ////a;|]
 ////b;
 
-//verify.noMoveToNewFile();
 verify.moveToNewFile({
     newFileContents: {
         "/a.ts":
-`b;`,
+`import { b } from "m";
+b;`,
         "/newFile.ts":
 `import { a } from "m";
-import { a, b } from "m";
 a;`,
     }
 });