Language-server-protocol: Support for 'codeAction/resolve'

[angelozerr]

Today textDocument/codeAction manages 2 things:

1) the position where code action can be executed
2) the execution of code action which computes the text edits.

I find it's shame to compute text edits 2) each time editor text changed although user could not execute it. More computing of text edits could be cost with some languages.

A proposal is to provide a new service called codeAction/resolve like LSP provides with codeLens/resolve and completionItem/resolve

In other words:

• textDocument/codeAction creates code actions with a given position and text edits (resolved) or not (not resolved)
• in case of code action is not resolved, codeAction/resolve is called to compute the text edits.

What do you think about this idea?

[matklad]

Big :+1: from me. Note that currently it is possible to implement this yourself, by making a code action to execute a custom command on the server, which then sends workspace edit to the client. This feels overly complicated (each server needs its own custom command), and also makes it difficult for the client to correlate original code-action and subsequent edit.

[angelozerr]

@fbricon @gorkem @snjeza @maxandersen I think this feature could b ereally interesting for vscode-java to improve performance with JDT LS to compute code action only when user requests the quick fix.

[kjeremy]

@angelozerr in my current thinking a CodeAction would be considered unresolved if both edit and command are undefined.

@dbaeumer Reading the current spec it says that one or both of those fields have to be set. What happens if they aren't?

[rchl]

What would be the difference between code action with command and unresolved? Technically a command is like unresolved code action as it doesn't provide any information apart from command name that has to be sent to the server.

[kjeremy]

@rchl In the unresolved case edit and command would both be empty in the response.

[rchl]

So this proposal is about having code actions that still have the position information but not the edit or command until they are resolved.

I wonder what's the specific use case for this since to know whether code action is available for a specific position, the server most likely has to do the heavy lifting of validating the code anyway.

If there is an error (diagnostic) in the code, it would be confusing (from the editor's UI standpoint) to report code action for it if there is no actual action associated with it (and it wouldn't be known if there is until resolving the code action).

Can you mention a very specific case where this solution would actually improve the performance of the server?

[matklad]

Can you mention a very specific case where this solution would actually improve the performance of the server?

Computing edits is usually more costly than validation, because you need to manipulate (and not just read) syntax tree. So my gut feeling is that even constant factor savings here are significant. Not important for, eg, latency, but probably worth it to improve power usage.

The case where savings are of O(N) -> O(1) type is when the assists trigger some workspace-wide refactoring. Something like "replace constructor with factory method" where the applicability is determined locally and syntactically, but computing the edit triggers "find usages", which is a costly operation.

See https://github.com/rust-analyzer/rust-analyzer/pull/4576 for a specific example in rust-analyzer.

[dbaeumer]

The 3.16.0 spec got updated and an implementation is provided for VS Code.

[matklad]

@dbaeumer I think, as written, the spec isn't really usable. There's no way for the server to figure out which specific CodeAction it needs to resolve.

That is, for CompletionItems, there's a data field which a server can fill with some custom token to identify the specific completion item in the /resolve request.

There's no analogous field for CodeAction.

[dbaeumer]

Good point. Will look into it.

[dbaeumer]

Reopeing...

[dbaeumer]

Done in spec and implemented.

[kjeremy]

Since resolve and data support are both in the proposed 3.16 version and data is only used for resolve can't we simplify things and imply that data is preserved and remove the dataSupport flag?

[dbaeumer]

I first did this but we usually don't mangle properties into other settings. That is why I added it. It is pure consistency.

[rwols]

Sorry, but isn’t this already possible by doing the following:

1) reply to textDocument/codeAction with a list of Commands
2) each command, let’s call it resolveEdits, has as parameter a unique ID (the client passes this along back to the server)
3) the server checks the ID, computes edits, and then requests workspace/applyEdit
4) client applies lazily computed edits

I believe clangd already does it this way.

[dbaeumer]

@rwols yes, but it has the disadvantage that a client can't render a preview for a code action (e.g. the change it will make to the workspace). With the new approach this is possible without computing all edits.

[rwols]

A valid point. Thanks for clarifying.

[testforstephen]

The 3.16.0 spec got updated and an implementation is provided for VS Code.

@dbaeumer Which version of LSP client supports this in VS Code?

[dbaeumer]

Need to publish a new next version. Will do this week.