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: feature: alternative 'references' query that reports implied references to struct fields (etc) #66356

Open
ThinkChaos opened this issue Mar 16, 2024 · 7 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@ThinkChaos
Copy link

ThinkChaos commented Mar 16, 2024

gopls version

gopls 0.14.2 from nixpkgs: /nix/store/1spk3fici3mrggasy75w31y0yvqlrk08-gopls-0.14.2/bin/gopls

$ gopls -v version
Build info
----------
golang.org/x/tools/gopls (devel)
    golang.org/x/tools/gopls@(devel)
    github.com/BurntSushi/toml@v1.2.1
    github.com/google/go-cmp@v0.5.9
    github.com/sergi/go-diff@v1.1.0
    golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338
    golang.org/x/mod@v0.14.0
    golang.org/x/sync@v0.4.0
    golang.org/x/sys@v0.14.0
    golang.org/x/telemetry@v0.0.0-20231114163143-69313e640400
    golang.org/x/text@v0.13.0
    golang.org/x/tools@v0.14.1-0.20231114185516-c9d3e7de13fd
    golang.org/x/vuln@v1.0.1
    honnef.co/go/tools@v0.4.5
    mvdan.cc/gofumpt@v0.4.0
    mvdan.cc/xurls/v2@v2.4.0
go: go1.21.6

go env

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/me/.cache/go-build'
GOENV='/home/me/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/me/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/me/go'
GOPRIVATE=''
GOPROXY='direct'
GOROOT='/nix/store/cw9dqybf9w6wp7827h23pb3ym8gs8h47-go-1.21.6/share/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/nix/store/cw9dqybf9w6wp7827h23pb3ym8gs8h47-go-1.21.6/share/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.6'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/me/Code/blocky/go.mod'
GOWORK='/home/me/Code/blocky/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/run/user/1000/go-build2624082058=/tmp/go-build -gno-record-gcc-switches'

What did you do?

package main

type T struct{ field int }

func main() {
	_ = T{0}
}

What did you see happen?

Seaching for references of field doesn't detect the T literal.

What did you expect to see?

A list with the literal included.

Editor and settings

N/A

Logs

No response

@ThinkChaos ThinkChaos added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Mar 16, 2024
@gopherbot gopherbot added this to the Unreleased milestone Mar 16, 2024
@findleyr
Copy link
Contributor

This is interesting. We could definitely match this reference, and I think it would be useful. We should experiment with doing this. Tentatively putting in the v0.17.0 milestone, as v0.16 is filling up.

CC @adonovan

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.17.0 Mar 20, 2024
@ThinkChaos
Copy link
Author

FWIW the inlay hints show the field names so the info is already wired in somewhere :)

@adonovan
Copy link
Member

By design gopls' references algorithm reports the locations of identifiers that reference a given symbol, whereas in your example there is no field identifier. So the operation is working as intended.

The algorithm does not report all of the many kinds of implicit references that arise in other places, which, in addition to your example, include references:

  • to anonymous fields, in selections where x.f is shorthand for x.a.b.c.f;
  • to parameters, when passing argument values in a function call;
  • to named result values, in a blank return statement;
  • to types, in assignments x = y that involve an implicit conversion x = T(y);

We have had other discussions about and requests for different kinds of 'references' queries, that are not invoked through the usual LSP 'references' operation. For example: show all the places where this struct field or slice element is accessed, even implicitly. It is easy to define the queries as code actions ("Source actions..." in VS Code); the main challenge is the lack of a way to communicate to the client that the result of this operation is a list of annotated source locations that should be displayed using essentially the same UI elements as a true references query. I think we should lobby for the inclusion of such a feature into LSP.

@ThinkChaos
Copy link
Author

Thank you for the thorough reply.

I understand your point and agree with most examples, but don't agree using "identifier is spelled-out" should be a hard rule, and think each case is worth examining from the point of "does this bring value to the user" and "is that value worth the work/complexity".

Maybe the intended use case can motivate some rule bending? It is finding all assignments to a field, which ATM cannot be easily done as you need to check all field references + all struct references. And doing that second search is not intuitive at all IMO.

I think we should lobby for the inclusion of such a feature into LSP.

I agree, having more than one type of symbol search would definitely be useful.

@adonovan
Copy link
Member

adonovan commented Mar 20, 2024

I understand your point and agree with most examples, but don't agree using "identifier is spelled-out" should be a hard rule, and think each case is worth examining from the point of "does this bring value to the user" and "is that value worth the work/complexity".

Many users would be surprised if they asked to see references to "foo" and the result included a line of text that doesn't contain foo. A better UX for this feature would be a query report that annotates the location with a comment such as "implicit selection of field foo". But again that requires LSP changes.

@ThinkChaos
Copy link
Author

ThinkChaos commented Mar 20, 2024

At least in that case you're presented with information so you ask yourself a question. The current situation doesn't confront you to anything and you're very likely to miss some instances of what you're looking for.
EDIT: also the inlay hint makes it pretty obvious.

Anyways feel free to close or rescope to something else if you don't plan on implementing what I opened the issue for originally.

@adonovan adonovan changed the title x/tools/gopls: find reference misses ones from struct literal element list x/tools/gopls: feature: alternative reference misses ones from struct literal element list Mar 20, 2024
@adonovan adonovan changed the title x/tools/gopls: feature: alternative reference misses ones from struct literal element list x/tools/gopls: feature: alternative 'references' query that reports implied references to struct fields (etc) Mar 20, 2024
@adonovan adonovan modified the milestones: gopls/v0.17.0, Backlog Mar 20, 2024
@adonovan
Copy link
Member

I've filed microsoft/language-server-protocol#1911 upstream for LSP support for generalized reference queries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants