getEditsForFileRename: Avoid changing import specifier ending

Author: Andy Hanson

src/compiler/moduleSpecifiers.ts

@@ -5,6 +5,47 @@ namespace ts.moduleSpecifiers {
         readonly importModuleSpecifierPreference?: "relative" | "non-relative";
     }
 
+    const enum RelativePreference { Relative, NonRelative, Auto }
+    // Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js"
+    const enum Ending { Minimal, Index, JsExtension }
+
+    // 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 +57,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 => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions)) ||
             first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
     }
 
@@ -41,60 +94,42 @@ 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 => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions));
         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)
-            || 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 +138,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];
@@ -271,37 +306,8 @@ namespace ts.moduleSpecifiers {
         return removeFileExtension(relativePath);
     }
 
-    function tryGetModuleNameFromTypeRoots(
-        options: CompilerOptions,
-        host: GetEffectiveTypeRootsHost,
-        getCanonicalFileName: (file: string) => string,
-        moduleFileName: string,
-        addJsExtension: boolean,
-    ): 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);
-            }
-        });
-    }
-
-    function tryGetModuleNameAsNodeModule(
-        options: CompilerOptions,
-        moduleFileName: string,
-        host: ModuleSpecifierResolutionHost,
-        getCanonicalFileName: (file: string) => string,
-        sourceDirectory: Path,
-    ): string | undefined {
-        if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) {
-            // nothing to do here
-            return undefined;
-        }
-
+    function tryGetModuleNameAsNodeModule(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, host: ModuleSpecifierResolutionHost, options: CompilerOptions): string | undefined {
         const parts: NodeModulePathParts = getNodeModulePathParts(moduleFileName)!;
-
         if (!parts) {
             return undefined;
         }
@@ -313,8 +319,12 @@ namespace ts.moduleSpecifiers {
         // Get a path that's relative to node_modules or the importing file's path
         // if node_modules folder is in this folder or any of its parent folders, no need to keep it.
         if (!startsWith(sourceDirectory, getCanonicalFileName(moduleSpecifier.substring(0, parts.topLevelNodeModulesIndex)))) return undefined;
+
         // If the module was found in @types, get the actual Node package name
-        return getPackageNameFromAtTypesDirectory(moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1));
+        const nodeModulesDirectoryName = moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1);
+        const packageName = getPackageNameFromAtTypesDirectory(nodeModulesDirectoryName);
+        // For classic resolution, only allow importing from node_modules/@types, not other node_modules
+        return getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs && packageName === nodeModulesDirectoryName ? undefined : packageName;
 
         function getDirectoryOrExtensionlessFileName(path: string): string {
             // If the file is the main module, it can be imported by the package name
@@ -428,13 +438,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";`,
+    },
+});
+

tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts

@@ -5,12 +5,22 @@
 // @Filename: /node_modules/@types/foo/index.d.ts
 ////export const xyz: number;
 
+// @Filename: /node_modules/bar/index.d.ts
+////export const qrs: number;
+
 // @Filename: /a.ts
-////[|xyz|]
+////xyz;
+////qrs;
 
 goTo.file("/a.ts");
-verify.importFixAtPosition([
+verify.codeFixAll({
+    fixId: "fixMissingImport",
+    fixAllDescription: "Add all missing imports",
+    newFileContent:
 `import { xyz } from "foo";
 
-xyz`
-]);
+import { qrs } from "./node_modules/bar/index";
+
+xyz;
+qrs;`,
+});