Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: annotations for implementations of interfaces #56695

Open
bika-c opened this issue Jun 22, 2022 · 19 comments
Open

x/tools/gopls: annotations for implementations of interfaces #56695

bika-c opened this issue Jun 22, 2022 · 19 comments
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@bika-c
Copy link

bika-c commented Jun 22, 2022

Since go does not have an explicit declaration of which interfaces one implements, it is hard to follow someone's code. I know that there is a command to show all the interfaces, but it is not very intuitive nor efficient.

Goland has the feature of showing a gutter when a function implements an interface. It would be nice to show maybe a codelens over the implementation and the type, or a similar solution as Goland.

Codelens:
pasted

Goland:
image

I found an extension that does kinda the opposite which is not very helpful in most scenario: galkowskit/go-interface-annotations
image

@dle8 dle8 added FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Jun 23, 2022
@2opremio
Copy link

That's exactly how rust-analyzer handles traits

@icearith
Copy link

icearith commented Sep 2, 2022

this feature seems exactly what I want.

@Shawn-Huang-Tron
Copy link

This feature is very useful and we use it frequently. A lot of my friends doesn't like vscode-go just because of the leak of this feature.So they change to goland.Hope to add such a feature.

@Tony-Sol
Copy link

Tony-Sol commented Nov 8, 2022

+1
this would be an extremely usefull

@hyangah
Copy link
Contributor

hyangah commented Nov 10, 2022

cc @golang/tools-team This can be expensive depending on the size of the search space, but given that there are already similar functionalities (Find All Implementations, or LSP textDocument/implementation), I guess gopls has sufficient information to package this info as a codelens. Is there any concern?

@findleyr
Copy link
Contributor

We have this in our long-term roadmap for gopls (along with a references codelens), most likely once we build out indexes that can satisfy these queries cheaply. Moving to the gopls issue tracker.

@findleyr findleyr changed the title UI: annotations for implementations of interfaces x/tools/gopls: annotations for implementations of interfaces Nov 10, 2022
@findleyr findleyr removed the gopls Issues related to the Go language server, gopls. label Nov 10, 2022
@findleyr findleyr transferred this issue from golang/vscode-go Nov 10, 2022
@gopherbot gopherbot added this to the Unreleased milestone Nov 10, 2022
@findleyr findleyr modified the milestones: Unreleased, gopls/later Nov 10, 2022
@tkgalk
Copy link

tkgalk commented Feb 6, 2023

I found an extension that does kinda the opposite which is not very helpful in most scenario: galkowskit/go-interface-annotations

It actually had bidirectional code lens, but only for structs. I released a 0.1.0 version that makes it work on non-struct types as well. Underneath, it's using the "Find Implementations" and "Find References" VS Code feature, so the gopls version would still be miles better. But it should tide people over until then, if they really need this feature. :)

@adonovan
Copy link
Member

adonovan commented Feb 6, 2023

We are in the process of rewriting gopls' implementations feature so that it uses a global persistent index. Currently gopls must type-check every package in the workspace (since any package might define a type that satsifies an arbitrary interface), but in the next month or so we expect that gopls will be able to make cache hits in the persistent index without type-checking, so it should dramatically reduce the cost of an implementations query in a large workspace on all but the very first run after installing a new gopls executable.

At that point it might become feasible to implement something like this feature request as a code lens.

@tooltitude-support
Copy link

tooltitude-support commented Feb 8, 2023

@bika-c Our team is working on a golang extension which augments the standard go language server with convenience and productivity features which are out of scope for gopls either due to infeasibility in larger repositories or lack of resources. We have more than 20 code actions now. Is there any way to contact you to discuss features you might want?

@adonovan Hope you don't mind us commenting in this repository.

P.S. The extension link is: https://marketplace.visualstudio.com/items?itemName=tooltitudeteam.tooltitude

@adonovan
Copy link
Member

adonovan commented Feb 8, 2023

@adonovan Hope you don't mind us commenting in this repository.

No, of course, that's fine.

Where can I find the source code for Tooltitude? The download page took me to an (obfuscated) executable, and of course there's no way I'm going to run that on my machine. Alternatively, is there any documentation about how it relates to gopls?

@marlongerson
Copy link

