getEditsForFileRename: Avoid changing import specifier ending

Author: Andy Hanson

src/compiler/moduleSpecifiers.ts

@@ -5,6 +5,50 @@ namespace ts.moduleSpecifiers {
         readonly importModuleSpecifierPreference?: "relative" | "non-relative";
     }
 
+    const enum RelativePreference { Relative, NonRelative, Auto }
+    const enum Ending {
+        Minimal, // Import "foo.ts" as "foo", "foo/index.ts" as "foo"
+        Index, // Import "foo.ts" as "foo", "foo/index.ts" as "foo/index"
+        JsExtension, // Import "foo.ts" as "foo.js", "foo/index.ts" as "foo/index.js"
+    }
+
+    // Processed preferences
+    interface Preferences {
+        readonly relativePreference: RelativePreference;
+        readonly ending: Ending;
+    }
+
+    function getPreferences({ importModuleSpecifierPreference }: ModuleSpecifierPreferences, compilerOptions: CompilerOptions, importingSourceFile: SourceFile): Preferences {
+        return {
+            relativePreference: importModuleSpecifierPreference === "relative" ? RelativePreference.Relative : importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative : RelativePreference.Auto,
+            ending: usesJsExtensionOnImports(importingSourceFile) ? Ending.JsExtension
+                : getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs ? Ending.Index : Ending.Minimal,
+        };
+    }
+
+    function getPreferencesForUpdate(_: ModuleSpecifierPreferences, compilerOptions: CompilerOptions, oldImportSpecifier: string): Preferences {
+        return {
+            relativePreference: isExternalModuleNameRelative(oldImportSpecifier) ? RelativePreference.Relative : RelativePreference.NonRelative,
+            ending: fileExtensionIs(oldImportSpecifier, Extension.Js) ? Ending.JsExtension
+                : getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs || endsWith(oldImportSpecifier, "index") || endsWith(oldImportSpecifier, "index.js") ? Ending.Index : Ending.Minimal,
+        };
+    }
+
+    export function updateModuleSpecifier(
+        compilerOptions: CompilerOptions,
+        importingSourceFileName: Path,
+        toFileName: string,
+        host: ModuleSpecifierResolutionHost,
+        files: ReadonlyArray<SourceFile>,
+        preferences: ModuleSpecifierPreferences = {},
+        redirectTargetsMap: RedirectTargetsMap,
+        oldImportSpecifier: string,
+    ): string | undefined {
+        const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferencesForUpdate(preferences, compilerOptions, oldImportSpecifier));
+        if (res === oldImportSpecifier) return undefined;
+        return res;
+    }
+
     // Note: importingSourceFile is just for usesJsExtensionOnImports
     export function getModuleSpecifier(
         compilerOptions: CompilerOptions,
@@ -16,9 +60,21 @@ namespace ts.moduleSpecifiers {
         preferences: ModuleSpecifierPreferences = {},
         redirectTargetsMap: RedirectTargetsMap,
     ): string {
-        const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host);
+        return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferences(preferences, compilerOptions, importingSourceFile));
+    }
+
+    function getModuleSpecifierWorker(
+        compilerOptions: CompilerOptions,
+        importingSourceFileName: Path,
+        toFileName: string,
+        host: ModuleSpecifierResolutionHost,
+        files: ReadonlyArray<SourceFile>,
+        redirectTargetsMap: RedirectTargetsMap,
+        preferences: Preferences
+    ): string {
+        const info = getInfo(importingSourceFileName, host);
         const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap);
-        return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) ||
+        return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions, preferences.ending)) ||
             first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
     }
 
@@ -41,60 +97,47 @@ namespace ts.moduleSpecifiers {
         importingSourceFile: SourceFile,
         host: ModuleSpecifierResolutionHost,
         files: ReadonlyArray<SourceFile>,
-        preferences: ModuleSpecifierPreferences,
+        userPreferences: ModuleSpecifierPreferences,
         redirectTargetsMap: RedirectTargetsMap,
     ): ReadonlyArray<ReadonlyArray<string>> {
         const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
         if (ambient) return [[ambient]];
 
-        const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.path, host);
+        const info = getInfo(importingSourceFile.path, host);
         if (!files) {
             return Debug.fail("Files list must be present to resolve symlinks in specifier resolution");
         }
         const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration);
         const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap);
 
-        const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
+        const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
+        const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions, preferences.ending));
         return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
             getLocalModuleSpecifiers(moduleFileName, info, compilerOptions, preferences));
     }
 
     interface Info {
-        readonly moduleResolutionKind: ModuleResolutionKind;
-        readonly addJsExtension: boolean;
         readonly getCanonicalFileName: GetCanonicalFileName;
         readonly sourceDirectory: Path;
     }
     // importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
-    function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
-        const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
-        const addJsExtension = usesJsExtensionOnImports(importingSourceFile);
+    function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
         const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
         const sourceDirectory = getDirectoryPath(importingSourceFileName);
