Skip to content

Commit d40df30

Browse files
author
Andy Hanson
committed
getEditsForFileRename: Avoid changing import specifier ending
1 parent 9df8831 commit d40df30

6 files changed

Lines changed: 127 additions & 108 deletions

‎src/compiler/moduleSpecifiers.ts‎

Lines changed: 83 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,47 @@ namespace ts.moduleSpecifiers {
55
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
66
}
77

8+
const enum RelativePreference { Relative, NonRelative, Auto }
9+
// Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js"
10+
const enum Ending { Minimal, Index, JsExtension }
11+
12+
// Processed preferences
13+
interface Preferences {
14+
readonly relativePreference: RelativePreference;
15+
readonly ending: Ending;
16+
}
17+
18+
function getPreferences({ importModuleSpecifierPreference }: ModuleSpecifierPreferences, compilerOptions: CompilerOptions, importingSourceFile: SourceFile): Preferences {
19+
return {
20+
relativePreference: importModuleSpecifierPreference === "relative" ? RelativePreference.Relative : importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative : RelativePreference.Auto,
21+
ending: usesJsExtensionOnImports(importingSourceFile) ? Ending.JsExtension
22+
: getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs ? Ending.Index : Ending.Minimal,
23+
};
24+
}
25+
26+
function getPreferencesForUpdate(_: ModuleSpecifierPreferences, compilerOptions: CompilerOptions, oldImportSpecifier: string): Preferences {
27+
return {
28+
relativePreference: isExternalModuleNameRelative(oldImportSpecifier) ? RelativePreference.Relative : RelativePreference.NonRelative,
29+
ending: fileExtensionIs(oldImportSpecifier, Extension.Js) ? Ending.JsExtension
30+
: getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs || endsWith(oldImportSpecifier, "index") || endsWith(oldImportSpecifier, "index.js") ? Ending.Index : Ending.Minimal,
31+
};
32+
}
33+
34+
export function updateModuleSpecifier(
35+
compilerOptions: CompilerOptions,
36+
importingSourceFileName: Path,
37+
toFileName: string,
38+
host: ModuleSpecifierResolutionHost,
39+
files: ReadonlyArray<SourceFile>,
40+
preferences: ModuleSpecifierPreferences = {},
41+
redirectTargetsMap: RedirectTargetsMap,
42+
oldImportSpecifier: string,
43+
): string | undefined {
44+
const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferencesForUpdate(preferences, compilerOptions, oldImportSpecifier));
45+
if (res === oldImportSpecifier) return undefined;
46+
return res;
47+
}
48+
849
// Note: importingSourceFile is just for usesJsExtensionOnImports
950
export function getModuleSpecifier(
1051
compilerOptions: CompilerOptions,
@@ -16,7 +57,19 @@ namespace ts.moduleSpecifiers {
1657
preferences: ModuleSpecifierPreferences = {},
1758
redirectTargetsMap: RedirectTargetsMap,
1859
): string {
19-
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host);
60+
return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferences(preferences, compilerOptions, importingSourceFile));
61+
}
62+
63+
function getModuleSpecifierWorker(
64+
compilerOptions: CompilerOptions,
65+
importingSourceFileName: Path,
66+
toFileName: string,
67+
host: ModuleSpecifierResolutionHost,
68+
files: ReadonlyArray<SourceFile>,
69+
redirectTargetsMap: RedirectTargetsMap,
70+
preferences: Preferences
71+
): string {
72+
const info = getInfo(importingSourceFileName, host);
2073
const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap);
2174
return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) ||
2275
first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences));
@@ -41,60 +94,46 @@ namespace ts.moduleSpecifiers {
4194
importingSourceFile: SourceFile,
4295
host: ModuleSpecifierResolutionHost,
4396
files: ReadonlyArray<SourceFile>,
44-
preferences: ModuleSpecifierPreferences,
97+
userPreferences: ModuleSpecifierPreferences,
4598
redirectTargetsMap: RedirectTargetsMap,
4699
): ReadonlyArray<ReadonlyArray<string>> {
47100
const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol);
48101
if (ambient) return [[ambient]];
49102

