Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lsp): unify code actions capabilities #4122

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

vitallium
Copy link
Contributor

@vitallium vitallium commented Sep 29, 2024

Summary

The biome language server sends to a client
specific code actions that the client can use later to send code actions:

  • quickfix.biome
  • source.fixAll.biome
  • source.organizeImports.biome

But under the hood the language server accepts
not announced code actions such as quickfix.suppressRule or quickfix.suppressRule.biome.suspicious.noDoubleEquals. Such code actions work in clients that do not rely on code actions capabilities sent before, VSCode for example. The LSP specification (^1) specifies that clients should not send unsupported requests (code actions).

This pull request unified code actions and their announcements by applying the following changes:

  1. Custom sub-categories like, e.g. quickfix.*.suspicious.noDoubleEquals are removed because they are dynamic and announcing them could be hard to maintain. Instead only the base category is used: quickfix.suppressRule.biome.suspicious.noDoubleEquals becomes quickfix.suppressRule.biome. The code action data already has all required information to handle it on the server side.
  2. Adds additional code actions to the list of code_action_kinds capabilities.

References:

  1. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind

Issue: #4116

Additional information

I am unsure about the compatibility policy for biomejs as this pull request seems to introduce breaking changes. I would appreciate your thoughts on the implementation itself and how we can maintain backward compatibility. Thanks!

Test Plan

  1. Install the Zed editor and the Biome extension
    Create a simple Typescript file with the following content:
let hello;

You should see the reported linting error from Biome LSP, but if you click on the available code actions, the quickfix.suppressRule option will be missing.

@github-actions github-actions bot added A-Linter Area: linter A-LSP Area: language server protocol labels Sep 29, 2024
Comment on lines +68 to +71
CodeActionKind::from("refactor.biome"),
CodeActionKind::from("refactor.extract.biome"),
CodeActionKind::from("refactor.inline.biome"),
CodeActionKind::from("refactor.rewrite.biome"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably these code actions are not available at all? Happy to remove them in this case.

@vitallium vitallium marked this pull request as ready for review September 29, 2024 18:34
@dyc3 dyc3 requested review from a team September 29, 2024 18:38
Copy link

codspeed-hq bot commented Sep 29, 2024

CodSpeed Performance Report

Merging #4122 will not alter performance

Comparing vitallium:vs/unify-lsp-code-actions (ab7a237) with main (011a709)

Summary

✅ 97 untouched benchmarks

@ematipico
Copy link
Member

The quickfix.suppression.biome isn't officially supported, so I wonder if it makes sense to change it to quickfix.biome.suppression

@vitallium
Copy link
Contributor Author

The quickfix.suppression.biome isn't officially supported, so I wonder if it makes sense to change it to quickfix.biome.suppression

According to LSP spec the code action kinds use a hierarchy like quickfix.fixAll.eslint, separated by a dot .. But wouldn't changing the name be a breaking change? Clients could rely on the name of this action.

@ematipico
Copy link
Member

Thank you. Could you please add a new line to the changelog? After that, we can merge it and release a patch

The `biome` language server sends to a client
specific code actions that the client can use later
to send code actions:

- `quickfix.biome`
- `source.fixAll.biome`
- `source.organizeImports.biome`

But under the hood the language server accepts
not announced code actions such as `quickfix.suppressRule`
or `quickfix.suppressRule.biome.suspicious.noDoubleEquals`.
Such code actions work in clients that do not rely on
code actions capabilities sent before, VSCode for example.
The LSP specification (^1) specifies that clients should
not send unsupported requests (code actions).

This pull request unified code actions and their announcements
by applying the following changes:
1. Custom sub-categories like, e.g.
	 `quickfix.*.suspicious.noDoubleEquals` are removed because they are
	 dynamic and announcing them could be hard to maintain. Instead only
	 the base category is used:
	 `quickfix.suppressRule.biome.suspicious.noDoubleEquals` becomes
	 `quickfix.suppressRule.biome`. The code action data already has all
	 required information to handle it on the server side.
2. Adds additional code actions to the list of `code_action_kinds`
	 capabilities.

References:
1. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind
@vitallium vitallium force-pushed the vs/unify-lsp-code-actions branch from ab7a237 to 20848f7 Compare October 17, 2024 13:16
@github-actions github-actions bot added the A-Changelog Area: changelog label Oct 17, 2024
@vitallium
Copy link
Contributor Author

Thank you. Could you please add a new line to the changelog? After that, we can merge it and release a patch

I added it under the Editors category. Please let me know if this is incorrect.

@ematipico
Copy link
Member

Yes that's the correct section :) thank you @vitallium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Changelog Area: changelog A-Linter Area: linter A-LSP Area: language server protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants