x/tools/gopls: include implementations in definition results for interfaces and interface methods

[cdbattags]

Is your feature request related to a problem? Please describe.
Using the cmd+click/option+click flow, I always go to the interface instead of going to the source for the implementation.

Describe the solution you'd like
I would like a setting that opens the "Go to Implementations" dialog instead or might even go to the first implementation automatically.

Describe alternatives you've considered
https://github.com/microsoft/vscode/blob/e1de2a458dfb770545489daf499131fd328924e7/extensions/typescript-language-features/package.json#L1485 as a reference for how TypeScript extension deals with this. I've tried to see if I could add this behavior myself but for now I just rebound the cmd+f12 keybind instead.

Additional context
N/A

[adonovan]

I'm a little confused by this request, because the default behavior of VS Code + Go is that the context menu always offers one item called "Go to Implementations", and the actual operation that this performs (as determined by gopls) depends on whether the selected type is a concrete type or an interface type. For concrete types, it shows interfaces, and for interface types, it shows concrete implementations.

This means that there is no way to ask for more specific interfaces that implement a given interface (see microsoft/language-server-protocol#2037). Is that what you are asking for?

[firelizzard18]

I believe the request is, As a user when I hover/select a method call of an interface value and I execute Go To Definition I want it to jump to the implementation of that interface method (if configured to do so).

If there are multiple implementations, clearly we can't jump since the destination is undefined. However, if the workspace only contains a single implementation of the interface, jumping to that implementation of that method is feasible. That being said, when I execute Go To Definition in JavaScript/TypeScript, sometimes it pops up a preview/hint that includes multiple results. So we should be able to do something similar and list all of the implementations when the user executes Go To Definition on an interface method with multiple implementations.

[adonovan]

I believe the request is, As a user when I hover/select a method call of an interface value and I execute Go To Definition I want it to jump to the implementation of that interface method (if configured to do so).

When an interface method call is selected, "Go to Definition" jumps to the definition of the symbol (in this case, an interface method) whereas "Go to Implementations" shows all the corresponding concrete methods. The first operation is strictly about the reference graph (which relates references to declarations), which is inherently a "downwards" operation in terms of the import graph; that is, the interface is always among the dependencies of the current package. The second operation is a workspace-wide query for matching methods; they could be anywhere at all.

I think it would be extremely confusing for users if we sometimes changed the meaning of "Go to Definition" to mean "Go to implementations". Why not just invoke the latter operation? They are sibling items on the same menu. Perhaps I am missing something.

[firelizzard18]

Why not just invoke the latter operation? They are sibling items on the same menu.

The majority of the time when I execute Go To Definition on a method it's because I want to see the source of that method. I don't open the menu, I control-click on the method. The reason this feature would be useful to me comes down to workflow. I'm jumping around through the code and I want to quickly jump to the implementation of a method, either to edit it or to read and understand it. Hitting this case is jarring. Generally what happens is I control-click the method, which jumps to the interface, so then I have to right click and execute Go To Implementations then jump to the implementation. Mentally I'm in the mode of reading code so when I control-click I expect to see the implementation, and having to switch gears breaks that flow. I could check each time if the receiver is an interface value, but that still breaks my flow. If the interface has multiple implementations I'm not certain whether my suggestion (show the preview window thing) is valuable, but for me personally Go To Definition jumping to the implementation if there's only one would be valuable more often than not.

I think it would be extremely confusing for users if we sometimes changed the meaning of "Go to Definition" to mean "Go to implementations"

Hence the request for an option. If I opt-in to this behavior I'm far less likely to find it confusing, and it would be actively beneficial to me in most cases.

[gopherbot]

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

[findleyr]

If I'm understanding correctly, this is really a feature request for gopls, because the LSP allows for multiple results from textDocument/definition

I'd really like to avoid a new setting here, so I'd first like to answer the question of whether or not users want 'jump to definition' on an interface or interface method to include implementations. Perhaps we can run a poll in the gopls / VS Code slack, or a survey.

The performance cost of this change, is nontrivial. As Alan noted, "downwards" operations are essentially free in gopls, whereas workspace wide queries require re-typechecking all invalidated packages. Unfortunately, for this reason we may need a setting, if collecting implementations is too slow in large workspaces.