Skip to content

Commit cfd0a62

Browse files
author
Andy
authored
When renaming module, ensure rename span is just the last component of the path (#27151)
1 parent 4e3e8f5 commit cfd0a62

File tree

4 files changed

+28
-10
lines changed

4 files changed

+28
-10
lines changed

src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4145,6 +4145,10 @@
41454145
"category": "Error",
41464146
"code": 8030
41474147
},
4148+
"You cannot rename a module via a global import.": {
4149+
"category": "Error",
4150+
"code": 8031
4151+
},
41484152
"Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clause.": {
41494153
"category": "Error",
41504154
"code": 9002

src/services/rename.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,18 @@ namespace ts.Rename {
3939
}
4040

4141
function getRenameInfoForModule(node: StringLiteralLike, sourceFile: SourceFile, moduleSymbol: Symbol): RenameInfo | undefined {
42+
if (!isExternalModuleNameRelative(node.text)) {
43+
return getRenameInfoError(Diagnostics.You_cannot_rename_a_module_via_a_global_import);
44+
}
45+
4246
const moduleSourceFile = find(moduleSymbol.declarations, isSourceFile);
4347
if (!moduleSourceFile) return undefined;
4448
const withoutIndex = node.text.endsWith("/index") || node.text.endsWith("/index.js") ? undefined : tryRemoveSuffix(removeFileExtension(moduleSourceFile.fileName), "/index");
4549
const name = withoutIndex === undefined ? moduleSourceFile.fileName : withoutIndex;
4650
const kind = withoutIndex === undefined ? ScriptElementKind.moduleElement : ScriptElementKind.directory;
51+
const indexAfterLastSlash = node.text.lastIndexOf("/") + 1;
52+
// Span should only be the last component of the path. + 1 to account for the quote character.
53+
const triggerSpan = createTextSpan(node.getStart(sourceFile) + 1 + indexAfterLastSlash, node.text.length - indexAfterLastSlash);
4754
return {
4855
canRename: true,
4956
fileToRename: name,
@@ -52,7 +59,7 @@ namespace ts.Rename {
5259
localizedErrorMessage: undefined,
5360
fullDisplayName: name,
5461
kindModifiers: ScriptElementKindModifier.none,
55-
triggerSpan: createTriggerSpanForNode(node, sourceFile),
62+
triggerSpan,
5663
};
5764
}
5865

tests/cases/fourslash/findAllRefs_importType_exportEquals.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
////export = [|T|];
99

1010
// @Filename: /b.ts
11-
////const x: import("[|./a|]") = 0;
12-
////const y: import("[|./a|]").U = "";
11+
////const x: import("[|./[|a|]|]") = 0;
12+
////const y: import("[|./[|a|]|]").U = "";
1313

1414
verify.noErrors();
1515

16-
const [r0, r1, r2, r3, r4] = test.ranges();
16+
const [r0, r1, r2, r3, r3b, r4, r4b] = test.ranges();
1717
verify.referenceGroups(r0, [{ definition: "type T = number\nnamespace T", ranges: [r0, r2, r3] }]);
1818
verify.referenceGroups(r1, [{ definition: "namespace T", ranges: [r1, r2] }]);
1919
verify.referenceGroups(r2, [{ definition: "type T = number\nnamespace T", ranges: [r0, r1, r2, r3] }]);
@@ -25,7 +25,7 @@ verify.referenceGroups([r3, r4], [
2525
verify.renameLocations(r0, [r0, r2]);
2626
verify.renameLocations(r1, [r1, r2]);
2727
verify.renameLocations(r2, [r0, r1, r2]);
28-
for (const range of [r3, r4]) {
28+
for (const range of [r3b, r4b]) {
2929
goTo.rangeStart(range);
3030
verify.renameInfoSucceeded(/*displayName*/ "/a.ts", /*fullDisplayName*/ "/a.ts", /*kind*/ "module", /*kindModifiers*/ "", /*fileToRename*/ "/a.ts", range);
3131
}

tests/cases/fourslash/renameImport.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,24 @@
22

33
// @allowJs: true
44

5+
// @Filename: /node_modules/global/index.d.ts
6+
////export const x: number;
7+
58
// @Filename: /a.ts
69
////export const x = 0;
710

811
// @Filename: /dir/index.ts
912
////export const x = 0;
1013

1114
// @Filename: /b.ts
12-
////import * as a from "[|./a|]";
13-
////import a2 = require("[|./a|]");
14-
////import * as dir from "[|{| "target": "dir" |}./dir|]";
15-
////import * as dir2 from "[|{| "target": "dir/index" |}./dir/index|]";
15+
////import * as a from "./[|a|]";
16+
////import a2 = require("./[|a|]");
17+
////import * as dir from "./[|{| "target": "dir" |}dir|]";
18+
////import * as dir2 from "./dir/[|{| "target": "dir/index" |}index|]";
1619

1720
// @Filename: /c.js
18-
////const a = require("[|./a|]");
21+
////const a = require("./[|a|]");
22+
////const global = require("/*global*/global");
1923

2024
verify.noErrors();
2125
goTo.eachRange(range => {
@@ -24,3 +28,6 @@ goTo.eachRange(range => {
2428
const kind = target === "dir" ? "directory" : "module";
2529
verify.renameInfoSucceeded(/*displayName*/ name, /*fullDisplayName*/ name, /*kind*/ kind, /*kindModifiers*/ "", /*fileToRename*/ name, range);
2630
});
31+
32+
goTo.marker("global");
33+
verify.renameInfoFailed("You cannot rename a module via a global import.");

0 commit comments

Comments
 (0)