50-
const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.path, host);
103+
const info = getInfo(importingSourceFile.path, host);
51104
if (!files) {
52105
return Debug.fail("Files list must be present to resolve symlinks in specifier resolution");
53106
}
54107
const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration);
55108
const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap);
56109

110+
const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile);
57111
const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions));
58112
return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName =>
59113
getLocalModuleSpecifiers(moduleFileName, info, compilerOptions, preferences));
60114
}
61115

62116
interface Info {
63-
readonly moduleResolutionKind: ModuleResolutionKind;
64-
readonly addJsExtension: boolean;
65117
readonly getCanonicalFileName: GetCanonicalFileName;
66118
readonly sourceDirectory: Path;
67119
}
68120
// importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path
69-
function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
70-
const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions);
71-
const addJsExtension = usesJsExtensionOnImports(importingSourceFile);
121+
function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info {
72122
const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true);
73123
const sourceDirectory = getDirectoryPath(importingSourceFileName);
74-
return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory };
124+
return { getCanonicalFileName, sourceDirectory };
75125
}
76126

77-
function getGlobalModuleSpecifier(
78-
moduleFileName: string,
79-
{ addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
80-
host: ModuleSpecifierResolutionHost,
81-
compilerOptions: CompilerOptions,
82-
) {
83-
return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension)
84-
|| tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory);
127+
function getGlobalModuleSpecifier(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, host: ModuleSpecifierResolutionHost, compilerOptions: CompilerOptions): string | undefined {
128+
return tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory);
85129
}
86130

87-
function getLocalModuleSpecifiers(
88-
moduleFileName: string,
89-
{ moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info,
90-
compilerOptions: CompilerOptions,
91-
preferences: ModuleSpecifierPreferences,
92-
): ReadonlyArray<string> {
131+
function getLocalModuleSpecifiers(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, compilerOptions: CompilerOptions, { ending, relativePreference }: Preferences): ReadonlyArray<string> {
93132
const { baseUrl, paths, rootDirs } = compilerOptions;
94133

95134
const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) ||
96-
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addJsExtension);
97-
if (!baseUrl || preferences.importModuleSpecifierPreference === "relative") {
135+
removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending);
136+
if (!baseUrl || relativePreference === RelativePreference.Relative) {
98137
return [relativePath];
99138
}
100139

@@ -103,19 +142,19 @@ namespace ts.moduleSpecifiers {
103142
return [relativePath];
104143
}
105144

106-
const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addJsExtension);
145+
const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, ending);
107146
if (paths) {
108147
const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths);
109148
if (fromPaths) {
110149
return [fromPaths];
111150
}
112151
}
113152

114-
if (preferences.importModuleSpecifierPreference === "non-relative") {
153+
if (relativePreference === RelativePreference.NonRelative) {
115154
return [importRelativeToBaseUrl];
116155
}
117156

118-
if (preferences.importModuleSpecifierPreference !== undefined) Debug.assertNever(preferences.importModuleSpecifierPreference);
157+
if (relativePreference !== RelativePreference.Auto) Debug.assertNever(relativePreference);
119158

120159
if (isPathRelativeToParent(relativeToBaseUrl)) {
121160
return [relativePath];
@@ -271,37 +310,14 @@ namespace ts.moduleSpecifiers {
271310
return removeFileExtension(relativePath);
272311
}
273312

274-
function tryGetModuleNameFromTypeRoots(
275-
options: CompilerOptions,
276-
host: GetEffectiveTypeRootsHost,
277-
getCanonicalFileName: (file: string) => string,
278-
moduleFileName: string,
279-
addJsExtension: boolean,
280-
): string | undefined {
281-
const roots = getEffectiveTypeRoots(options, host);
282-
return firstDefined(roots, unNormalizedTypeRoot => {
283-
const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName);
284-
if (startsWith(moduleFileName, typeRoot)) {
285-
// For a type definition, we can strip `/index` even with classic resolution.
286-
return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addJsExtension);
287-
}
288-
});
289-
}
290-
291313
function tryGetModuleNameAsNodeModule(
292314
options: CompilerOptions,
293315
moduleFileName: string,
294316
host: ModuleSpecifierResolutionHost,
295317
getCanonicalFileName: (file: string) => string,
296318
sourceDirectory: Path,
297319
): string | undefined {
298-
if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) {
299-
// nothing to do here
300-
return undefined;
301-
}
302-
303320
const parts: NodeModulePathParts = getNodeModulePathParts(moduleFileName)!;
304-
305321
if (!parts) {
306322
return undefined;
307323
}
@@ -313,8 +329,12 @@ namespace ts.moduleSpecifiers {
313329
// Get a path that's relative to node_modules or the importing file's path
314330
// if node_modules folder is in this folder or any of its parent folders, no need to keep it.
315331
if (!startsWith(sourceDirectory, getCanonicalFileName(moduleSpecifier.substring(0, parts.topLevelNodeModulesIndex)))) return undefined;
332+
316333
// If the module was found in @types, get the actual Node package name
317-
return getPackageNameFromAtTypesDirectory(moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1));
334+
const nodeModulesDirectoryName = moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1);
335+
const packageName = getPackageNameFromAtTypesDirectory(nodeModulesDirectoryName);
336+
// For classic resolution, only allow importing from node_modules/@types, not other node_modules
337+
return getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs && packageName === nodeModulesDirectoryName ? undefined : packageName;
318338

