Updating move to file (microsoft#58257)

src/services/codefixes/importFixes.ts

@@ -404,6 +404,15 @@ function createImportAdderWorker(sourceFile: SourceFile | FutureSourceFile, prog
                         entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly));
                         break;
                     case ImportKind.CommonJS:
+                        if (compilerOptions.verbatimModuleSyntax) {
+                            const prevValue = (entry.namedImports ||= new Map()).get(symbolName);
+                            entry.namedImports.set(symbolName, reduceAddAsTypeOnlyValues(prevValue, addAsTypeOnly));
+                        }
+                        else {
+                            Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName");
+                            entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly };
+                        }
+                        break;
                     case ImportKind.Namespace:
                         Debug.assert(entry.namespaceLikeImport === undefined || entry.namespaceLikeImport.name === symbolName, "Namespacelike import shoudl be missing or match symbolName");
                         entry.namespaceLikeImport = { importKind, name: symbolName, addAsTypeOnly };

src/services/refactors/helpers.ts

@@ -1,3 +1,15 @@
+import {
+    codefix,
+    Debug,
+    findAncestor,
+    isAnyImportOrRequireStatement,
+    Program,
+    skipAlias,
+    SourceFile,
+    Symbol,
+    TypeChecker,
+} from "../_namespaces/ts";
+import { addImportsForMovedSymbols } from "./moveToFile";
 /**
  * Returned by refactor functions when some error message needs to be surfaced to users.
  *
@@ -26,3 +38,30 @@ export function refactorKindBeginsWith(known: string, requested: string | undefi
     if (!requested) return true;
     return known.substr(0, requested.length) === requested;
 }
+
+/** @internal */
+export function addTargetFileImports(
+    oldFile: SourceFile,
+    importsToCopy: Map<Symbol, [boolean, codefix.ImportOrRequireAliasDeclaration | undefined]>,
+    targetFileImportsFromOldFile: Map<Symbol, boolean>,
+    checker: TypeChecker,
+    program: Program,
+    importAdder: codefix.ImportAdder,
+) {
+    /**
+     * Recomputing the imports is preferred with importAdder because it manages multiple import additions for a file and writes then to a ChangeTracker,
+     * but sometimes it fails because of unresolved imports from files, or when a source file is not available for the target file (in this case when creating a new file).
+     * So in that case, fall back to copying the import verbatim.
+     */
+    importsToCopy.forEach(([isValidTypeOnlyUseSite, declaration], symbol) => {
+        const targetSymbol = skipAlias(symbol, checker);
+        if (checker.isUnknownSymbol(targetSymbol)) {
+            importAdder.addVerbatimImport(Debug.checkDefined(declaration ?? findAncestor(symbol.declarations?.[0], isAnyImportOrRequireStatement)));
+        }
+        else {
+            importAdder.addImportFromExportedSymbol(targetSymbol, isValidTypeOnlyUseSite, declaration);
+        }
+    });
+
+    addImportsForMovedSymbols(targetFileImportsFromOldFile, oldFile.fileName, importAdder, program);
+}

src/services/refactors/moveToNewFile.ts

@@ -1,56 +1,31 @@
 import {
-    append,
     ApplicableRefactorInfo,
+    codefix,
+    createFutureSourceFile,
     Debug,
     Diagnostics,
     emptyArray,
-    fileShouldUseJavaScriptRequire,
-    getBaseFileName,
     getLineAndCharacterOfPosition,
     getLocaleSpecificMessage,
-    getQuotePreference,
-    hasSyntacticModifier,
     hostGetCanonicalFileName,
-    Identifier,
-    insertImports,
-    isPrologueDirective,
     LanguageServiceHost,
     last,
-    ModifierFlags,
-    nodeSeenTracker,
+    ModuleKind,
     Program,
-    QuotePreference,
+    RefactorContext,
     RefactorEditInfo,
     SourceFile,
-    Symbol,
-    SyntaxKind,
-    takeWhile,
     textChanges,
-    TypeChecker,
     UserPreferences,
 } from "../_namespaces/ts";
 import {
-    addExports,
-    addExportToChanges,
     addNewFileToTsconfig,
     createNewFileName,
-    createOldFileImportsFromTargetFile,
-    deleteMovedStatements,
-    deleteUnusedOldImports,
-    filterImport,
-    forEachImportInStatement,
+    getNewStatementsAndRemoveFromOldFile,
     getStatementsToMove,
-    getTopLevelDeclarationStatement,
     getUsageInfo,
-    isTopLevelDeclaration,
-    makeImportOrRequire,
-    moduleSpecifierFromImport,
-    nameOfTopLevelDeclaration,
     registerRefactor,
-    SupportedImportStatement,
     ToMove,
-    updateImportsInOtherFiles,
-    UsageInfo,
 } from "../_namespaces/ts.refactor";
 
 const refactorName = "Move to a new file";
