Skip to content

Commit b94061c

Browse files
author
Andy
authored
getEditsForFileRename: Avoid changing import specifier ending (#26177)
* getEditsForFileRename: Avoid changing import specifier ending * Support .json and .jsx extensions * Restore typeRoots tests * Fix json test * When --jsx preserve is set, import ".tsx" file with ".jsx" extension * Support ending preference in UserPreferences
1 parent 3931b72 commit b94061c

15 files changed

+309
-187
lines changed

‎src/compiler/checker.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3926,13 +3926,22 @@ namespace ts {
39263926
const links = getSymbolLinks(symbol);
39273927
let specifier = links.specifierCache && links.specifierCache.get(contextFile.path);
39283928
if (!specifier) {
3929-
specifier = moduleSpecifiers.getModuleSpecifierForDeclarationFile(
3929+
const isBundle = (compilerOptions.out || compilerOptions.outFile);
3930+
// For declaration bundles, we need to generate absolute paths relative to the common source dir for imports,
3931+
// just like how the declaration emitter does for the ambient module declarations - we can easily accomplish this
3932+
// using the `baseUrl` compiler option (which we would otherwise never use in declaration emit) and a non-relative
3933+
// specifier preference
3934+
const { moduleResolverHost } = context.tracker;
3935+
const specifierCompilerOptions = isBundle ? { ...compilerOptions, baseUrl: moduleResolverHost.getCommonSourceDirectory() } : compilerOptions;
3936+
specifier = first(first(moduleSpecifiers.getModuleSpecifiers(
39303937
symbol,
3931-
compilerOptions,
3938+
specifierCompilerOptions,
39323939
contextFile,
3933-
context.tracker.moduleResolverHost,
3940+
moduleResolverHost,
3941+
host.getSourceFiles(),
3942+
{ importModuleSpecifierPreference: isBundle ? "non-relative" : "relative" },
39343943
host.redirectTargetsMap,
3935-
);
3944+
)));
39363945
links.specifierCache = links.specifierCache || createMap();
39373946
links.specifierCache.set(contextFile.path, specifier);
39383947
}

‎src/compiler/moduleSpecifiers.ts

Lines changed: 104 additions & 94 deletions
Large diffs are not rendered by default.

‎src/compiler/types.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5331,8 +5331,6 @@ namespace ts {
53315331
useCaseSensitiveFileNames?(): boolean;
53325332
fileExists?(path: string): boolean;
53335333
readFile?(path: string): string | undefined;
5334-
getSourceFiles?(): ReadonlyArray<SourceFile>; // Used for cached resolutions to find symlinks without traversing the fs (again)
5335-
getCommonSourceDirectory?(): string;
53365334
}
53375335

53385336
// Note: this used to be deprecated in our public API, but is still used internally
@@ -5345,7 +5343,7 @@ namespace ts {
53455343
reportInaccessibleThisError?(): void;
53465344
reportPrivateInBaseOfClassExpression?(propertyName: string): void;
53475345
reportInaccessibleUniqueSymbolError?(): void;
5348-
moduleResolverHost?: ModuleSpecifierResolutionHost;
5346+
moduleResolverHost?: EmitHost;
53495347
trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void;
53505348
trackExternalModuleSymbolOfImportTypeNode?(symbol: Symbol): void;
53515349
}
@@ -5595,4 +5593,15 @@ namespace ts {
55955593
get<TKey extends keyof PragmaPsuedoMap>(key: TKey): PragmaPsuedoMap[TKey] | PragmaPsuedoMap[TKey][];
55965594
forEach(action: <TKey extends keyof PragmaPsuedoMap>(value: PragmaPsuedoMap[TKey] | PragmaPsuedoMap[TKey][], key: TKey) => void): void;
55975595
}
5596+
5597+
export interface UserPreferences {
5598+
readonly disableSuggestions?: boolean;
5599+
readonly quotePreference?: "double" | "single";
5600+
readonly includeCompletionsForModuleExports?: boolean;
5601+
readonly includeCompletionsWithInsertText?: boolean;
5602+
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
5603+
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
5604+
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
5605+
readonly allowTextChangesInNewFiles?: boolean;
5606+
}
55985607
}

‎src/compiler/utilities.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7936,6 +7936,7 @@ namespace ts {
79367936
/** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */
79377937
export const supportedTypescriptExtensionsForExtractExtension: ReadonlyArray<Extension> = [Extension.Dts, Extension.Ts, Extension.Tsx];
79387938
export const supportedJavascriptExtensions: ReadonlyArray<Extension> = [Extension.Js, Extension.Jsx];
7939+
export const supportedJavaScriptAndJsonExtensions: ReadonlyArray<Extension> = [Extension.Js, Extension.Jsx, Extension.Json];
79397940
const allSupportedExtensions: ReadonlyArray<Extension> = [...supportedTypeScriptExtensions, ...supportedJavascriptExtensions];
79407941

79417942
export function getSupportedExtensions(options?: CompilerOptions, extraFileExtensions?: ReadonlyArray<FileExtensionInfo>): ReadonlyArray<string> {
@@ -7961,6 +7962,10 @@ namespace ts {
79617962
return some(supportedJavascriptExtensions, extension => fileExtensionIs(fileName, extension));
79627963
}
79637964

7965+
export function hasJavaScriptOrJsonFileExtension(fileName: string): boolean {
7966+
return supportedJavaScriptAndJsonExtensions.some(ext => fileExtensionIs(fileName, ext));
7967+
}
7968+
79647969
export function hasTypeScriptFileExtension(fileName: string): boolean {
79657970
return some(supportedTypeScriptExtensions, extension => fileExtensionIs(fileName, extension));
79667971
}

‎src/services/getEditsForFileRename.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace ts {
66
newFileOrDirPath: string,
77
host: LanguageServiceHost,
88
formatContext: formatting.FormatContext,
9-
preferences: UserPreferences,
9+
_preferences: UserPreferences,
1010
sourceMapper: SourceMapper,
1111
): ReadonlyArray<FileTextChanges> {
1212
const useCaseSensitiveFileNames = hostUsesCaseSensitiveFileNames(host);
@@ -15,7 +15,7 @@ namespace ts {
1515
const newToOld = getPathUpdater(newFileOrDirPath, oldFileOrDirPath, getCanonicalFileName, sourceMapper);
1616
return textChanges.ChangeTracker.with({ host, formatContext }, changeTracker => {
1717
updateTsconfigFiles(program, changeTracker, oldToNew, newFileOrDirPath, host.getCurrentDirectory(), useCaseSensitiveFileNames);
18-
updateImports(program, changeTracker, oldToNew, newToOld, host, getCanonicalFileName, preferences);
18+
updateImports(program, changeTracker, oldToNew, newToOld, host, getCanonicalFileName);
1919
});
2020
}
2121

@@ -122,7 +122,6 @@ namespace ts {
122122
newToOld: PathUpdater,
123123
host: LanguageServiceHost,
124124
getCanonicalFileName: GetCanonicalFileName,
125-
preferences: UserPreferences,
126125
): void {
127126
const allFiles = program.getSourceFiles();
128127
for (const sourceFile of allFiles) {
@@ -156,7 +155,7 @@ namespace ts {
156155

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

212211
function updateImportsWorker(sourceFile: SourceFile, changeTracker: textChanges.ChangeTracker, updateRef: (refText: string) => string | undefined, updateImport: (importLiteral: StringLiteralLike) => string | undefined) {
213-
for (const ref of sourceFile.referencedFiles) {
212+
for (const ref of sourceFile.referencedFiles || emptyArray) { // TODO: GH#26162
214213
const updated = updateRef(ref.fileName);
215214
if (updated !== undefined && updated !== sourceFile.text.slice(ref.pos, ref.end)) changeTracker.replaceRangeWithText(sourceFile, ref, updated);
216215
}

‎src/services/types.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,6 @@ namespace ts {
233233
installPackage?(options: InstallPackageOptions): Promise<ApplyCodeActionCommandResult>;
234234
}
235235

236-
export interface UserPreferences {
237-
readonly disableSuggestions?: boolean;
238-
readonly quotePreference?: "double" | "single";
239-
readonly includeCompletionsForModuleExports?: boolean;
240-
readonly includeCompletionsWithInsertText?: boolean;
241-
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
242-
readonly allowTextChangesInNewFiles?: boolean;
243-
}
244236
/* @internal */
245237
export const emptyOptions = {};
246238

‎tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2990,6 +2990,16 @@ declare namespace ts {
29902990
Parameters = 1296,
29912991
IndexSignatureParameters = 4432
29922992
}
2993+
interface UserPreferences {
2994+
readonly disableSuggestions?: boolean;
2995+
readonly quotePreference?: "double" | "single";
2996+
readonly includeCompletionsForModuleExports?: boolean;
2997+
readonly includeCompletionsWithInsertText?: boolean;
2998+
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
2999+
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
3000+
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
3001+
readonly allowTextChangesInNewFiles?: boolean;
3002+
}
29933003
}
29943004
declare function setTimeout(handler: (...args: any[]) => void, timeout: number): any;
29953005
declare function clearTimeout(handle: any): void;
@@ -4636,14 +4646,6 @@ declare namespace ts {
46364646
isKnownTypesPackageName?(name: string): boolean;
46374647
installPackage?(options: InstallPackageOptions): Promise<ApplyCodeActionCommandResult>;
46384648
}
4639-
interface UserPreferences {
4640-
readonly disableSuggestions?: boolean;
4641-
readonly quotePreference?: "double" | "single";
4642-
readonly includeCompletionsForModuleExports?: boolean;
4643-
readonly includeCompletionsWithInsertText?: boolean;
4644-
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
4645-
readonly allowTextChangesInNewFiles?: boolean;
4646-
}
46474649
interface LanguageService {
46484650
cleanupSemanticCache(): void;
46494651
getSyntacticDiagnostics(fileName: string): DiagnosticWithLocation[];

‎tests/baselines/reference/api/typescript.d.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2990,6 +2990,16 @@ declare namespace ts {
29902990
Parameters = 1296,
29912991
IndexSignatureParameters = 4432
29922992
}
2993+
interface UserPreferences {
2994+
readonly disableSuggestions?: boolean;
2995+
readonly quotePreference?: "double" | "single";
2996+
readonly includeCompletionsForModuleExports?: boolean;
2997+
readonly includeCompletionsWithInsertText?: boolean;
2998+
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
2999+
/** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */
3000+
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
3001+
readonly allowTextChangesInNewFiles?: boolean;
3002+
}
29933003
}
29943004
declare function setTimeout(handler: (...args: any[]) => void, timeout: number): any;
29953005
declare function clearTimeout(handle: any): void;
@@ -4636,14 +4646,6 @@ declare namespace ts {
46364646
isKnownTypesPackageName?(name: string): boolean;
46374647
installPackage?(options: InstallPackageOptions): Promise<ApplyCodeActionCommandResult>;
46384648
}
4639-
interface UserPreferences {
4640-
readonly disableSuggestions?: boolean;
4641-
readonly quotePreference?: "double" | "single";
4642-
readonly includeCompletionsForModuleExports?: boolean;
4643-
readonly includeCompletionsWithInsertText?: boolean;
4644-
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
4645-
readonly allowTextChangesInNewFiles?: boolean;
4646-
}
46474649
interface LanguageService {
46484650
cleanupSemanticCache(): void;
46494651
getSyntacticDiagnostics(fileName: string): DiagnosticWithLocation[];

‎tests/cases/fourslash/fourslash.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -528,10 +528,11 @@ declare namespace FourSlashInterface {
528528
filesToSearch?: ReadonlyArray<string>;
529529
}
530530
interface UserPreferences {
531-
quotePreference?: "double" | "single";
532-
includeCompletionsForModuleExports?: boolean;
533-
includeInsertTextCompletions?: boolean;
534-
importModuleSpecifierPreference?: "relative" | "non-relative";
531+
readonly quotePreference?: "double" | "single";
532+
readonly includeCompletionsForModuleExports?: boolean;
533+
readonly includeInsertTextCompletions?: boolean;
534+
readonly importModuleSpecifierPreference?: "relative" | "non-relative";
535+
readonly importModuleSpecifierEnding?: "minimal" | "index" | "js";
535536
}
536537
interface CompletionsAtOptions extends UserPreferences {
537538
triggerCharacter?: string;

‎tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @allowJs: true
4+
// @checkJs: true
5+
// @strict: true
6+
// @jsx: preserve
7+
// @resolveJsonModule: true
8+
9+
// @Filename: /index.js
10+
////export const x = 0;
11+
12+
// @Filename: /jsx.jsx
13+
////export const y = 0;
14+
15+
// @Filename: /j.jonah.json
16+
////{ "j": 0 }
17+
18+
// @Filename: /a.js
19+
////import { x as x0 } from ".";
20+
////import { x as x1 } from "./index";
21+
////import { x as x2 } from "./index.js";
22+
////import { y } from "./jsx.jsx";
23+
////import { j } from "./j.jonah.json";
24+
25+
verify.noErrors();
26+
27+
verify.getEditsForFileRename({
28+
oldPath: "/a.js",
29+
newPath: "/b.js",
30+
newFileContents: {}, // No change
31+
});
32+
33+
verify.getEditsForFileRename({
34+
oldPath: "/b.js",
35+
newPath: "/src/b.js",
36+
newFileContents: {
37+
"/b.js":
38+
`import { x as x0 } from "..";
39+
import { x as x1 } from "../index";
40+
import { x as x2 } from "../index.js";
41+
import { y } from "../jsx.jsx";
42+
import { j } from "../j.jonah.json";`,
43+
},
44+
});

‎tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
1-
/// <reference path="fourslash.ts" />
2-
3-
// @Filename: a/f1.ts
4-
//// [|foo/*0*/();|]
5-
6-
// @Filename: types/random/index.ts
7-
//// export function foo() {};
8-
9-
// @Filename: tsconfig.json
10-
//// {
11-
//// "compilerOptions": {
12-
//// "typeRoots": [
13-
//// "./types"
14-
//// ]
15-
//// }
16-
//// }
17-
18-
verify.importFixAtPosition([
19-
`import { foo } from "random";
20-
21-
foo();`
22-
]);
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a/f1.ts
4+
//// [|foo/*0*/();|]
5+
6+
// @Filename: types/random/index.ts
7+
//// export function foo() {};
8+
9+
// @Filename: tsconfig.json
10+
//// {
11+
//// "compilerOptions": {
12+
//// "typeRoots": [
13+
//// "./types"
14+
//// ]
15+
//// }
16+
//// }
17+
18+
// "typeRoots" does not affect module resolution. Importing from "random" would be a compile error.
19+
verify.importFixAtPosition([
20+
`import { foo } from "../types/random";
21+
22+
foo();`
23+
]);

‎tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,27 @@
1-
/// <reference path="fourslash.ts" />
2-
3-
// @Filename: a/f1.ts
4-
//// [|foo/*0*/();|]
5-
6-
// @Filename: types/random/index.ts
7-
//// export function foo() {};
8-
9-
// @Filename: tsconfig.json
10-
//// {
11-
//// "compilerOptions": {
12-
//// "baseUrl": ".",
13-
//// "typeRoots": [
14-
//// "./types"
15-
//// ]
16-
//// }
17-
//// }
18-
19-
verify.importFixAtPosition([
20-
`import { foo } from "random";
21-
22-
foo();`
23-
]);
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: a/f1.ts
4+
//// [|foo/*0*/();|]
5+
6+
// @Filename: types/random/index.ts
7+
//// export function foo() {};
8+
9+
// @Filename: tsconfig.json
10+
//// {
11+
//// "compilerOptions": {
12+
//// "baseUrl": ".",
13+
//// "typeRoots": [
14+
//// "./types"
15+
//// ]
16+
//// }
17+
//// }
18+
19+
// "typeRoots" does not affect module resolution. Importing from "random" would be a compile error.
20+
verify.importFixAtPosition([
21+
`import { foo } from "types/random";
22+
23+
foo();`,
24+
`import { foo } from "../types/random";
25+
26+
foo();`
27+
]);

‎tests/cases/fourslash/importNameCodeFix_endingPreference.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @moduleResolution: node
4+
5+
// @Filename: /foo/index.ts
6+
////export const foo = 0;
7+
8+
// @Filename: /a.ts
9+
////foo;
10+
11+
// @Filename: /b.ts
12+
////foo;
13+
14+
// @Filename: /c.ts
15+
////foo;
16+
17+
const tests: ReadonlyArray<[string, FourSlashInterface.UserPreferences["importModuleSpecifierEnding"], string]> = [
18+
["/a.ts", "js", "./foo/index.js"],
19+
["/b.ts", "index", "./foo/index"],
20+
["/c.ts", "minimal", "./foo"],
21+
];
22+
23+
for (const [fileName, importModuleSpecifierEnding, specifier] of tests) {
24+
goTo.file(fileName);
25+
verify.importFixAtPosition([`import { foo } from "${specifier}";\n\nfoo;`,], /*errorCode*/ undefined, { importModuleSpecifierEnding });
26+
}

0 commit comments

Comments
 (0)