319339
function getDirectoryOrExtensionlessFileName(path: string): string {
320340
// If the file is the main module, it can be imported by the package name
@@ -428,13 +448,18 @@ namespace ts.moduleSpecifiers {
428448
});
429449
}
430450

431-
function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addJsExtension: boolean): string {
451+
function removeExtensionAndIndexPostFix(fileName: string, ending: Ending): string {
432452
const noExtension = removeFileExtension(fileName);
433-
return addJsExtension
434-
? noExtension + ".js"
435-
: moduleResolutionKind === ModuleResolutionKind.NodeJs
436-
? removeSuffix(noExtension, "/index")
437-
: noExtension;
453+
switch (ending) {
454+
case Ending.Minimal:
455+
return removeSuffix(noExtension, "/index");
456+
case Ending.Index:
457+
return noExtension;
458+
case Ending.JsExtension:
459+
return noExtension + ".js";
460+
default:
461+
return Debug.assertNever(ending);
462+
}
438463
}
439464

440465
function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: GetCanonicalFileName): string | undefined {

‎src/services/getEditsForFileRename.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ namespace ts {
156156

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

‎tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
5+
// @Filename: /index.js
6+
////export const x = 0;
7+
8+
// @Filename: /a.js
9+
////import { x as x0 } from ".";
10+
////import { x as x1 } from "./index";
11+
////import { x as x2 } from "./index.js";
12+
13+
verify.getEditsForFileRename({
14+
oldPath: "/a.js",
15+
newPath: "/b.js",
16+
newFileContents: {}, // No change
17+
});
18+
19+
verify.getEditsForFileRename({
20+
oldPath: "/b.js",
21+
newPath: "/src/b.js",
22+
newFileContents: {
23+
"/b.js":
24+
`import { x as x0 } from "..";
25+
import { x as x1 } from "../index";
26+
import { x as x2 } from "../index.js";`,
27+
},
28+
});
29+

‎tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts‎

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎tests/cases/fourslash/importNameCodeFix_types_classic.ts‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,22 @@
55
// @Filename: /node_modules/@types/foo/index.d.ts
66
////export const xyz: number;
77

8+
// @Filename: /node_modules/bar/index.d.ts
9+
////export const qrs: number;
10+
811
// @Filename: /a.ts
9-
////[|xyz|]
12+
////xyz;
13+
////qrs;
1014

1115
goTo.file("/a.ts");
12-
verify.importFixAtPosition([
16+
verify.codeFixAll({
17+
fixId: "fixMissingImport",
18+
fixAllDescription: "Add all missing imports",
19+
newFileContent:
1320
`import { xyz } from "foo";
1421
15-
xyz`
16-
]);
22+
import { qrs } from "./node_modules/bar/index";
23+
24+
xyz;
25+
qrs;`,
26+
});

0 commit comments

Comments
 (0)