@@ -81,112 +56,19 @@ registerRefactor(refactorName, {
     getEditsForAction: function getRefactorEditsToMoveToNewFile(context, actionName): RefactorEditInfo {
         Debug.assert(actionName === refactorName, "Wrong refactor invoked");
         const statements = Debug.checkDefined(getStatementsToMove(context));
-        const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences));
+        const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context, context.preferences));
         return { edits, renameFilename: undefined, renameLocation: undefined };
     },
 });
 
-function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void {
+function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, context: RefactorContext, preferences: UserPreferences): void {
     const checker = program.getTypeChecker();
     const usage = getUsageInfo(oldFile, toMove.all, checker);
     const newFilename = createNewFileName(oldFile, program, host, toMove);
+    const newSourceFile = createFutureSourceFile(newFilename, oldFile.externalModuleIndicator ? ModuleKind.ESNext : oldFile.commonJsModuleIndicator ? ModuleKind.CommonJS : undefined, program, host);
 
-    // If previous file was global, this is easy.
-    changes.createNewFile(oldFile, newFilename, getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFilename, preferences));
-
+    const importAdderForOldFile = codefix.createImportAdder(oldFile, context.program, context.preferences, context.host);
+    const importAdderForNewFile = codefix.createImportAdder(newSourceFile, context.program, context.preferences, context.host);
+    getNewStatementsAndRemoveFromOldFile(oldFile, newSourceFile, usage, changes, toMove, program, host, preferences, importAdderForNewFile, importAdderForOldFile);
     addNewFileToTsconfig(program, changes, oldFile.fileName, newFilename, hostGetCanonicalFileName(host));
 }
