An operation for refactorings

[mickaelistria]

Out of the rename operation, I couldn't find an operation for refactorings. I would expect something like

path: /refactorings
params: TextDocumentIdentifier, Range
result: Refactoring[] with Refactoring={String title, WorkspaceEdit[]}

is there already an operation I missed, which is meant to support that?

[dbaeumer]

@mickaelistria no there isn't. We started a discussion about how a refactoring protocol could look like but we haven't found a good way to spec that yet since this is very dependent on the programming language you are using.

[dbaeumer]

Sorry didn't want to close the issue.

[mickaelistria]

Thanks for your answer. Can you please add some details about some corner-cases you've identified for whatever language or refactoring hat wouldn't fit the suggestion of a refactoring operation I made above?

[dbaeumer]

The problem we see so far are:

• parameter capturing. For rename we made this trivial by only supplying the new name. However Java for example allows on renaming a type to rename variable with that typeName in its identifier as well.
• communication on sematic shift warnings. Refactoring should be semantic preserving. But sometimes the semantic shift is wanted and we need to let the user confirm this.
• how do we handle undos Should the editor simply revert the textual changes or do we need to involve the server as well.

[mickaelistria]

parameter capturing. For rename we made this trivial by only supplying the new name. However Java for example allows on renaming a type to rename variable with that typeName in its identifier as well.

I was thinking about rendering parameter capturing with "edits" like the {{xxx}} in completion, assuming multiple {{xxx}} means that all occurences should be renamed simultaneously. xxx would be generated as best according to the context. Depending on the tool, those could be linked edits (like Eclipse rename) or be presented as a list of values to refine before applying the edit.

communication on sematic shift warnings. Refactoring should be semantic preserving. But sometimes the semantic shift is wanted and we need to let the user confirm this.

Ok. If this is the only case, then we could have a flag and message in the refactoring response. Tools can decide whether to honor it and ask for confirmation before applying change.

how do we handle undos Should the editor simply revert the textual changes or do we need to involve the server as well.

IMO, the undo is something I'd expect to be provided by the tool. The protocol doesn't have an undo for rename AFAIK, so I don't see why other refactorings should be treated differently.

[dbaeumer]

With parameter capturing I was referring to capturing input parameter for the refactoring. For rename for example rename in comments and strings. How would the server be able to describe these input parameters. And they might be different from languages if for example the language wouldn't support comments.

[mickaelistria]

If for example we use a convention of TextEdits with `{{param}}`` value for example, then the parameters are passed by server to client and it's up to clients to deal with those parameters.

[alanz]

To make it concrete, some of the refactorings for haskell require the following parameters to be specified by the user in the client and passed to the server as input to the refactoring

rename FILE NEWNAME line col
demote FILE line col
dupdef FILE NEWNAME line col
iftocase FILE startline startcol endline endcol

So, the renaming maps onto the spec, but for the others some need a Position, some need a Range, and in addition some need a new name.

But this mapping of required parameters and command to return to the server needs to be provided as a capability at server start up.

[dbaeumer]

@mickaelistria does the new executeCommand and applyEdit pair help you here?

[mickaelistria]

This seems to do most of the work properly, I just have a few questions (that may be worth answering in the spec): When the command is executed, assuming the command is a refactoring, is the state of the files on server-side modified? ie, If client ignores the applyEdit notification, has anything changed on server side? ie Does the command only compute the necessary change or does it apply and compute it?

[dbaeumer]

@mickaelistria good point. The current behavior on the VS Code client side is as follows:

• for all resources manipulated via a workspace edit we open them in an internal buffer.
• this fires an open event which will be forwarded to the server
• the client changes the file
• this triggers a change event sent to the server
• the client closes the file which again sends and event to the server

So the bottom line is that the server content should be updated without that the server replays the edits. If the server directly manipulates the files on disk (which he is allowed to do for files which are not open) then the server is responsible to update its internal buffers as well.

[mickaelistria]

I have the impression that the command framework is a bit overkill for some refactoring or codelens. In most case I've seen, those codelens and commands actually give a WorkspaceEdit as argument and expect the tool to apply it through a command. IMHO, everything that return a Command could also return a WorkspaceEdit and expect the tool to apply it - rather than assuming an "applyEdit"-like command is defined for each tool and language server.

[dbaeumer]

What if the computation of the workspace edit is expensive and for example code actions return more than one possible action. The having this resolve step does make sense. However for a single thing it might be more convenient to include the workspace edit in the command.

[kristijanhusak]

Any news on this?

[albertocavalcante]

Is this still being looked at?