From 8dfbfcb05809940333b60ba209ace7e7b896dcf7 Mon Sep 17 00:00:00 2001 From: Isabel Duan Date: Fri, 8 Dec 2023 09:32:52 -0800 Subject: [PATCH] fix54492: allow editor to check for original file extension for rename (#56680) --- src/server/protocol.ts | 1 + src/services/rename.ts | 8 +- src/services/types.ts | 4 + src/testRunner/unittests/tsserver/rename.ts | 19 ++ tests/baselines/reference/api/typescript.d.ts | 5 + .../rename-TS-file-with-js-extension.js | 197 ++++++++++++++++++ .../rename/works-with-fileToRename.js | 4 +- .../findAllRefs_importType_exportEquals.ts | 2 +- tests/cases/fourslash/renameImport.ts | 5 +- 9 files changed, 236 insertions(+), 9 deletions(-) create mode 100644 tests/baselines/reference/tsserver/rename/rename-TS-file-with-js-extension.js diff --git a/src/server/protocol.ts b/src/server/protocol.ts index daa1738650b..ae6c5bc0551 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1336,6 +1336,7 @@ export interface RenameInfoSuccess { /** * Full display name of item to be renamed. + * If item to be renamed is a file, then this is the original text of the module specifer */ fullDisplayName: string; diff --git a/src/services/rename.ts b/src/services/rename.ts index 4083612dae4..1260fef23d5 100644 --- a/src/services/rename.ts +++ b/src/services/rename.ts @@ -185,17 +185,17 @@ function getRenameInfoForModule(node: StringLiteralLike, sourceFile: SourceFile, const moduleSourceFile = moduleSymbol.declarations && find(moduleSymbol.declarations, isSourceFile); if (!moduleSourceFile) return undefined; const withoutIndex = endsWith(node.text, "/index") || endsWith(node.text, "/index.js") ? undefined : tryRemoveSuffix(removeFileExtension(moduleSourceFile.fileName), "/index"); - const name = withoutIndex === undefined ? moduleSourceFile.fileName : withoutIndex; + const fileName = withoutIndex === undefined ? moduleSourceFile.fileName : withoutIndex; const kind = withoutIndex === undefined ? ScriptElementKind.moduleElement : ScriptElementKind.directory; const indexAfterLastSlash = node.text.lastIndexOf("/") + 1; // Span should only be the last component of the path. + 1 to account for the quote character. const triggerSpan = createTextSpan(node.getStart(sourceFile) + 1 + indexAfterLastSlash, node.text.length - indexAfterLastSlash); return { canRename: true, - fileToRename: name, + fileToRename: fileName, kind, - displayName: name, - fullDisplayName: name, + displayName: fileName, + fullDisplayName: node.text, kindModifiers: ScriptElementKindModifier.none, triggerSpan, }; diff --git a/src/services/types.ts b/src/services/types.ts index cfa61dc9195..9bb560af02a 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -1291,6 +1291,10 @@ export interface RenameInfoSuccess { */ fileToRename?: string; displayName: string; + /** + * Full display name of item to be renamed. + * If item to be renamed is a file, then this is the original text of the module specifer + */ fullDisplayName: string; kind: ScriptElementKind; kindModifiers: string; diff --git a/src/testRunner/unittests/tsserver/rename.ts b/src/testRunner/unittests/tsserver/rename.ts index 6ab02ae47b7..c967cf05a6a 100644 --- a/src/testRunner/unittests/tsserver/rename.ts +++ b/src/testRunner/unittests/tsserver/rename.ts @@ -193,4 +193,23 @@ describe("unittests:: tsserver:: rename", () => { }); baselineTsserverLogs("rename", "with symlinks and case difference", session); }); + + it("rename TS file with js extension", () => { + const aTs: File = { path: "/a.ts", content: "export const a = 1;" }; + const bTs: File = { path: "/b.ts", content: `import * as foo from './a.js';` }; + + const host = createServerHost([aTs, bTs]); + const session = new TestSession(host); + openFilesForSession([aTs, bTs], session); + + session.executeCommandSeq({ + command: ts.server.protocol.CommandTypes.Configure, + arguments: { preferences: { allowRenameOfImportPath: true } }, + }); + session.executeCommandSeq({ + command: ts.server.protocol.CommandTypes.Rename, + arguments: protocolFileLocationFromSubstring(bTs, "a.js"), + }); + baselineTsserverLogs("rename", "rename TS file with js extension", session); + }); }); diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 450444f4cc7..1ed2cd96de1 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -1087,6 +1087,7 @@ declare namespace ts { displayName: string; /** * Full display name of item to be renamed. + * If item to be renamed is a file, then this is the original text of the module specifer */ fullDisplayName: string; /** @@ -11092,6 +11093,10 @@ declare namespace ts { */ fileToRename?: string; displayName: string; + /** + * Full display name of item to be renamed. + * If item to be renamed is a file, then this is the original text of the module specifer + */ fullDisplayName: string; kind: ScriptElementKind; kindModifiers: string; diff --git a/tests/baselines/reference/tsserver/rename/rename-TS-file-with-js-extension.js b/tests/baselines/reference/tsserver/rename/rename-TS-file-with-js-extension.js new file mode 100644 index 00000000000..dd2d27ac17a --- /dev/null +++ b/tests/baselines/reference/tsserver/rename/rename-TS-file-with-js-extension.js @@ -0,0 +1,197 @@ +currentDirectory:: / useCaseSensitiveFileNames: false +Info seq [hh:mm:ss:mss] Provided types map file "/typesMap.json" doesn't exist +Before request +//// [/a.ts] +export const a = 1; + +//// [/b.ts] +import * as foo from './a.js'; + + +Info seq [hh:mm:ss:mss] request: + { + "command": "open", + "arguments": { + "file": "/a.ts" + }, + "seq": 1, + "type": "request" + } +Info seq [hh:mm:ss:mss] Search path: / +Info seq [hh:mm:ss:mss] For info: /a.ts :: No config files found. +Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /dev/null/inferredProject1* +Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 500 undefined Project: /dev/null/inferredProject1* WatchType: Missing file +Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /dev/null/inferredProject1* Version: 1 structureChanged: true structureIsReused:: Not Elapsed:: *ms +Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject1*' (Inferred) +Info seq [hh:mm:ss:mss] Files (1) + /a.ts SVC-1-0 "export const a = 1;" + + + a.ts + Root file specified for compilation + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject1*' (Inferred) +Info seq [hh:mm:ss:mss] Files (1) + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] Open files: +Info seq [hh:mm:ss:mss] FileName: /a.ts ProjectRootPath: undefined +Info seq [hh:mm:ss:mss] Projects: /dev/null/inferredProject1* +Info seq [hh:mm:ss:mss] response: + { + "responseRequired": false + } +After request + +PolledWatches:: +/a/lib/lib.d.ts: *new* + {"pollingInterval":500} + +Before request + +Info seq [hh:mm:ss:mss] request: + { + "command": "open", + "arguments": { + "file": "/b.ts" + }, + "seq": 2, + "type": "request" + } +Info seq [hh:mm:ss:mss] Search path: / +Info seq [hh:mm:ss:mss] For info: /b.ts :: No config files found. +Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /dev/null/inferredProject2* +Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 500 undefined Project: /dev/null/inferredProject2* WatchType: Missing file +Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /dev/null/inferredProject2* Version: 1 structureChanged: true structureIsReused:: Not Elapsed:: *ms +Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject2*' (Inferred) +Info seq [hh:mm:ss:mss] Files (2) + /a.ts SVC-1-0 "export const a = 1;" + /b.ts SVC-1-0 "import * as foo from './a.js';" + + + a.ts + Imported via './a.js' from file 'b.ts' + b.ts + Root file specified for compilation + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] `remove Project:: +Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject1*' (Inferred) +Info seq [hh:mm:ss:mss] Files (1) + /a.ts + + + a.ts + Root file specified for compilation + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] FileWatcher:: Close:: WatchInfo: /a/lib/lib.d.ts 500 undefined Project: /dev/null/inferredProject1* WatchType: Missing file +Info seq [hh:mm:ss:mss] Project '/dev/null/inferredProject2*' (Inferred) +Info seq [hh:mm:ss:mss] Files (2) + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] Open files: +Info seq [hh:mm:ss:mss] FileName: /a.ts ProjectRootPath: undefined +Info seq [hh:mm:ss:mss] Projects: /dev/null/inferredProject2* +Info seq [hh:mm:ss:mss] FileName: /b.ts ProjectRootPath: undefined +Info seq [hh:mm:ss:mss] Projects: /dev/null/inferredProject2* +Info seq [hh:mm:ss:mss] response: + { + "responseRequired": false + } +After request + +Before request + +Info seq [hh:mm:ss:mss] request: + { + "command": "configure", + "arguments": { + "preferences": { + "allowRenameOfImportPath": true + } + }, + "seq": 3, + "type": "request" + } +Info seq [hh:mm:ss:mss] response: + { + "seq": 0, + "type": "response", + "command": "configure", + "request_seq": 3, + "success": true, + "performanceData": { + "updateGraphDurationMs": * + } + } +Info seq [hh:mm:ss:mss] response: + { + "responseRequired": false + } +After request + +Before request + +Info seq [hh:mm:ss:mss] request: + { + "command": "rename", + "arguments": { + "file": "/b.ts", + "line": 1, + "offset": 25 + }, + "seq": 4, + "type": "request" + } +Info seq [hh:mm:ss:mss] response: + { + "response": { + "info": { + "canRename": true, + "fileToRename": "/a.ts", + "displayName": "/a.ts", + "fullDisplayName": "./a.js", + "kind": "module", + "kindModifiers": "", + "triggerSpan": { + "start": { + "line": 1, + "offset": 25 + }, + "end": { + "line": 1, + "offset": 29 + } + } + }, + "locs": [ + { + "file": "/b.ts", + "locs": [ + { + "start": { + "line": 1, + "offset": 23 + }, + "end": { + "line": 1, + "offset": 29 + }, + "contextStart": { + "line": 1, + "offset": 1 + }, + "contextEnd": { + "line": 1, + "offset": 31 + } + } + ] + } + ] + }, + "responseRequired": true + } +After request diff --git a/tests/baselines/reference/tsserver/rename/works-with-fileToRename.js b/tests/baselines/reference/tsserver/rename/works-with-fileToRename.js index ecc3762016c..d43f0f7596b 100644 --- a/tests/baselines/reference/tsserver/rename/works-with-fileToRename.js +++ b/tests/baselines/reference/tsserver/rename/works-with-fileToRename.js @@ -132,7 +132,7 @@ Info seq [hh:mm:ss:mss] response: "canRename": true, "fileToRename": "/a.ts", "displayName": "/a.ts", - "fullDisplayName": "/a.ts", + "fullDisplayName": "./a", "kind": "module", "kindModifiers": "", "triggerSpan": { @@ -259,7 +259,7 @@ Info seq [hh:mm:ss:mss] response: "canRename": true, "fileToRename": "/a.ts", "displayName": "/a.ts", - "fullDisplayName": "/a.ts", + "fullDisplayName": "./a", "kind": "module", "kindModifiers": "", "triggerSpan": { diff --git a/tests/cases/fourslash/findAllRefs_importType_exportEquals.ts b/tests/cases/fourslash/findAllRefs_importType_exportEquals.ts index 2b815911a1b..8b5961e859e 100644 --- a/tests/cases/fourslash/findAllRefs_importType_exportEquals.ts +++ b/tests/cases/fourslash/findAllRefs_importType_exportEquals.ts @@ -17,6 +17,6 @@ verify.baselineFindAllReferences('0', '1', '2', '3', '4', 'export'); verify.baselineRename([r0, r1, r2]); for (const range of [r3b, r4b]) { goTo.rangeStart(range); - verify.renameInfoSucceeded(/*displayName*/ "/a.ts", /*fullDisplayName*/ "/a.ts", /*kind*/ "module", /*kindModifiers*/ "", /*fileToRename*/ "/a.ts", range); + verify.renameInfoSucceeded(/*displayName*/ "/a.ts", /*fullDisplayName*/ "./a", /*kind*/ "module", /*kindModifiers*/ "", /*fileToRename*/ "/a.ts", range); verify.renameInfoFailed("You cannot rename this element.", { allowRenameOfImportPath: false }); } diff --git a/tests/cases/fourslash/renameImport.ts b/tests/cases/fourslash/renameImport.ts index 21ec3420462..4819098619b 100644 --- a/tests/cases/fourslash/renameImport.ts +++ b/tests/cases/fourslash/renameImport.ts @@ -24,9 +24,10 @@ verify.noErrors(); goTo.eachRange(range => { const target = range.marker && range.marker.data && range.marker.data.target; - const name = target === "dir" ? "/dir" : target === "dir/index" ? "/dir/index.ts" : "/a.ts"; + const displayName = target === "dir" ? "./dir" : target === "dir/index" ? "./dir/index" : "./a"; + const fileName = target === "dir" ? "/dir" : target === "dir/index" ? "/dir/index.ts" : "/a.ts"; const kind = target === "dir" ? "directory" : "module"; - verify.renameInfoSucceeded(/*displayName*/ name, /*fullDisplayName*/ name, /*kind*/ kind, /*kindModifiers*/ "", /*fileToRename*/ name, range); + verify.renameInfoSucceeded(/*displayName*/ fileName, /*fullDisplayName*/ displayName, /*kind*/ kind, /*kindModifiers*/ "", /*fileToRename*/ fileName, range); verify.renameInfoFailed("You cannot rename this element.", { allowRenameOfImportPath: false }); });