-
-function getNewStatementsAndRemoveFromOldFile(
-    oldFile: SourceFile,
-    usage: UsageInfo,
-    changes: textChanges.ChangeTracker,
-    toMove: ToMove,
-    program: Program,
-    host: LanguageServiceHost,
-    newFilename: string,
-    preferences: UserPreferences,
-) {
-    const checker = program.getTypeChecker();
-    const prologueDirectives = takeWhile(oldFile.statements, isPrologueDirective);
-    if (oldFile.externalModuleIndicator === undefined && oldFile.commonJsModuleIndicator === undefined && usage.oldImportsNeededByTargetFile.size === 0) {
-        deleteMovedStatements(oldFile, toMove.ranges, changes);
-        return [...prologueDirectives, ...toMove.all];
-    }
-
-    const useEsModuleSyntax = !fileShouldUseJavaScriptRequire(newFilename, program, host, !!oldFile.commonJsModuleIndicator);
-    const quotePreference = getQuotePreference(oldFile, preferences);
-    const importsFromNewFile = createOldFileImportsFromTargetFile(oldFile, usage.oldFileImportsFromTargetFile, newFilename, program, host, useEsModuleSyntax, quotePreference);
-    if (importsFromNewFile) {
-        insertImports(changes, oldFile, importsFromNewFile, /*blankLineBetween*/ true, preferences);
-    }
-
-    deleteUnusedOldImports(oldFile, toMove.all, changes, usage.unusedImportsFromOldFile, checker);
-    deleteMovedStatements(oldFile, toMove.ranges, changes);
-    updateImportsInOtherFiles(changes, program, host, oldFile, usage.movedSymbols, newFilename, quotePreference);
-
-    const imports = getNewFileImportsAndAddExportInOldFile(oldFile, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, changes, checker, program, host, useEsModuleSyntax, quotePreference);
-    const body = addExports(oldFile, toMove.all, usage.oldFileImportsFromTargetFile, useEsModuleSyntax);
-    if (imports.length && body.length) {
-        return [
-            ...prologueDirectives,
-            ...imports,
-            SyntaxKind.NewLineTrivia as const,
-            ...body,
-        ];
-    }
-
-    return [
-        ...prologueDirectives,
-        ...imports,
-        ...body,
-    ];
-}
-
-function getNewFileImportsAndAddExportInOldFile(
-    oldFile: SourceFile,
-    importsToCopy: Map<Symbol, boolean>,
-    newFileImportsFromOldFile: Set<Symbol>,
-    changes: textChanges.ChangeTracker,
-    checker: TypeChecker,
-    program: Program,
-    host: LanguageServiceHost,
-    useEsModuleSyntax: boolean,
-    quotePreference: QuotePreference,
-): readonly SupportedImportStatement[] {
-    const copiedOldImports: SupportedImportStatement[] = [];
-    for (const oldStatement of oldFile.statements) {
-        forEachImportInStatement(oldStatement, i => {
-            append(copiedOldImports, filterImport(i, moduleSpecifierFromImport(i), name => importsToCopy.has(checker.getSymbolAtLocation(name)!)));
-        });
-    }
-
-    // Also, import things used from the old file, and insert 'export' modifiers as necessary in the old file.
-    let oldFileDefault: Identifier | undefined;
-    const oldFileNamedImports: string[] = [];
-    const markSeenTop = nodeSeenTracker(); // Needed because multiple declarations may appear in `const x = 0, y = 1;`.
-    newFileImportsFromOldFile.forEach(symbol => {
-        if (!symbol.declarations) {
-            return;
-        }
-        for (const decl of symbol.declarations) {
-            if (!isTopLevelDeclaration(decl)) continue;
-            const name = nameOfTopLevelDeclaration(decl);
-            if (!name) continue;
-
-            const top = getTopLevelDeclarationStatement(decl);
-            if (markSeenTop(top)) {
-                addExportToChanges(oldFile, top, name, changes, useEsModuleSyntax);
-            }
-            if (hasSyntacticModifier(decl, ModifierFlags.Default)) {
-                oldFileDefault = name;
-            }
-            else {
-                oldFileNamedImports.push(name.text);
-            }
-        }
-    });
-
-    append(copiedOldImports, makeImportOrRequire(oldFile, oldFileDefault, oldFileNamedImports, getBaseFileName(oldFile.fileName), program, host, useEsModuleSyntax, quotePreference));
-    return copiedOldImports;
-}

src/services/utilities.ts

@@ -2507,11 +2507,6 @@ export function moduleResolutionUsesNodeModules(moduleResolution: ModuleResoluti
         || moduleResolution === ModuleResolutionKind.Bundler;
 }
 
