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: semantic tokens should report modifiers for the top-level type constructor of each symbol (e.g. struct, number, pointer, interface, etc) #68975

Closed
xzbdmw opened this issue Aug 20, 2024 · 13 comments
Assignees
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

@xzbdmw
Copy link

xzbdmw commented Aug 20, 2024

gopls version

Build info

golang.org/x/tools/gopls v0.0.0-20240816163142-66adacf20fc4
golang.org/x/tools/[email protected] h1:iSreTB1mHFYjQkoNmGjFjKRGkrhedt4wmfg
47nKSo28=
github.com/BurntSushi/[email protected] h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/[email protected] h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/[email protected] h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH
3x7MAiqGW6Y=
golang.org/x/[email protected] h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0=
golang.org/x/[email protected] h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
golang.org/x/[email protected] h1:nU8/tAV/21mkPrCjACUeSibjhynTovgRMXc32
+Y1Aec=
golang.org/x/[email protected] h1:XtiM5bkSOt+ewxlOE/aE/AKEHibwj/6gvWMl9Rsh0Qc=
golang.org/x/[email protected] h1:PoPnfVMls3TamN2+Zq6FsI1uSjUOqW1mt6AXfY
w3kdw=
golang.org/x/[email protected] h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
honnef.co/go/[email protected] h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs=
mvdan.cc/[email protected] h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/[email protected] h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.22.4

go env

GO111MODULE='auto'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/xzb/Library/Caches/go-build'
GOENV='/Users/xzb/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/xzb/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/xzb/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.4'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD=''
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -f
file-prefix-map=/var/folders/gv/r110hgbx1gbgzp95kf_q71x40000gn/T/go-build3574151232=/tmp/go-build -gno
-record-gcc-switches -fno-common'

What did you do?

in semtok.go This does not have any actual implementation, and lack of interface color make it very hard to decide wether a param/return variable is a interface or a struct

What did you see happen?

The output of :inspect shows A and B both have

Semantic Tokens
  - @lsp.type.type.go links to Type priority: 125
  - @lsp.mod.definition.go links to @lsp priority: 126
  - @lsp.typemod.type.definition.go links to @lsp priority: 127

and color is the same.
截屏2024-08-21 02 53 46

What did you expect to see?

I want to see different colors of struct and interface in definition and param position. I'd like to implement this if it is not that hard, thanks to any guidance in advance!

Editor and settings

neovim

Logs

No response

@xzbdmw xzbdmw 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 Aug 20, 2024
@gopherbot gopherbot added this to the Unreleased milestone Aug 20, 2024
@adonovan
Copy link
Member

Semantic tokens are mostly concerned with properties of identifiers, which refer to symbols (named entities). The interesting properties of symbols in Go are things like whether they are var/func/const/type/label, and so on; the actual underlying type (map or channel, say), is not something that semantic tokens cares about. In Go, interface is more like map and channel than like var/func/const/type. So I argue that it's not an appropriate token type for us to report. (By contrast, in Java, "class" and "interface" are properties of the declared symbol, so it does make sense to report them.)

@xzbdmw
Copy link
Author

xzbdmw commented Aug 22, 2024

The interesting properties of symbols in Go are things like whether they are var/func/const/type/label

But they can be easily identified by regex and treesitter, while recognizing whether a typename is an interface requires true understanding of code as the term semantic highlights implies.

In Go, interface is more like map and channel than like var/func/const/type. So I argue that it's not an appropriate token type for us to report

Hmm at leat interface looks like the same abstract level as an concrete type despite it is itself a type, given interface are so widespread in codebase and semantically different from a concrete struct type, the ability to distinguish them brings real values, especially in an unfamaliar codebase.

@adonovan
Copy link
Member

Hmm at leat interface looks like the same abstract level as an concrete type despite it is itself a type, given interface are so widespread in codebase and semantically different from a concrete struct type, the ability to distinguish them brings real values, especially in an unfamaliar codebase.

I'm not sure I follow. Why is knowing that a symbol's type is an interface more important than knowing that it's a struct, map, channel, or number? There's no technical reason that we couldn't extend semantic tokens to report distinct modifiers for each top-level type constructor of a symbol's type, but I don't understand why would we do this just for interface types.

@xzbdmw
Copy link
Author

xzbdmw commented Aug 23, 2024

Sorry to be unclear here, what I mean "interface" is the typename of an identifier instead of the identifier itself, for example
截屏2024-08-23 23 22 25
The ResponseWriter is an interface type and Request is a concrete type, and it would be better to paint ResponseWriter a different color by semantic tokens so we can know in a glance that this param needs to pass a struct implements this interface.

@adonovan
Copy link
Member

The ResponseWriter is an interface type and Request is a concrete type, and it would be better to paint ResponseWriter a different color by semantic tokens so we can know in a glance that this param needs to pass a struct implements this interface.

Understood, but my question is: what's special about interface types, as opposed to pointer, struct, number types, etc? Why does only one particular type constructor warrant this special treatment?

@xzbdmw
Copy link
Author

xzbdmw commented Aug 23, 2024

Because we know a pointer has a prefix *, number types are some finite set such as int32 or float32, and map or slice both has special typedef syntax, but interfaces and structs are hard to distinguish just by their content like the image above.

@adonovan
Copy link
Member

Because we know a pointer has a prefix *, number types are some finite set such as int32 or float32, and map or slice both has special typedef syntax, but interfaces and structs are hard to distinguish just by their content like the image above.

But that's not true in general. Go has three kinds of named types (alias types, defined types, and type parameters), all of which can obscure the underlying type so that you can't tell (without digging) whether type T is a pointer, a struct, or a number.

@adonovan
Copy link
Member

I will reopen this issue as a more general feature request for semantic tokens to include the top-level type constructor of each identifier.

@adonovan adonovan reopened this Aug 23, 2024
@adonovan adonovan changed the title x/tools/gopls: semantic tokens lackes interface x/tools/gopls: semantic tokens should report the top-level type constructor of each symbol (e.g. struct, number, pointer, interface, etc) Aug 23, 2024
@xzbdmw
Copy link
Author

xzbdmw commented Aug 23, 2024

I have read the semantic code and it seems there are no easy ways to see the top-level types by design, do you have any suggestions how to implements this?

@adonovan
Copy link
Member

I have read the semantic code and it seems there are no easy ways to see the top-level types by design, do you have any suggestions how to implements this?

I would start by looking at (*tokenVisitor).ident, calling obj.Type().Underlying(), type-switching over the cases *types.{Map,Slice,Pointer,...}, and adding a modifier ("map", "slice", "pointer") based on the case.

@adonovan adonovan changed the title x/tools/gopls: semantic tokens should report the top-level type constructor of each symbol (e.g. struct, number, pointer, interface, etc) x/tools/gopls: semantic tokens should report modifiers for the top-level type constructor of each symbol (e.g. struct, number, pointer, interface, etc) Aug 23, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/608156 mentions this issue: gopls: report semantic tokens of top-level type constructor modifiers

@adonovan
Copy link
Member

Thanks for contributing the implementation!

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

Successfully merging a pull request may close this issue.

5 participants