Goto super method in hierarchy of inheritance

[kpbochenek]

Fixes #1416

Goto super method + lenses

Preview of how it looks like in VSCode:

During requesting lenses metals will also calculate if any definition of a method in requested file overrides another method, if yes it will attach lens to this position to be able to go to super method. Lenses also link to methods in external dependencies and are displayed in files from libraries.

They are configurable from UserConfiguration, example from VSCode:

If they are found to be disturbing your workflow they can be easily disabled(they are enabled by default). Goto Super Method lenses should be quick to calculate but if you notice significant slowdown you can also disable them(when disabled they are not only not displayed but also not calculated at all)

There is also exposed server command: goto-super-method that can be mapped and used through shortcut. As example from VSCode PR (in client it is metals.go-to-super-method):

Goto Super Method in hierarchy

Preview how it looks like in VSCode:

Metals server exposes command super-method-hierarchy that can be invoked on a method definition. It calculated super method hierarchy and if more than one position is found it displays quickPick window (same as from new scala file) with super methods and when user selects one entry it jumps to that definition.

As example from VSCode it can be bind to a shortcut this way:

Internal

At core of those changes is implementation of a function to perform linearization of any provided class. It is used in this PR to find first next super method or list super methods according to linearization rules but it can also be used in a future for another features.

Other changes:

• a few typos fixed
• tests for both functionalities
• related changes in VSCode-metals (Goto Super Method functionality metals-vscode#233)

[tgodzik]

Thanks for the amazing work! I have some comments, but mostly around making the code more maintainable.

[tgodzik]

I think it's almost ready! I left a couple of comments and will try to finish the review tomorrow

[olafurpg]

Thank you for this contribution! A few minor comments on style, the only blocking comments are to remove usage of asInstanceOf

[olafurpg]

This is super impressive work @kpbochenek I agree the configuration option to enable/disable the code lenses is merited in this case, even if we try to avoid user-facing configuration options. I'm really excited to try this out 😄

[ckipp01]

First of all, this is awesome :)

Is it possible to completely separate this from being dependent on having debuggingProvider set to true? I was trying to get this to work with coc-metals which currently doesn't have debuggingProvider set to true, which with these new cases is causing the findLens to result in nothing. I couldn't figure out why this was happening for an embarrassingly long time before it clicked and I turned the value to true, which immoderately made the code lenses appear. Since various clients might not provide debugger support, I think it's important to be able to offer this feature regardless of that setting.

[kpbochenek]

Updated and commented on all requests.

There should probably still be some documentation added for users but I would like to firstly get this one merged, then give it a moment to settle and verify usability is okay, then update docs in another PR.

[kpbochenek]

@ckipp01 do you think you could try test command "metals.super-method-hierarchy" how it behaves/looks in vim?
I tried it for VSCode and what I needed to do was to add a command that can be bound to a shortcut which will call metals with:
name of command: "metals.super-method-hierarchy"
arguments:

            {
              document: <current document uri>,  (string)
              position: <curson position>  (org.eclipse.lsp4j.Position(line: Int, character: Int)
            }

If you are on overridden method it should show you popup(the same as for NewScalaFile) with list of super methods you want to go (it is good to test if there is C.x <: B.x <: A.x, call it on C.x and see a list: [B.x, A.x])

I assumed you might be familiar with vim <-> metals (coc-vim?) stuff but if I am wrong do you know who I could talk to about this?

[ckipp01]

@ckipp01 do you think you could try test command "metals.super-method-hierarchy" how it behaves/looks in vim?
I tried it for VSCode and what I needed to do was to add a command that can be bound to a shortcut which will call metals with:
name of command: "metals.super-method-hierarchy"
arguments:

            {
              document: <current document uri>,  (string)
              position: <curson position>  (org.eclipse.lsp4j.Position(line: Int, character: Int)
            }

If you are on overridden method it should show you popup(the same as for NewScalaFile) with list of super methods you want to go (it is good to test if there is C.x <: B.x <: A.x, call it on C.x and see a list: [B.x, A.x])

I assumed you might be familiar with vim <-> metals (coc-vim?) stuff but if I am wrong do you know who I could talk to about this?

Sure. The implementation ends up being a tad different since the coc.nvim api doesn't have registerTextEditorCommand, but just registerCommand. However, the situation you outlined looks like this:

I'll probably also just make a shortcut to easily be able to trigger it instead of using the command pallete.

[kpbochenek]

@ckipp01 thanks for checking 👍 if it works with 2 editors it should work with all :D

[tgodzik]

Looks good, let's have it merged after the release. I would feel better if we let it sit in the SNAPSHOT for a while, if no one objects?

Thanks @kpbochenek for the amazing work!

[olafurpg]

@tgodzik I think it's OK to merge now and include it in the release notes. It's a very impressive feature!

[tgodzik]

I wonder if it would be faster to go over symbols, check if they are field or method and only as the last step find occurence?

[tgodzik]

Overall, I think this is ready to merge. I added one comment about possible optimization, but maybe until we have proper benchmarks we cannot really assume anything.

[tgodzik]

The question remains on how we go around and merge it? If we merge this PR this will cause lense to appear which do not do anything. We need the merge in the VS Code plugin for that.

@olafurpg @gabro @ckipp01 What do you think?

1. Merge here and just wait for the next release. If somebody uses snapshots just tell them those don't work?

2. Or merge with the default setting to false? Later just before the release we enable them.

3. Or merge the VS Code PR and release without the server update, which will in turn create commands that do not do anything.

I think 3 is most reasonable for now.

[ckipp01]

The question remains on how we go around and merge it? If we merge this PR this will cause lense to appear which do not do anything. We need the merge in the VS Code plugin for that.

@olafurpg @gabro @ckipp01 What do you think?

1. Merge here and just wait for the next release. If somebody uses snapshots just tell them those don't work?

2. Or merge with the default setting to false? Later just before the release we enable them.

3. Or merge the VS Code PR and release without the server update, which will in turn create commands that do not do anything.

I think 3 is most reasonable for now.

I think we should just do nightly releases and not worry about it 😏 ...

Jokes aside, yea we ran into this in the last big release as well, and I think it will continue to happen 🤔 . For coc-metals I error on the side of merging things in that require the snapshot of metals since it's easy to use both the master branch of coc-metals and a snapshot or vice versa. However, in VS Code we have the tricky situation where I assume the average user doesn't want to build the VS Code extension locally in order to use a Metals snapshot.

Would it be possible to do a hybrid of one, and turn the feature off by default unless explicitly turned on by the client? That way the lenses won't appear? Then when we do merge and release the vs code client the default "on" will trigger it and the lenses will then appear?

[tgodzik]

Would it be possible to do a hybrid of one, and turn the feature off by default unless explicitly turned on by the client? That way the lenses won't appear? Then when we do merge and release the vs code client the default "on" will trigger it and the lenses will then appear?

I think that's a great idea! We can update the vscode plugin and label it to wait for the release.