-/** @internal */
-export function makeImportIfNecessary(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string, quotePreference: QuotePreference): ImportDeclaration | undefined {
-    return defaultImport || namedImports && namedImports.length ? makeImport(defaultImport, namedImports, moduleSpecifier, quotePreference) : undefined;
-}
-
 /** @internal */
 export function makeImport(defaultImport: Identifier | undefined, namedImports: readonly ImportSpecifier[] | undefined, moduleSpecifier: string | Expression, quotePreference: QuotePreference, isTypeOnly?: boolean): ImportDeclaration {
     return factory.createImportDeclaration(

tests/baselines/reference/tsserver/fourslashServer/moveToFile_emptyTargetFile.js

@@ -241,13 +241,13 @@ Info seq  [hh:mm:ss:mss] response:
           ]
         },
         {
-          "name": "Move to a new file",
-          "description": "Move to a new file",
+          "name": "Move to file",
+          "description": "Move to file",
           "actions": [
             {
-              "name": "Move to a new file",
-              "description": "Move to a new file",
-              "kind": "refactor.move.newFile",
+              "name": "Move to file",
+              "description": "Move to file",
+              "kind": "refactor.move.file",
               "range": {
                 "start": {
                   "line": 1,
@@ -262,13 +262,13 @@ Info seq  [hh:mm:ss:mss] response:
           ]
         },
         {
-          "name": "Move to file",
-          "description": "Move to file",
+          "name": "Move to a new file",
+          "description": "Move to a new file",
           "actions": [
             {
-              "name": "Move to file",
-              "description": "Move to file",
-              "kind": "refactor.move.file",
+              "name": "Move to a new file",
+              "description": "Move to a new file",
+              "kind": "refactor.move.newFile",
               "range": {
                 "start": {
                   "line": 1,

tests/baselines/reference/tsserver/getApplicableRefactors/returns-the-affected-range-of-text-for-'move-to-file'-and-'move-to-new-file'-refactors.js

@@ -107,13 +107,13 @@ Info seq  [hh:mm:ss:mss] response:
     {
       "response": [
         {
-          "name": "Move to a new file",
-          "description": "Move to a new file",
+          "name": "Move to file",
+          "description": "Move to file",
           "actions": [
             {
-              "name": "Move to a new file",
-              "description": "Move to a new file",
-              "kind": "refactor.move.newFile",
+              "name": "Move to file",
+              "description": "Move to file",
+              "kind": "refactor.move.file",
               "range": {
                 "start": {
                   "line": 1,
@@ -128,13 +128,13 @@ Info seq  [hh:mm:ss:mss] response:
           ]
         },
         {
-          "name": "Move to file",
-          "description": "Move to file",
+          "name": "Move to a new file",
+          "description": "Move to a new file",
           "actions": [
             {
-              "name": "Move to file",
-              "description": "Move to file",
-              "kind": "refactor.move.file",
+              "name": "Move to a new file",
+              "description": "Move to a new file",
+              "kind": "refactor.move.newFile",
               "range": {
                 "start": {
                   "line": 1,

tests/cases/fourslash/moveToFile_blankExistingFile.ts

@@ -20,8 +20,6 @@ verify.moveToFile({
         "/bar.ts":
 `//
 import { p } from './a';
-
-
 import { b } from './other';
 
 

tests/cases/fourslash/moveToFile_consistentQuoteStyle.ts

@@ -21,7 +21,6 @@ verify.moveToFile({
 
         "/bar.ts":
 `import { a } from './a';
-
 import { b } from './other';
 
 export const tt = 2;

tests/cases/fourslash/moveToFile_differentDirectories2.ts

@@ -24,7 +24,6 @@ y;`,
 
         "/src/dir2/bar.ts":
 `import { a } from '../dir1/a';
-
 import { b } from '../dir1/other';
 
 

tests/cases/fourslash/moveToFile_expandSelectionRange4.ts

@@ -28,7 +28,7 @@ export const x: T={
 `,
 
         "/bar.ts":
-`import { x } from "./a";
+`import { x } from './a';
 import { b } from './other';
 const t = b;
 const b = x.a;

tests/cases/fourslash/moveToFile_namedImports.ts

@@ -12,10 +12,13 @@
 ////
 ////export const p = 0;
 ////export const b = 1;
-////[|const y = p + b;|]
+////[|export const y = p + b;
+////export const z = 1;|]
 
 // @Filename: /other.ts
+////import { z, y } from './a';
 ////export const t = 2;
+////const u = z + t + y;
 
 
 verify.moveToFile({
@@ -34,8 +37,14 @@ export const b = 1;
 `import { p, b } from './a';
 
 const q = 0;
-const y = p + b;
+export const y = p + b;
+export const z = 1;
 `,
+
+"/other.ts":
+`import { z, y } from './bar';
+export const t = 2;
+const u = z + t + y;`
     },
     interactiveRefactorArguments: { targetFile: "/bar.ts" },