Would be nice to have a gutter icon as well like in Goland.

@tkgalk
Copy link

tkgalk commented Jul 26, 2023

Any updates on this one? I tried to implement something for neovim, but I can't seem to get gopls to return what I what, so I guess this is still not implemented. Is that correct?

@Tesohh
Copy link

Tesohh commented Aug 9, 2023

I found an extension that does kinda the opposite which is not very helpful in most scenario: galkowskit/go-interface-annotations

It actually had bidirectional code lens, but only for structs. I released a 0.1.0 version that makes it work on non-struct types as well. Underneath, it's using the "Find Implementations" and "Find References" VS Code feature, so the gopls version would still be miles better. But it should tide people over until then, if they really need this feature. :)

i think they added this in the extension, but still, it would be nice to have these types of features in the go extension by default

@tkgalk
Copy link

tkgalk commented Aug 9, 2023

i think they added this in the extension, but still, it would be nice to have these types of features in the go extension by default

https://github.com/tkg-codes/go-interface-annotations <- I have a hacked solution in my VSCode extension. But it might start to chug on large codebases, I haven't tested it thoroughly.

I would still like to have this be supported by gopls instead of just VSCode, as otherwise I don't think support for Vim/Neovim, Sublime, Zed et cetera is possible. If this feature would be supported by gopls directly then it would be much more performant.

@adonovan
Copy link
Member

gopls now has an index of implementations, so it can compute the corresponding types for a single type efficiently, but this feature needs the ability to query the correspondences for every type in the file. There's still substantial work to do to make an efficient batch-parallel implementations query. Until then, the client side code lens implementation is as good as one can expect, although of course it works only in VS Code.

@tkgalk
Copy link

tkgalk commented Oct 11, 2023

Unrelated, but in case someone gets here by googling and sees your last post about VSCode.

I'm working on a neovim version as well, and for anyone interested there is already lsp-lens.nvim, too. :)

So while gopls version would be better, there are client-side solutions out there.

@adonovan
Copy link
Member

adonovan commented Dec 6, 2023

Someone kindly offered (via email) to contribute this feature. For posterity and transparency, I've shared part of my response to them below.

--

I think this particular feature may be quite challenging to implement efficiently, even now that we've completed the changes we were planning to the architecture of gopls. (In fact, our changes may actually have made this problem more difficult because it is more expensive to join type information from an arbitrary pair of packages.) You may find this blog post and this talk a useful introduction to some of the recent design changes.

Each of the proposed annotations on a type requires a global query to compute the set of interfaces that it implements, or is implemented by. (Yes, the "find implementations" algorithm works both ways.) When I say global, I mean every package, not just the forward or reverse dependencies, because interfaces can be implemented by types that aren't dependencies in either direction. And any change to any file can in principle invalidate the information previously computed, so it's far from clear to me what the invalidation story is for this feature.

This is unlike most other features, which are either a property of forward dependencies, e.g. diagnostics, type information, static analysis, etc, which are served by the index; or of reverse information, e.g. references, also served from an index; or both, e.g. rename, which requires type checking the entire reverse closure (which is expensive, but the feature is infrequently used).

A more achievable goal might be to report only the relationships between each declared type and some other type mentioned in the same package, so for example, os.File would say "implements io.Reader, io.Writer, ... and perhaps others", and the "perhaps others" part could be a link that activates the expensive global query to compute the definitive answer for that particular type at that moment. In order to implement that, you would just need to gather the set of "interesting" types after type checking, compute the pairwise implements relation, and record the set of matches in the source.Package.

--

@findleyr
Copy link
Contributor

findleyr commented Dec 6, 2023

@adonovan I'm not sure this is so infeasible. We already keep a global index of package data live in memory: the set of type checking diagnostics. We also have a serializable format for method sets. Suppose that, as part of analyzing a package, we kept the method set index in memory (IIRC, the serialized method set data is quite small). How expensive would it be to, for all types declared in a single file (i.e. within the context of a codelens request), cross reference this method set index? Suppose also that a small false positive rate were acceptable, as I think may be the case here (or we could detect potential false positives and re-type check a small number of packages to eliminate them).

@JulianGuSM
Copy link

Any updates on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests