Skip to content

fix(lsp): support TypeScript source action kinds#4496

Open
TorinAsakura wants to merge 3 commits into
microsoft:mainfrom
TorinAsakura:fix/lsp-specific-source-action-kinds
Open

fix(lsp): support TypeScript source action kinds#4496
TorinAsakura wants to merge 3 commits into
microsoft:mainfrom
TorinAsakura:fix/lsp-specific-source-action-kinds

Conversation

@TorinAsakura

Copy link
Copy Markdown
Contributor

Summary

  • Advertise TypeScript-specific source action kinds with the .ts suffix.
  • Keep the existing generic source action kinds for backward compatibility.
  • Accept TypeScript-specific only filters for organize imports and fix all code actions.
  • Return the requested TypeScript-specific kind when the client asks for it.

Fixes #3793.

Before

The server advertised only generic source action kinds:

quickfix
source.organizeImports
source.removeUnusedImports
source.sortImports
source.fixAll

A client could not target the TypeScript server specifically with a code actions on save entry such as source.removeUnusedImports.ts.

After

The server advertises both generic and TypeScript-specific source action kinds:

quickfix
source.organizeImports
source.organizeImports.ts
source.removeUnusedImports
source.removeUnusedImports.ts
source.sortImports
source.sortImports.ts
source.fixAll
source.fixAll.ts

Requests filtered to source.organizeImports.ts, source.removeUnusedImports.ts, source.sortImports.ts, or source.fixAll.ts now return matching TypeScript-specific code actions, while the existing generic kinds still work.

Validation

  • go test ./internal/fourslash/tests -run 'TestOrganizeImports_(sortModuleSpecifiersTsKind|coalesceImportsTsKind|removeUnusedTsKind)|TestSourceFixAllCodeActionTsKind'
  • go test ./internal/lsp ./internal/ls ./internal/fourslash/tests -run 'TestInitializeAdvertisesTypeScriptSourceActionKinds|TestGetOrganizeImportsActionsForTypeScriptKinds|TestIsFixAllKindAcceptsTypeScriptKind|TestSourceFixAllCodeAction|TestOrganizeImports_(sortModuleSpecifiersTsKind|coalesceImportsTsKind|removeUnused)'
  • go test ./internal/lsp ./internal/ls ./internal/fourslash/tests ./internal/lsp/lsproto

P.S. I did not find an existing microsoft/TypeScript precedent for these .ts source action kinds. This follows the LSP hierarchical kind model and keeps the generic kinds advertised and accepted for compatibility.

Copilot AI review requested due to automatic review settings June 30, 2026 21:27
@jakebailey

Copy link
Copy Markdown
Member

P.S. I did not find an existing microsoft/TypeScript precedent for these .ts source action kinds. This follows the LSP hierarchical kind model and keeps the generic kinds advertised and accepted for compatibility.

You should probably be looking at the vscode repo's typescript-language-features extension

That being said, if the old extensions could not do this, then is this critical to do?

@TorinAsakura

Copy link
Copy Markdown
Contributor Author

That being said, if the old extensions could not do this, then is this critical to do?

I checked vscode’s typescript-language-features too; same result, I don’t see any .ts-specific source action kinds there.

BTW, I re-read the issue, and I don’t think it’s rly about matching the current VS Code extension. It’s more about the multi-server case. With source.removeUnusedImports in codeActionsOnSave, the client is asking for a generic LSP source action, so any server attached to that doc can match it. source.removeUnusedImports.ts gives clients a TS-specific knob, while source.removeUnusedImports stays around for existing setups.

That’s why I wired the actual only handling too, not just the initialize list. Otherwise we’d advertise a kind that doesn’t do anything.

So yeah, it’s not something VS Code seems to need today. The PR is more about making that escape hatch available for clients that do need to split source actions between several servers.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread internal/lsp/server_capabilities_test.go
CodeActionKindSourceRemoveUnusedImportsTs CodeActionKind = "source.removeUnusedImports.ts"
CodeActionKindSourceSortImports CodeActionKind = "source.sortImports"
CodeActionKindSourceSortImportsTs CodeActionKind = "source.sortImports.ts"
CodeActionKindSourceFixAllTs CodeActionKind = "source.fixAll.ts"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also support .js for all these if we support .ts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably it could be js/ts to match all of our existing setting names, but, not sure what's standard in LSP

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, fair. The intent here isn’t file-extension targeting, it’s TS server targeting, so clients can separate tsgo from other LSPs.

.ts is ambiguous for that since this server handles JS/JSX/TSX too. I didn’t add .js aliases because then we still have the TSX/JSX question, and the surface gets weird fast.

Maybe the better shape is a broader suffix. js/ts matches the existing setting names, but I’m also not sure it’s a great LSP kind segment. Happy to switch this PR to whatever naming you think fits the server boundary best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[lsp] make source action kinds more specific

4 participants