Add basic support for move function refactoring

[jmbockhorst]

Part of #194. It adds basic support for a new code action to move a function to a new file. It is a fair amount of changes so I wanted to get your feedback to make sure it was a good direction.

Because this type of refactoring requires client support (quick pick for the destination file),
I added a new extendedCapabilites property to the client settings that the client can initialize. Right now the only property is moveFunctionRefactoringSupport, but more can easily be added.

I also added a protocol.ts that defines the custom requests between the server and client.

Currently, in vscode, you can move a function to a new file and it adds new imports to all files with references. Dependent on elm-tooling/elm-language-client-vscode#88.

[razzeee]

Approach looks okay to me, it's unfortunate, that it needs client support, but that's how it is with language server at the moment. Until this moves further microsoft/language-server-protocol#764

Can you please install and use prettier via https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
You seem to be using tabs and some other styling that is inconsistent to our prettier rules.

[jmbockhorst]

The core functionality is complete. Moving a function properly updates the module exposing values for the old and new files, and removes and adds the proper imports for all references. I tested this for a while and it seemed to work pretty well, but there is sure to be an edge case that could break it.

[razzeee]

1. I'm not convinced, auto exposing it in every file is the way to go, especially if it was never used like this.

2. If it has been fully referenced e.g. Feed.viewPagination doesn't get changed to xxx.viewPagination.

3. Doc comments doesn't seem to be moved together with the function.

I think we should at least fix 1 and 3 before we merge this.

[jmbockhorst]

What do you mean by 1? Auto exposing the moved function in the destination file? Currently it only gets exposed if there is an external reference that needs it.

[razzeee]

Kinda correct, it correlates with 2, as you don't seem to differentiate between fully qualified (Feed.viewPagination) and exposed (viewPagination)

I tested it with the elm-spa-example https://github.com/rtfeldman/elm-spa-example/blob/master/src/Article/Feed.elm#L172

[jmbockhorst]

We also need to update references or add imports for any functions or types used within the moved function.

[razzeee]

Yes, would be nice, but I think it would already be useful without, as Elm will just complain to you about it :)

[jmbockhorst]

So to be clear, if a file only has fully qualified references to the function (xxx.function), we don't need to expose it (just import). Only expose it if we need it, when there is an exposed reference to it in the file (function).

[razzeee]

Yes, that's my understanding

…

On Mon, 23 Mar 2020, 00:46 Jon Bockhorst, ***@***.***> wrote: So to be clear, if a file only has fully qualified references to the function (xxx.function), we don't need to expose it (just import). Only expose it if we need it, when there is an exposed reference to it in the file (function). — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#226 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABNLEZDYIC66FXTLZ5CC5ALRI2PMVANCNFSM4LQB74AA> .

[jmbockhorst]

It now properly handles fully qualified references:

• Don't expose the import if all uses are fully qualified
• Change the module name of all fully qualified references

[razzeee]

So I ended up with this as a module declaration :)
module Session exposing (Session, changes, cred, fromViewer, navKey, viewer), viewPagination

[razzeee]

So I ended up with this as a module declaration :)
module Session exposing (Session, changes, cred, fromViewer, navKey, viewer), viewPagination

Not able to do that again, but every time I use the declaration (not the type declaration) to initalize the move on viewPagination to the Session module I end up with module Session exposing (Session, changes, cred, fromViewer, navKey, viewer, viewPagination toMsg page (Model feed))

[jmbockhorst]

Thanks, I think I fixed it.

[razzeee]

=== please

[razzeee]

Merging this now, will address some of the smaller stuff in one of the next commits.

Thank you so much! 🌈