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

Multiple code action kinds requests during code actions on save #2510

Open
rchl opened this issue Aug 20, 2024 · 3 comments
Open

Multiple code action kinds requests during code actions on save #2510

rchl opened this issue Aug 20, 2024 · 3 comments
Labels

Comments

@rchl
Copy link
Member

rchl commented Aug 20, 2024

Describe the bug

I've mentioned that (vscode interoperability) issue in some other places but I'm too lazy to find those now.

When asking servers for code actions during "code actions on save" task, we should create separate requests per code action kind and not include multiple code action kinds in a single request. Otherwise we might try to apply multiple text edits that are not compatible with each other (if server doesn't provide version for text edits).

@predragnikolic
Copy link
Member

predragnikolic commented Aug 20, 2024

I've mentioned that (vscode interoperability) issue in some other place but I'm too lazy to find those now.

here it is #2421 (comment)

some additional info on how to reproduce the issue is to add:

"lsp_code_actions_on_save":
    {
        "source.fixAll.eslint": true,
        "source.fixAll.ts": true,
        "source.organizeImports.ts": true,
        "source.addMissingImports.ts": true,
        "source.sortImports.ts": true,
    },

@jwortmann
Copy link
Member

jwortmann commented Aug 20, 2024

To be fair, I think this has nothing really to do with VSCode interoperability, but the current design used here is just flawed. The server responds with a list of code actions for the current version of the document, which is totally expected. As soon as one of the code action gets applied, in general the document will change and the workspace edits of all other code actions are therefore invalidated. The server does not know beforehand whether the code actions will be applied, and if so in what order.

It would be the same situation with the regular code actions for the current cursor position; if you would try to apply all actions (if there are multiple) at the same time, the resulting code would probably be broken as well.

I think in the LSP-ruff example from sublimelsp/LSP-ruff#72, the server not sending a version in the edits could be considered as a bug and a misinterpretation of the specs. As far as I understand, version should only be null if the document is not opened in the editor:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#optionalVersionedTextDocumentIdentifier

         /* If an optional versioned text document
	 * identifier is sent from the server to the client and the file is not
	 * open in the editor (the server has not received an open notification
	 * before) the server can send `null` to indicate that the version is
	 * known and the content on disk is the master
	 */

But yes, the way how multiple code actions on save are handled in LSP is wrong too and should be fixed. Currently in the best case the version is included in the edits, which means that only one of the specified code actions on save will be applied and the others are ignored due to mismatched version after the first change.

@rchl
Copy link
Member Author

rchl commented Aug 21, 2024

Yes, it's probably more a bug than interoperability issue but since vscode is a spec, maybe you can call it either. :)

@predragnikolic I think to reproduce the biome issue you need the config from the thread you've linked. The one you've pasted is related to recently discussed on Discord LSP-typescript issue but I'm not convinced that that is related to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants