From a0acda603fad64d70591e2041edecb9abe6a665f Mon Sep 17 00:00:00 2001 From: Rafal Chlodnicki Date: Mon, 15 Sep 2025 11:16:20 +0200 Subject: [PATCH 1/2] fix: code actions handling - Sync code actions handling with upstream - make returned code action "kind" values more specific. - Fix matching of client-provided `only` values. It only worked if client provided for example `only: ["refactor"]` but not if client provided `only: ["refactor.move"]`. - Make separate code actions request to TS for each "only" kind that the client has passed. Previously we'd only make request for the first one. - Add test for "Move to file" interactive code action. --- src/lsp-server.test.ts | 167 ++++++++++++++++++++++++++++++++++------- src/lsp-server.ts | 31 +++++--- src/refactor.ts | 68 ++++++++++++----- 3 files changed, 206 insertions(+), 60 deletions(-) diff --git a/src/lsp-server.test.ts b/src/lsp-server.test.ts index ce7caecc..f68844a2 100644 --- a/src/lsp-server.test.ts +++ b/src/lsp-server.test.ts @@ -1354,7 +1354,7 @@ describe('code actions', () => { }, kind: 'quickfix', }); - const refactorAction = result.find(diagnostic => diagnostic.kind === 'refactor'); + const refactorAction = result.find(diagnostic => diagnostic.kind === 'refactor.rewrite.parameters.toDestructured'); expect(refactorAction).toBeDefined(); expect(refactorAction).toMatchObject({ title: 'Convert parameters to destructured object', @@ -1373,9 +1373,9 @@ describe('code actions', () => { }, ], }, - kind: 'refactor', + kind: 'refactor.rewrite.parameters.toDestructured', }); - const refactorMoveAction = result.find(diagnostic => diagnostic.kind === 'refactor.move'); + const refactorMoveAction = result.find(diagnostic => diagnostic.kind === 'refactor.move.newFile'); expect(refactorMoveAction).toBeDefined(); expect(refactorMoveAction).toMatchObject({ title: 'Move to a new file', @@ -1394,7 +1394,7 @@ describe('code actions', () => { }, ], }, - kind: 'refactor.move', + kind: 'refactor.move.newFile', }); }); @@ -1415,30 +1415,11 @@ describe('code actions', () => { code: 6133, message: 'unused arg', }], - only: ['refactor', 'invalid-action'], + only: ['refactor.rewrite', 'invalid-action'], }, }); expect(result).toMatchObject([ - { - title: 'Move to a new file', - kind: 'refactor.move', - command: { - title: 'Move to a new file', - command: '_typescript.applyRefactoring', - arguments: [ - { - file: filePath('bar.ts'), - startLine: 2, - startOffset: 26, - endLine: 2, - endOffset: 50, - refactor: 'Move to a new file', - action: 'Move to a new file', - }, - ], - }, - }, { command: { arguments: [ @@ -1455,7 +1436,7 @@ describe('code actions', () => { command: '_typescript.applyRefactoring', title: 'Convert parameters to destructured object', }, - kind: 'refactor', + kind: 'refactor.rewrite.parameters.toDestructured', title: 'Convert parameters to destructured object', }, ]); @@ -1777,6 +1758,136 @@ accessSync('t');`, }); }); +describe('code actions (interactive)', () => { + const MOVE_TO_FILE_TARGET_FILENAME = 'moveFileTarget.ts'; + let localServer: TestLspServer; + + beforeAll(async () => { + localServer = await createServer({ + rootUri: uri(), + publishDiagnostics: () => {}, + initializationOptionsOverrides: { + supportsMoveToFileCodeAction: true, + }, + }); + }); + + beforeEach(() => { + localServer.closeAllForTesting(); + localServer.workspaceEdits = []; + }); + + afterAll(async () => { + localServer.closeAllForTesting(); + localServer.shutdown(); + await fs.unlink(filePath(MOVE_TO_FILE_TARGET_FILENAME)).catch(() => {}); + }); + + const doc = { + uri: uri('bar.ts'), + languageId: 'typescript', + version: 1, + text: 'export function foo(bar: string, baz?:boolean): void {}', + }; + + it('provides "Move to file" code action', async () => { + await openDocumentAndWaitForDiagnostics(localServer, doc); + const actions = await localServer.codeAction({ + textDocument: doc, + range: { + start: { line: 0, character: 1 }, + end: { line: 0, character: 1 }, + }, + context: { + diagnostics: [], + }, + }); + + const moveToFileAction = actions.find(action => action.title === 'Move to file'); + expect(moveToFileAction).toBeDefined(); + expect(moveToFileAction).toMatchObject({ + title: 'Move to file', + kind: 'refactor.move.file', + command: { + title: 'Move to file', + command: '_typescript.applyRefactoring', + arguments: [ + { + file: '/usr/local/workspace/github/typescript-language-server/test-data/bar.ts', + startLine: 1, + startOffset: 2, + endLine: 1, + endOffset: 2, + refactor: 'Move to file', + action: 'Move to file', + }, + ], + }, + }); + + // The new file needs to exist physically and be opened by tsserver for the move command to work. + const moveFileTargetPath = filePath(MOVE_TO_FILE_TARGET_FILENAME); + await fs.writeFile(moveFileTargetPath, ''); + const moveFileTargetDoc = { + uri: uri(MOVE_TO_FILE_TARGET_FILENAME), + languageId: 'typescript', + version: 1, + text: '', + }; + await openDocumentAndWaitForDiagnostics(localServer, moveFileTargetDoc); + + const moveCommand: lsp.ExecuteCommandParams = { + command: moveToFileAction!.command!.command, + arguments: [ + { + ...moveToFileAction!.command!.arguments![0], + interactiveRefactorArguments: { + targetFile: moveFileTargetPath, + }, + }, + ], + }; + await localServer.executeCommand(moveCommand); + + // Verify the received workspace edits. + expect(localServer.workspaceEdits).toHaveLength(1); + const { changes } = localServer.workspaceEdits[0].edit; + expect(changes).toBeDefined(); + expect(changes).toMatchObject({ + [uri('bar.ts')]: [ + { + range: { + start: { + line: 0, + character: 0, + }, + end: { + line: 0, + character: 55, + }, + }, + newText: '', + }, + ], + [uri(MOVE_TO_FILE_TARGET_FILENAME)]: [ + { + range: { + start: { + line: 0, + character: 1, + }, + end: { + line: 0, + character: 1, + }, + }, + newText: '\nexport function foo(bar: string, baz?: boolean): void { }\n', + }, + ], + }); + }); +}); + describe('executeCommand', () => { it('apply refactoring (move to new file)', async () => { const fooUri = uri('foo.ts'); @@ -1858,7 +1969,7 @@ describe('executeCommand', () => { text: 'export function fn(): void {}\nexport function newFn(): void {}', }; await openDocumentAndWaitForDiagnostics(server, doc); - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const result = await server.executeCommand({ command: Commands.TS_SERVER_REQUEST, arguments: [ @@ -1874,9 +1985,9 @@ describe('executeCommand', () => { lowPriority: true, }, ], - }); + }) as {body: {configFileName: string;};}; expect(result).toBeDefined(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + expect(result.body).toMatchObject({ // tsserver returns non-native path separators on Windows. configFileName: filePath('tsconfig.json').replace(/\\/g, '/'), diff --git a/src/lsp-server.ts b/src/lsp-server.ts index 0a7e2573..6e3b58a5 100644 --- a/src/lsp-server.ts +++ b/src/lsp-server.ts @@ -794,10 +794,10 @@ export class LspServer { const fileRangeArgs = Range.toFileRangeRequestArgs(document.filepath, params.range); const actions: lsp.CodeAction[] = []; const kinds = params.context.only?.map(kind => new CodeActionKind(kind)); - if (!kinds || kinds.some(kind => kind.contains(CodeActionKind.QuickFix))) { + if (!kinds || kinds.some(kind => CodeActionKind.QuickFix.contains(kind))) { actions.push(...provideQuickFix(await this.getCodeFixes(fileRangeArgs, params.context, token), this.tsClient)); } - if (!kinds || kinds.some(kind => kind.contains(CodeActionKind.Refactor))) { + if (!kinds || kinds.some(kind => CodeActionKind.Refactor.contains(kind))) { actions.push(...provideRefactors(await this.getRefactors(fileRangeArgs, params.context, this.features, token), fileRangeArgs, this.features)); } @@ -852,18 +852,25 @@ export class LspServer { const response = await this.tsClient.execute(CommandTypes.GetCodeFixes, args, token); return response.type === 'response' ? response : undefined; } - protected async getRefactors(fileRangeArgs: ts.server.protocol.FileRangeRequestArgs, context: lsp.CodeActionContext, features: SupportedFeatures, token?: lsp.CancellationToken): Promise { - const args: ts.server.protocol.GetApplicableRefactorsRequestArgs = { - ...fileRangeArgs, - triggerReason: context.triggerKind === lsp.CodeActionTriggerKind.Invoked ? 'invoked' : undefined, - kind: context.only?.length === 1 ? context.only[0] : undefined, - includeInteractiveActions: features.moveToFileCodeActionSupport, - }; - const response = await this.tsClient.execute(CommandTypes.GetApplicableRefactors, args, token); - return response.type === 'response' ? response : undefined; + protected async getRefactors(fileRangeArgs: ts.server.protocol.FileRangeRequestArgs, context: lsp.CodeActionContext, features: SupportedFeatures, token?: lsp.CancellationToken): Promise { + // Make separate request for each "kind" that was specified or a single request otherwise. + const kinds = context.only || [undefined]; + + const responses = await Promise.all(kinds.map(async (kind) => { + const args: ts.server.protocol.GetApplicableRefactorsRequestArgs = { + ...fileRangeArgs, + triggerReason: context.triggerKind === lsp.CodeActionTriggerKind.Invoked ? 'invoked' : undefined, + kind, + includeInteractiveActions: features.moveToFileCodeActionSupport, + }; + const response = await this.tsClient.execute(CommandTypes.GetApplicableRefactors, args, token); + return response.type === 'response' && response.body ? response.body : []; + })); + + return responses.flat(); } - async executeCommand(params: lsp.ExecuteCommandParams, token?: lsp.CancellationToken, workDoneProgress?: lsp.WorkDoneProgressReporter): Promise { + async executeCommand(params: lsp.ExecuteCommandParams, token?: lsp.CancellationToken, workDoneProgress?: lsp.WorkDoneProgressReporter): Promise { if (params.command === Commands.APPLY_WORKSPACE_EDIT && params.arguments) { const edit = params.arguments[0] as lsp.WorkspaceEdit; await this.options.lspClient.applyWorkspaceEdit({ edit }); diff --git a/src/refactor.ts b/src/refactor.ts index 9c0aaaff..05ba47bb 100644 --- a/src/refactor.ts +++ b/src/refactor.ts @@ -9,16 +9,18 @@ import * as lsp from 'vscode-languageserver'; import { Commands } from './commands.js'; import type { ts, SupportedFeatures } from './ts-protocol.js'; -export function provideRefactors(response: ts.server.protocol.GetApplicableRefactorsResponse | undefined, args: ts.server.protocol.FileRangeRequestArgs, features: SupportedFeatures): lsp.CodeAction[] { - if (!response?.body) { - return []; - } +// Defining locally until new version of vscode-languageserver is reladed. +namespace CodeActionKind { + export const RefactorMove = 'refactor.move'; +} + +export function provideRefactors(refactors: ts.server.protocol.ApplicableRefactorInfo[], args: ts.server.protocol.FileRangeRequestArgs, features: SupportedFeatures): lsp.CodeAction[] { const actions: lsp.CodeAction[] = []; - for (const info of response.body) { - if (info.inlineable === false) { - actions.push(asSelectRefactoring(info, args)); + for (const refactor of refactors) { + if (refactor.inlineable === false) { + actions.push(asSelectRefactoring(refactor, args)); } else { - const relevantActions = info.actions.filter(action => { + const relevantActions = refactor.actions.filter(action => { if (action.notApplicableReason && !features.codeActionDisabledSupport) { return false; } @@ -28,23 +30,23 @@ export function provideRefactors(response: ts.server.protocol.GetApplicableRefac return true; }); for (const action of relevantActions) { - actions.push(asApplyRefactoring(action, info, args)); + actions.push(asApplyRefactoring(action, refactor, args)); } } } return actions; } -export function asSelectRefactoring(info: ts.server.protocol.ApplicableRefactorInfo, args: ts.server.protocol.FileRangeRequestArgs): lsp.CodeAction { +export function asSelectRefactoring(refactor: ts.server.protocol.ApplicableRefactorInfo, args: ts.server.protocol.FileRangeRequestArgs): lsp.CodeAction { return lsp.CodeAction.create( - info.description, - lsp.Command.create(info.description, Commands.SELECT_REFACTORING, info, args), + refactor.description, + lsp.Command.create(refactor.description, Commands.SELECT_REFACTORING, refactor, args), lsp.CodeActionKind.Refactor, ); } -export function asApplyRefactoring(action: ts.server.protocol.RefactorActionInfo, info: ts.server.protocol.ApplicableRefactorInfo, args: ts.server.protocol.FileRangeRequestArgs): lsp.CodeAction { - const codeAction = lsp.CodeAction.create(action.description, asKind(info)); +export function asApplyRefactoring(action: ts.server.protocol.RefactorActionInfo, refactor: ts.server.protocol.ApplicableRefactorInfo, args: ts.server.protocol.FileRangeRequestArgs): lsp.CodeAction { + const codeAction = lsp.CodeAction.create(action.description, asKind(action)); if (action.notApplicableReason) { codeAction.disabled = { reason: action.notApplicableReason }; } else { @@ -53,7 +55,7 @@ export function asApplyRefactoring(action: ts.server.protocol.RefactorActionInfo Commands.APPLY_REFACTORING, { ...args, - refactor: info.name, + refactor: refactor.name, action: action.name, }, ); @@ -61,13 +63,39 @@ export function asApplyRefactoring(action: ts.server.protocol.RefactorActionInfo return codeAction; } -function asKind(refactor: ts.server.protocol.RefactorActionInfo): lsp.CodeActionKind { - if (refactor.name.startsWith('function_')) { +function asKind(action: ts.server.protocol.RefactorActionInfo): lsp.CodeActionKind { + if (action.kind) { + return action.kind; + } + if (action.name.startsWith('function_')) { return `${lsp.CodeActionKind.RefactorExtract}.function`; - } else if (refactor.name.startsWith('constant_')) { + } + if (action.name.startsWith('constant_')) { return `${lsp.CodeActionKind.RefactorExtract}.constant`; - } else if (refactor.name.startsWith('Move')) { - return `${lsp.CodeActionKind.Refactor}.move`; + } + if (action.name.startsWith('Extract to type alias')) { + return `${lsp.CodeActionKind.RefactorExtract}.type`; + } + if (action.name.startsWith('Extract to interface')) { + return `${lsp.CodeActionKind.RefactorExtract}.interface`; + } + if (action.name.startsWith('Move to file')) { + return `${CodeActionKind.RefactorMove}.file`; + } + if (action.name.startsWith('Move to a new file')) { + return `${CodeActionKind.RefactorMove}.newFile`; + } + if (action.name.startsWith('Convert namespace import') || action.name.startsWith('Convert named imports')) { + return `${lsp.CodeActionKind.RefactorRewrite}.import`; + } + if (action.name.startsWith('Convert default export') || action.name.startsWith('Convert named export')) { + return `${lsp.CodeActionKind.RefactorRewrite}.export`; + } + if (action.name.startsWith('Convert parameters to destructured object')) { + return `${lsp.CodeActionKind.RefactorRewrite}.parameters.toDestructured`; + } + if (action.name.startsWith('Generate \'get\' and \'set\' accessors')) { + return `${lsp.CodeActionKind.RefactorRewrite}.property.generateAccessors`; } return lsp.CodeActionKind.Refactor; } From 946a3683929a8caf62c41c7eadbc5299d0f285a0 Mon Sep 17 00:00:00 2001 From: Rafal Chlodnicki Date: Mon, 15 Sep 2025 12:25:49 +0200 Subject: [PATCH 2/2] fix test --- src/lsp-server.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lsp-server.test.ts b/src/lsp-server.test.ts index f68844a2..242cc798 100644 --- a/src/lsp-server.test.ts +++ b/src/lsp-server.test.ts @@ -1813,7 +1813,7 @@ describe('code actions (interactive)', () => { command: '_typescript.applyRefactoring', arguments: [ { - file: '/usr/local/workspace/github/typescript-language-server/test-data/bar.ts', + file: filePath('bar.ts'), startLine: 1, startOffset: 2, endLine: 1,