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: Display struct methods #54008

Open
MathiasHandle opened this issue Jul 17, 2022 · 7 comments
Open

x/tools/gopls: Display struct methods #54008

MathiasHandle opened this issue Jul 17, 2022 · 7 comments
Labels
FeatureRequest gopls help wanted Tools
Milestone

Comments

@MathiasHandle
Copy link

MathiasHandle commented Jul 17, 2022

Is your feature request related to a problem? Please describe.

I have difficulties finding all available methods for given struct, hence this request.

Describe the solution you'd like

When on hover on struct I'd like to display all methods belonging to that struct.

This should work across files. As currently the outline view displays only types defined in currently opened file.

There are 2 solutions that come to my mind:

1) Display methods in hover docs (preferred solution)

  • I've took this implementation from JetBrain's Goland IDE as I find it as a best solution

method_overview3

2) Sort methods in outline view

  • If the first implementation wouldn't be possible, I think this could still make code navigation easier.

Currently the outline view can be sorted by
1.) position
2.) Name
3.) Category

But the methods aren't visually assigned to struct they belong to.
outline

Additional context

These proposed changes don't make sense in this simple example, but in real project with a lot of types, methods, packages, etc.. it would make better & cleaner code navigation.

@jamalc jamalc added FeatureRequest gopls labels Jul 19, 2022
@jamalc jamalc transferred this issue from golang/vscode-go Jul 22, 2022
@jamalc jamalc changed the title Display struct methods x/tools/gopls: Display struct methods Jul 22, 2022
@gopherbot gopherbot added the Tools label Jul 22, 2022
@gopherbot gopherbot added this to the Unreleased milestone Jul 22, 2022
@hyangah
Copy link
Contributor

hyangah commented Jul 25, 2022

We discussed this offline and agree that the option 1 is reasonable.

Relevant code location:

https://github.com/golang/tools/blob/3d474c89054e6c6094b99e57fe652ad6d45c2976/internal/lsp/source/hover.go#L246

For people who love more concise hover docs, I think this should be controlled by an option but I will leave @findleyr for the guide.

@gopherbot
Copy link

gopherbot commented Aug 2, 2022

Change https://go.dev/cl/420714 mentions this issue: internal/lsp/source: add functions to struct hover output

@brianpursley
Copy link

brianpursley commented Aug 2, 2022

/assign

@findleyr
Copy link
Contributor

findleyr commented Aug 2, 2022

Thanks @brianpursley. I commented on your CL, but I think we should use the type-checker to compute the method set for us, rather than trying to resolve it ourselves (resolving a method set is a surprisingly difficult task!).

go/types has a function that does this: go/types.NewMethodSet. Here is an example: https://go.dev/play/p/7s8iE66SJjH

You can get the type by looking it up in types.Info.Types, and then pass it to NewMethodSet to compute a representation of its methods. This will work for more than just structs: any defined type may have methods. We probably don't want to do this for interface types though, as I think it will be redundant with the type definition.

Other questions to answer:

  • do we want to show all methods (included those that are added via embedding), or just those that are directly defined (if just the latter, maybe we don't even need NewMethodSet and could just use the go/types.Named API.
  • how do we want to qualify names in the type signature. Relative to the package defining the type, or relative to the current package being hovered? go/types.TypeString and go/types.ObjectString provide APIs for formatting types and objects with a custom qualifier
  • how best do we present the methods visually? Do we want to include their receiver, or just show their name and function signature?

@brianpursley
Copy link

brianpursley commented Aug 2, 2022

Thanks @findleyr . I had some "issues" with my PR, but will work through them and have a better one soon 😅

Thanks for the advice too, I am new to the go/tools codebase, so it will be helpful to get my bearings and much appreciated.

I'll consider your points and see what can be done for each of them.

Regarding appearance, I was thinking to include the receiver, in case you want to know whether it is MyStruct or *MyStruct

For what its worth, Goland does it like this:

type MyStruct struct {
    Value1 int
    Value2 int
}
Methods on (*MyStruct):  Add(x int) int
                         Multiply() int
Methods on (MyStruct):  subtract(x int) int

My first thought was to just include the whole signature, but I don't feel too strongly in either direction though.

I'll un-abandon my PR and mark it WIP until I can get it to a better place.

@findleyr
Copy link
Contributor

findleyr commented Aug 2, 2022

Cool! This is a great place to contribute as the change is mostly isolated to hover.go (though our hover handling is a bit hard to follow).

Something like what you propose looks good to me. IIRC if we use go/types.TypeString or go/types.ObjectString, we will format the receiver (func (*MyStruct) Add(x int) int), which we probably don't want. So we may need to do a bit of massaging to get it into that form.

@findleyr
Copy link
Contributor

findleyr commented Aug 2, 2022

Another thought: it would be nice to link to the method locations using file:// links. Does not need to be done in the first pass though.
microsoft/vscode#8223 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls help wanted Tools
Projects
None yet
Development

No branches or pull requests

6 participants