From 3c52c4fd98d2293ed54b7e075d22b448092ab37c Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 18 May 2023 18:39:04 +0000 Subject: [PATCH] Cherry-pick PR #54300 into release-5.1 Component commits: d2fa7002e4 fix(54283): add notApplicableReason to response. allow the 'moveToFile' refactoring exclusively for js/ts files --- src/compiler/diagnosticMessages.json | 4 + src/harness/client.ts | 5 +- src/server/protocol.ts | 1 + src/server/session.ts | 3 +- src/services/refactors/moveToFile.ts | 10 +- src/services/types.ts | 1 + .../unittests/tsserver/refactors.ts | 75 +++++++++++---- .../reference/api/tsserverlibrary.d.ts | 2 + tests/baselines/reference/api/typescript.d.ts | 1 + ...es-moving-statement-to-an-existing-file.js | 8 +- ...dles-moving-statements-to-a-non-TS-file.js | 96 +++++++++++++++++++ 11 files changed, 176 insertions(+), 30 deletions(-) create mode 100644 tests/baselines/reference/tsserver/refactors/handles-moving-statements-to-a-non-TS-file.js diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 62f4bf4829b18..a9dd5ddf857f2 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -7612,6 +7612,10 @@ "category": "Message", "code": 95178 }, + "Cannot move to file, selected file is invalid": { + "category": "Message", + "code": 95179 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/harness/client.ts b/src/harness/client.ts index e56cf5a206347..a97fde1279ed9 100644 --- a/src/harness/client.ts +++ b/src/harness/client.ts @@ -818,7 +818,7 @@ export class SessionClient implements LanguageService { const response = this.processResponse(request); if (!response.body) { - return { edits: [], renameFilename: undefined, renameLocation: undefined }; + return { edits: [], renameFilename: undefined, renameLocation: undefined, notApplicableReason: undefined }; } const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits); @@ -832,7 +832,8 @@ export class SessionClient implements LanguageService { return { edits, renameFilename, - renameLocation + renameLocation, + notApplicableReason: response.body.notApplicableReason, }; } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 4e4432d0bc2e7..0538a71798850 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -721,6 +721,7 @@ export interface RefactorEditInfo { */ renameLocation?: Location; renameFilename?: string; + notApplicableReason?: string; } /** diff --git a/src/server/session.ts b/src/server/session.ts index 5d6eb52337748..9f7786f7ecb3a 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -2707,7 +2707,8 @@ export class Session implements EventSender { return { renameLocation: mappedRenameLocation, renameFilename, - edits: this.mapTextChangesToCodeEdits(edits) + edits: this.mapTextChangesToCodeEdits(edits), + notApplicableReason: result.notApplicableReason, }; } return result; diff --git a/src/services/refactors/moveToFile.ts b/src/services/refactors/moveToFile.ts index 20ee5c05647d1..aa566694e8a9b 100644 --- a/src/services/refactors/moveToFile.ts +++ b/src/services/refactors/moveToFile.ts @@ -53,7 +53,9 @@ import { getRelativePathFromFile, getSynthesizedDeepClone, getUniqueName, + hasJSFileExtension, hasSyntacticModifier, + hasTSFileExtension, hostGetCanonicalFileName, Identifier, ImportDeclaration, @@ -162,8 +164,12 @@ registerRefactor(refactorNameForMoveToFile, { Debug.assert(actionName === refactorNameForMoveToFile, "Wrong refactor invoked"); const statements = Debug.checkDefined(getStatementsToMove(context)); Debug.assert(interactiveRefactorArguments, "No interactive refactor arguments available"); - const edits = textChanges.ChangeTracker.with(context, t => doChange(context, context.file, interactiveRefactorArguments.targetFile, context.program, statements, t, context.host, context.preferences)); - return { edits, renameFilename: undefined, renameLocation: undefined }; + const targetFile = interactiveRefactorArguments.targetFile; + if (hasJSFileExtension(targetFile) || hasTSFileExtension(targetFile)) { + const edits = textChanges.ChangeTracker.with(context, t => doChange(context, context.file, interactiveRefactorArguments.targetFile, context.program, statements, t, context.host, context.preferences)); + return { edits, renameFilename: undefined, renameLocation: undefined }; + } + return { edits: [], renameFilename: undefined, renameLocation: undefined, notApplicableReason: getLocaleSpecificMessage(Diagnostics.Cannot_move_to_file_selected_file_is_invalid) }; } }); diff --git a/src/services/types.ts b/src/services/types.ts index beccf81d1f109..c8c670b9ebfca 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -1002,6 +1002,7 @@ export interface RefactorEditInfo { renameFilename?: string; renameLocation?: number; commands?: CodeActionCommand[]; + notApplicableReason?: string; } export type RefactorTriggerReason = "implicit" | "invoked"; diff --git a/src/testRunner/unittests/tsserver/refactors.ts b/src/testRunner/unittests/tsserver/refactors.ts index 49ae2f77aeb57..71d68dfac7372 100644 --- a/src/testRunner/unittests/tsserver/refactors.ts +++ b/src/testRunner/unittests/tsserver/refactors.ts @@ -96,28 +96,61 @@ describe("unittests:: tsserver:: refactors", () => { }); it("handles moving statement to an existing file", () => { - const aTs: File = { path: "/Foo/a.ts", content: "const x = 0;" }; - const bTs: File = { - path: "/Foo/b.ts", content: `import {} from "./bar"; -const a = 1;`}; - const tsconfig: File = { path: "/Foo/tsconfig.json", content: `{ "files": ["./a.ts", "./b.ts"] }` }; - const host = createServerHost([aTs, bTs, tsconfig]); - const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) }); - openFilesForSession([aTs], session); + const aTs: File = { path: "/Foo/a.ts", content: "const x = 0;" }; + const bTs: File = { + path: "/Foo/b.ts", content: `import {} from "./bar"; + const a = 1;`}; + const tsconfig: File = { path: "/Foo/tsconfig.json", content: `{ "files": ["./a.ts", "./b.ts"] }` }; + const host = createServerHost([aTs, bTs, tsconfig]); + const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) }); + openFilesForSession([aTs], session); - session.executeCommandSeq({ - command: ts.server.protocol.CommandTypes.GetEditsForRefactor, - arguments: { - file: aTs.path, - startLine: 1, - startOffset: 1, - endLine: 2, - endOffset: aTs.content.length, - refactor: "Move to file", - action: "Move to file", - interactiveRefactorArguments: { targetFile: "/Foo/b.ts" }, - } + session.executeCommandSeq({ + command: ts.server.protocol.CommandTypes.GetEditsForRefactor, + arguments: { + file: aTs.path, + startLine: 1, + startOffset: 1, + endLine: 2, + endOffset: aTs.content.length, + refactor: "Move to file", + action: "Move to file", + interactiveRefactorArguments: { targetFile: "/Foo/b.ts" }, + } + }); + baselineTsserverLogs("refactors", "handles moving statement to an existing file", session); }); - baselineTsserverLogs("refactors", "handles moving statement to an existing file", session); + + it("handles moving statements to a non-TS file", () => { + const aTs: File = { + path: "/Foo/a.ts", + content: "const x = 0;" + }; + const bTxt: File = { + path: "/Foo/b.txt", + content: "" + }; + const tsconfig: File = { + path: "/Foo/tsconfig.json", + content: `{ "files": ["./a.ts"] }` + }; + const host = createServerHost([aTs, bTxt, tsconfig]); + const session = createSession(host, { logger: createLoggerWithInMemoryLogs(host) }); + openFilesForSession([aTs], session); + + session.executeCommandSeq({ + command: ts.server.protocol.CommandTypes.GetEditsForRefactor, + arguments: { + file: aTs.path, + startLine: 1, + startOffset: 1, + endLine: 2, + endOffset: aTs.content.length, + refactor: "Move to file", + action: "Move to file", + interactiveRefactorArguments: { targetFile: "/Foo/b.txt" }, + } + }); + baselineTsserverLogs("refactors", "handles moving statements to a non-TS file", session); }); }); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 870d881644902..511fbb4a27444 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -619,6 +619,7 @@ declare namespace ts { */ renameLocation?: Location; renameFilename?: string; + notApplicableReason?: string; } /** * Organize imports by: @@ -10480,6 +10481,7 @@ declare namespace ts { renameFilename?: string; renameLocation?: number; commands?: CodeActionCommand[]; + notApplicableReason?: string; } type RefactorTriggerReason = "implicit" | "invoked"; interface TextInsertion { diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 862560ba05a7d..84267bf7aecf7 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -6517,6 +6517,7 @@ declare namespace ts { renameFilename?: string; renameLocation?: number; commands?: CodeActionCommand[]; + notApplicableReason?: string; } type RefactorTriggerReason = "implicit" | "invoked"; interface TextInsertion { diff --git a/tests/baselines/reference/tsserver/refactors/handles-moving-statement-to-an-existing-file.js b/tests/baselines/reference/tsserver/refactors/handles-moving-statement-to-an-existing-file.js index 8f6e4a458ba4a..6be03ac67f229 100644 --- a/tests/baselines/reference/tsserver/refactors/handles-moving-statement-to-an-existing-file.js +++ b/tests/baselines/reference/tsserver/refactors/handles-moving-statement-to-an-existing-file.js @@ -6,7 +6,7 @@ const x = 0; //// [/Foo/b.ts] import {} from "./bar"; -const a = 1; + const a = 1; //// [/Foo/tsconfig.json] { "files": ["./a.ts", "./b.ts"] } @@ -41,7 +41,7 @@ Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /Foo/tsconfig.jso Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured) Info seq [hh:mm:ss:mss] Files (2) /Foo/a.ts SVC-1-0 "const x = 0;" - /Foo/b.ts Text-1 "import {} from \"./bar\";\nconst a = 1;" + /Foo/b.ts Text-1 "import {} from \"./bar\";\n const a = 1;" a.ts @@ -119,11 +119,11 @@ Info seq [hh:mm:ss:mss] response: { "start": { "line": 2, - "offset": 13 + "offset": 17 }, "end": { "line": 2, - "offset": 13 + "offset": 17 }, "newText": "\nconst x = 0;\n" } diff --git a/tests/baselines/reference/tsserver/refactors/handles-moving-statements-to-a-non-TS-file.js b/tests/baselines/reference/tsserver/refactors/handles-moving-statements-to-a-non-TS-file.js new file mode 100644 index 0000000000000..889d3de54b223 --- /dev/null +++ b/tests/baselines/reference/tsserver/refactors/handles-moving-statements-to-a-non-TS-file.js @@ -0,0 +1,96 @@ +currentDirectory:: / useCaseSensitiveFileNames: false +Info seq [hh:mm:ss:mss] Provided types map file "/a/lib/typesMap.json" doesn't exist +Before request +//// [/Foo/a.ts] +const x = 0; + +//// [/Foo/b.txt] + + +//// [/Foo/tsconfig.json] +{ "files": ["./a.ts"] } + + +Info seq [hh:mm:ss:mss] request: + { + "command": "open", + "arguments": { + "file": "/Foo/a.ts" + }, + "seq": 1, + "type": "request" + } +Info seq [hh:mm:ss:mss] Search path: /Foo +Info seq [hh:mm:ss:mss] For info: /Foo/a.ts :: Config file name: /Foo/tsconfig.json +Info seq [hh:mm:ss:mss] Creating configuration project /Foo/tsconfig.json +Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /Foo/tsconfig.json 2000 undefined Project: /Foo/tsconfig.json WatchType: Config file +Info seq [hh:mm:ss:mss] Config: /Foo/tsconfig.json : { + "rootNames": [ + "/Foo/a.ts" + ], + "options": { + "configFilePath": "/Foo/tsconfig.json" + } +} +Info seq [hh:mm:ss:mss] Starting updateGraphWorker: Project: /Foo/tsconfig.json +Info seq [hh:mm:ss:mss] FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 500 undefined Project: /Foo/tsconfig.json WatchType: Missing file +Info seq [hh:mm:ss:mss] Finishing updateGraphWorker: Project: /Foo/tsconfig.json Version: 1 structureChanged: true structureIsReused:: Not Elapsed:: *ms +Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured) +Info seq [hh:mm:ss:mss] Files (1) + /Foo/a.ts SVC-1-0 "const x = 0;" + + + a.ts + Part of 'files' list in tsconfig.json + +Info seq [hh:mm:ss:mss] ----------------------------------------------- +Info seq [hh:mm:ss:mss] Project '/Foo/tsconfig.json' (Configured) +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: /Foo/a.ts ProjectRootPath: undefined +Info seq [hh:mm:ss:mss] Projects: /Foo/tsconfig.json +Info seq [hh:mm:ss:mss] response: + { + "responseRequired": false + } +After request + +PolledWatches:: +/a/lib/lib.d.ts: *new* + {"pollingInterval":500} + +FsWatches:: +/foo/tsconfig.json: *new* + {} + +Before request + +Info seq [hh:mm:ss:mss] request: + { + "command": "getEditsForRefactor", + "arguments": { + "file": "/Foo/a.ts", + "startLine": 1, + "startOffset": 1, + "endLine": 2, + "endOffset": 12, + "refactor": "Move to file", + "action": "Move to file", + "interactiveRefactorArguments": { + "targetFile": "/Foo/b.txt" + } + }, + "seq": 2, + "type": "request" + } +Info seq [hh:mm:ss:mss] response: + { + "response": { + "edits": [], + "notApplicableReason": "Cannot move to file, selected file is invalid" + }, + "responseRequired": true + } +After request