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: find references should link concrete methods to interface methods #35999

Open
muirdm opened this issue Dec 5, 2019 · 5 comments
Open

Comments

@muirdm
Copy link

@muirdm muirdm commented Dec 5, 2019

For example:

type fooer interface {
  foo()
}

type impl struct{}
func (impl) foo() {}

func _() {
  var f fooer = impl{}
  f.foo()
}

Currently if you find-references on the "foo" struct method, you don't get anything. It would be useful to return the interface method "fooer.foo". Otherwise, there is no way to find how the concrete "foo" method is used. (It would still be a two step process: first jump to the declaration of fooer.foo, then run find references again to see where it is used).

The same logic doesn't apply to the "impl" struct itself since it will actually be referenced somewhere when it is constructed.

Another idea would be to overload the find-implementations to work in reverse as well. In other words, if you run find-implementations on a concrete type or method, it would show you the interface types/methods that are implemented.

@gopherbot gopherbot added this to the Unreleased milestone Dec 5, 2019
@gopherbot
Copy link

@gopherbot gopherbot commented Dec 5, 2019

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Dec 5, 2019
@muirdm
Copy link
Author

@muirdm muirdm commented Dec 11, 2019

I've realized that making find-references work this way has the disadvantage of needing to search all packages, not just transitive dependents.

Overloading find-implementations is better in certain ways but probably too confusing/undiscoverable since no one will expect it to work backwards as well.

Do you have any thoughts @stamblerre?

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 12, 2019

We do have an issue tracking the reverse of implementations, which I agree is a better fit here. Maybe it's something we could do through a custom gopls-only textDocument/implements method that we could then propose to the LSP folks as an addition to the protocol? It seems like they like to have a reference implementation before adding a feature, so it would basically act as proof to them that this is useful.

@muirdm
Copy link
Author

@muirdm muirdm commented Dec 12, 2019

Sounds good. Closing as dupe of #35550

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 2, 2020

Reopening because of the discussion on microsoft/vscode-go#3140. Not sure if this is worth implementing, but it might be nice to reconsider in light of recent changes to gopls.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.0 Apr 2, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.