Move definition to another module code action

[lpil]

It would be very useful to be able to move a function/type/etc to another module.

One problem is how do we determine what module to move the item to. It doesn't seem that the language server protocol supports refactorings that ask for additional information like this. What can we do here?

[dbond09]

One way to approach this is to interactively ask for the target module after dispatching the code action. Typescript language server solves this by relying on custom client implementation. This is outside the LSP standard and is not easily maintainable.

I would propose another approach that works similarly to a clipboard. The code action would be comprised of two stages detailed below. As far as I understand, this complies with the standard and, by some precursory experimentation, works fine in most clients.

First stage: "Mark `name` for moving"

This is a code action that is available when the cursor is over a movable function or type definition. Dispatching it does one thing which is storing the definition source location as marked for moving in the engine state. This is done by sending a workspace/executeCommand request with the source location in its arguments.

As an aid to suggest to the programmer that this is supposed to be a stateful two-stage code action, an info-level diagnostic could be set on the source definition saying e.g. "Function marked for moving".

Second stage: "Move `name` here"

This code action is available when there is a marked definition and the cursor is located over a feasible target location in a different module. At this point, all the necessary information is known to the language server, so the necessary changes can be computed in response to the usual textDocument/codeAction request including removing the definition from the source module, inserting it into the current module, updating the imports where necessary, qualifications etc.

If you deem this approach acceptable, I'd be glad to start implementing it.

[giacomocavalieri]

I think a different approach could also be nice, using the rename action. If someone renames a type/function/const/... by adding a module name before it the language server could move it there automatically. For example:

// In the wibble module
pub type Wibble
//       ^ [rename to: wobble.Wibble]
// would move the definition to the wobble module

[lpil]

Of the two approaches I think I prefer the rename one, though it would be nice to have support actually built in to LSP. I'm hoping this will come as TypeScript is going to start using LSP in future.

Probably would be worth continuing with one of these solutions until that happens though!

[dbond09]

I see some issues with the rename approach.

First, it feels kind of unintuitive that an action called "rename" moves code, and does so by setting the name to a syntactically invalid one (as pub type wobble.Wibble is invalid, even though it will never appear in the code, I view it as an intermediary state in the rename-move procedure). Also, in my interpretation, the standard only allows renaming the symbol, but please correct me if I'm wrong.

Second, in the example rename to: wobble.Wibble, to which module does wobble actually refer? It only makes sense when there is an import in the original module that makes the name wobble available either as an alias or the last part of an absolute module name:

import wabble/wobble

or

import wabble/wubble as wobble

Otherwise, it is ambiguous as several modules with the name wobble may exist in the project. To me, this seems like an overly strict constraint and this approach loses its straightforwardness if we have to tweak imports before actually being able to dispatch the move.

I just wanted to highlight these issues to take into consideration, but if you still prefer the rename approach, I'd be happy to implement it as well.

[lpil]

We wouldn't use aliases, we would use module names, for they are unambiguous.

Both proposals are extremely unintuitive. I would not expect anyone to be able to discover either, users would need to learn about either from the language server documentation.

[giacomocavalieri]

Since this issue is not immediately actionable I would turn it into a discussion