-        return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory };
+        return { getCanonicalFileName, sourceDirectory };
     }
 
-    function getGlobalModuleSpecifier(
-        moduleFileName: string,
-        { addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
-        host: ModuleSpecifierResolutionHost,
-        compilerOptions: CompilerOptions,
-    ) {
-        return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension)
+    function getGlobalModuleSpecifier(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, host: ModuleSpecifierResolutionHost, compilerOptions: CompilerOptions, ending: Ending) {
+        return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, ending)
             || tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory);
     }
 
-    function getLocalModuleSpecifiers(
-        moduleFileName: string,
-        { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
-        compilerOptions: CompilerOptions,
-        preferences: ModuleSpecifierPreferences,
-    ): ReadonlyArray<string> {
+    function getLocalModuleSpecifiers(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, compilerOptions: CompilerOptions, { ending, relativePreference }: Preferences): ReadonlyArray<string> {
         const { baseUrl, paths, rootDirs } = compilerOptions;
 
         const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) ||
-            removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addJsExtension);
-        if (!baseUrl || preferences.importModuleSpecifierPreference === "relative") {
+            removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending);
+        if (!baseUrl || relativePreference === RelativePreference.Relative) {
             return [relativePath];
         }
 
@@ -103,19 +146,19 @@ namespace ts.moduleSpecifiers {
             return [relativePath];
         }
 
-        const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addJsExtension);
+        const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, ending);
         if (paths) {
             const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths);
             if (fromPaths) {
                 return [fromPaths];
             }
         }
 
-        if (preferences.importModuleSpecifierPreference === "non-relative") {
+        if (relativePreference === RelativePreference.NonRelative) {
             return [importRelativeToBaseUrl];
         }
 
-        if (preferences.importModuleSpecifierPreference !== undefined) Debug.assertNever(preferences.importModuleSpecifierPreference);
+        if (relativePreference !== RelativePreference.Auto) Debug.assertNever(relativePreference);
 
         if (isPathRelativeToParent(relativeToBaseUrl)) {
             return [relativePath];
@@ -276,14 +319,14 @@ namespace ts.moduleSpecifiers {
         host: GetEffectiveTypeRootsHost,
         getCanonicalFileName: (file: string) => string,
         moduleFileName: string,
-        addJsExtension: boolean,
+        ending: Ending,
     ): string | undefined {
         const roots = getEffectiveTypeRoots(options, host);
         return firstDefined(roots, unNormalizedTypeRoot => {
             const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName);
             if (startsWith(moduleFileName, typeRoot)) {
                 // For a type definition, we can strip `/index` even with classic resolution.
-                return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addJsExtension);
+                return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ending);
             }
         });
     }
@@ -428,13 +471,18 @@ namespace ts.moduleSpecifiers {
         });
     }
 
-    function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addJsExtension: boolean): string {
+    function removeExtensionAndIndexPostFix(fileName: string, ending: Ending): string {
         const noExtension = removeFileExtension(fileName);
-        return addJsExtension
-            ? noExtension + ".js"
-            : moduleResolutionKind === ModuleResolutionKind.NodeJs
-                ? removeSuffix(noExtension, "/index")
-                : noExtension;
+        switch (ending) {
+            case Ending.Minimal:
+                return removeSuffix(noExtension, "/index");
+            case Ending.Index:
+                return noExtension;
+            case Ending.JsExtension:
+                return noExtension + ".js";
+            default:
+                return Debug.assertNever(ending);
+        }
     }
 
     function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: GetCanonicalFileName): string | undefined {

src/services/getEditsForFileRename.ts

@@ -156,7 +156,7 @@ namespace ts {
 
                     // Need an update if the imported file moved, or the importing file moved and was using a relative path.
                     return toImport !== undefined && (toImport.updated || (importingSourceFileMoved && pathIsRelative(importLiteral.text)))
-                        ? moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, allFiles, preferences, program.redirectTargetsMap)
+                        ? moduleSpecifiers.updateModuleSpecifier(program.getCompilerOptions(), newImportFromPath, toImport.newFileName, host, allFiles, preferences, program.redirectTargetsMap, importLiteral.text)
                         : undefined;
                 });
         }

tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts

@@ -0,0 +1,29 @@
+/// <reference path='fourslash.ts' />
+
+// @allowJs: true
+
+// @Filename: /index.js
+////export const x = 0;
+
+// @Filename: /a.js
+////import { x as x0 } from ".";
+////import { x as x1 } from "./index";
+////import { x as x2 } from "./index.js";
+
+verify.getEditsForFileRename({
+    oldPath: "/a.js",
+    newPath: "/b.js",
+    newFileContents: {}, // No change
+});
+
+verify.getEditsForFileRename({
+    oldPath: "/b.js",
+    newPath: "/src/b.js",
+    newFileContents: {
+        "/b.js":
+`import { x as x0 } from "..";
+import { x as x1 } from "../index";
+import { x as x2 } from "../index.js";`,
+    },
+});
+