x/tools/gopls: semantic tokens: highlight control flow tokens #67663

jmg-duarte · 2024-05-27T07:56:44Z

gopls version

Build info

golang.org/x/tools/gopls v0.15.3
golang.org/x/tools/[email protected] h1:zbdOidFrPTc8Bx0YrN5QKgJ0zCjyGi0L27sKQ/bDG5o=
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+54aec2uopH3x7MAiqGW6Y=
golang.org/x/[email protected] h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8=
golang.org/x/[email protected] h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
golang.org/x/[email protected] h1:vcVnuftN4J4UKLRcgetjzfU9FjjgXUUYUc3JhFplgV4=
golang.org/x/[email protected] h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
golang.org/x/[email protected] h1:uH9jJYgeLCvblH0S+03kFO0qUDxRkbLRLFiKVVDl7ak=
golang.org/x/[email protected] h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
honnef.co/go/[email protected] h1:oFEHCKeID7to/3autwsWfnuv69j3NsfcXbvJKuIcep8=
mvdan.cc/[email protected] h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/[email protected] h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.22.3

go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/jmgd/Library/Caches/go-build'
GOENV='/Users/jmgd/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/jmgd/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/jmgd/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.3'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/dev/null'
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 -ffile-prefix-map=/var/folders/4d/v4vg0mc15_gf9skpnjgmmhdm0000gn/T/go-build2911028327=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Write code with some sort of control flow:

func controlFlow(i int64) {
    if i < 0 {
        return true
    } else {
        return false
    }
}

What did you see happen?

return, if and else (i.e. control flow keywords) are marked as keyword only, while the TextMate grammar already has support for control flow

What did you expect to see?

The controlFlow modifier along with the respective keywords, like in this Rust example

Editor and settings

  "go.toolsManagement.autoUpdate": true,
  "gopls": {
    "ui.semanticTokens": true
  },

Logs

No response

The text was updated successfully, but these errors were encountered:

hyangah · 2024-05-28T20:36:05Z

I don't see controlFlow from https://code.visualstudio.com/api/language-extensions/semantic-highlight-guide#semantic-token-scope-map or LSP spec.

It looks like this is rust-analyzer's VS Code extension specific modifier (https://github.com/rust-lang/rust-analyzer/blob/b32f181f477576bb203879f7539608f3327b6178/editors/code/package.json#L2092).

jmg-duarte · 2024-05-29T07:24:12Z

Hmm might be... That's unfortunate, but I think it would still be useful to add it (and I'm open to do so if there's interest).
For a bit more context: my themes highlight control flow as they're a really important part of the code, the idea is based on https://lunacookies.github.io/sema/

Anyway, I also found the issue on my theme, it was that pesky "keyword": "".

If you think the controlFlow addition is not useful, we can close this.

adonovan · 2024-05-29T15:03:40Z

The LSP semantic tokens design is extensible, so servers are not bound to emit all and only the tokens enumerated in the standard; this seems like a well-defined and reasonable class of tokens to highlight; and it's easy enough to add. Would you care to send us a CL? The function you need to change is golang.org/x/tools/gopls/internal/golang.(*tokenVisitor).inspect.

jmg-duarte · 2024-05-29T20:35:59Z

The following diff doesn't work and I don't understand why, isn't it the modifier that I want to change?

I'm testing using const semDebug = true and gopls semtok foo.go > /dev/null but pointing to the gopls/internal/protocol/semtok/semtok.go and looking for ReturnStmts as they're the most prolific ones.

@adonovan if you could take a look, that would be amazing! Thanks in advance!

diff --git a/gopls/internal/golang/semtok.go b/gopls/internal/golang/semtok.go
index ec628f501..a23cf92a7 100644
--- a/gopls/internal/golang/semtok.go
+++ b/gopls/internal/golang/semtok.go
@@ -356,9 +356,9 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 		tv.token(n.OpPos, len(n.Op.String()), semtok.TokOperator, nil)
 	case *ast.BlockStmt:
 	case *ast.BranchStmt:
-		tv.token(n.TokPos, len(n.Tok.String()), semtok.TokKeyword, nil)
+		tv.token(n.TokPos, len(n.Tok.String()), semtok.TokKeyword, []string{"controlFlow"})
 		if n.Label != nil {
-			tv.token(n.Label.Pos(), len(n.Label.Name), semtok.TokLabel, nil)
+			tv.token(n.Label.Pos(), len(n.Label.Name), semtok.TokLabel, []string{"controlFlow"})
 		}
 	case *ast.CallExpr:
 		if n.Ellipsis.IsValid() {
@@ -369,7 +369,7 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 		if n.List == nil {
 			iam = "default"
 		}
-		tv.token(n.Case, len(iam), semtok.TokKeyword, nil)
+		tv.token(n.Case, len(iam), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.ChanType:
 		// chan | chan <- | <- chan
 		switch {
@@ -392,7 +392,7 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 	case *ast.CompositeLit:
 	case *ast.DeclStmt:
 	case *ast.DeferStmt:
-		tv.token(n.Defer, len("defer"), semtok.TokKeyword, nil)
+		tv.token(n.Defer, len("defer"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.Ellipsis:
 		tv.token(n.Ellipsis, len("..."), semtok.TokOperator, nil)
 	case *ast.EmptyStmt:
@@ -400,7 +400,7 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 	case *ast.Field:
 	case *ast.FieldList:
 	case *ast.ForStmt:
-		tv.token(n.For, len("for"), semtok.TokKeyword, nil)
+		tv.token(n.For, len("for"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.FuncDecl:
 	case *ast.FuncLit:
 	case *ast.FuncType:
@@ -410,15 +410,15 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 	case *ast.GenDecl:
 		tv.token(n.TokPos, len(n.Tok.String()), semtok.TokKeyword, nil)
 	case *ast.GoStmt:
-		tv.token(n.Go, len("go"), semtok.TokKeyword, nil)
+		tv.token(n.Go, len("go"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.Ident:
 		tv.ident(n)
 	case *ast.IfStmt:
-		tv.token(n.If, len("if"), semtok.TokKeyword, nil)
+		tv.token(n.If, len("if"), semtok.TokKeyword, []string{"controlFlow"})
 		if n.Else != nil {
 			// x.Body.End() or x.Body.End()+1, not that it matters
 			pos := tv.findKeyword("else", n.Body.End(), n.Else.Pos())
-			tv.token(pos, len("else"), semtok.TokKeyword, nil)
+			tv.token(pos, len("else"), semtok.TokKeyword, []string{"controlFlow"})
 		}
 	case *ast.ImportSpec:
 		tv.importSpec(n)
@@ -445,9 +445,9 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 		pos := tv.findKeyword("range", offset, n.X.Pos())
 		tv.token(pos, len("range"), semtok.TokKeyword, nil)
 	case *ast.ReturnStmt:
-		tv.token(n.Return, len("return"), semtok.TokKeyword, nil)
+		tv.token(n.Return, len("return"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.SelectStmt:
-		tv.token(n.Select, len("select"), semtok.TokKeyword, nil)
+		tv.token(n.Select, len("select"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.SelectorExpr:
 	case *ast.SendStmt:
 		tv.token(n.Arrow, len("<-"), semtok.TokOperator, nil)
@@ -457,7 +457,7 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 	case *ast.StructType:
 		tv.token(n.Struct, len("struct"), semtok.TokKeyword, nil)
 	case *ast.SwitchStmt:
-		tv.token(n.Switch, len("switch"), semtok.TokKeyword, nil)
+		tv.token(n.Switch, len("switch"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.TypeAssertExpr:
 		if n.Type == nil {
 			pos := tv.findKeyword("type", n.Lparen, n.Rparen)
@@ -465,7 +465,7 @@ func (tv *tokenVisitor) inspect(n ast.Node) (descend bool) {
 		}
 	case *ast.TypeSpec:
 	case *ast.TypeSwitchStmt:
-		tv.token(n.Switch, len("switch"), semtok.TokKeyword, nil)
+		tv.token(n.Switch, len("switch"), semtok.TokKeyword, []string{"controlFlow"})
 	case *ast.UnaryExpr:
 		tv.token(n.OpPos, len(n.Op.String()), semtok.TokOperator, nil)
 	case *ast.ValueSpec:

adonovan · 2024-05-29T21:56:10Z

The Encode function computes the intersection of the tokens and modifiers that were enumerated with the tokens and modifiers that were requested by the client. You'll need to add the new modified to this list in the protocol package

	semanticModifiers = [...]string{
		"declaration", "definition", "readonly", "static",
		"deprecated", "abstract", "async", "modification", "documentation", "defaultLibrary",
	}

This list determines the set of modifiers used in the fake clients in the tests. You will need to make your real client send the new modifier too (though apparently it already does, at least when configured to talk to the Rust LSP server).

While it's not defined in the LSP specification (as of version 3.17) it's supported by other tools such as Rust Analyzer Fixes golang/go#67663

While it's not defined in the LSP specification (as of version 3.17) it's supported by other tools such as Rust Analyzer. Fixes golang/go#67663

jmg-duarte 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 May 27, 2024

gopherbot added this to the Unreleased milestone May 27, 2024

adonovan changed the title ~~x/tools/gopls: add control flow token support~~ x/tools/gopls: semantic tokens: highlight control flow tokens May 29, 2024

adonovan added FeatureRequest help wanted labels May 29, 2024

jmg-duarte linked a pull request May 30, 2024 that will close this issue

x/tools/gopls: add "controlFlow" semantic tokens modifier golang/tools#495

Open

findleyr modified the milestones: Unreleased, gopls/backlog May 30, 2024

gabyhelp mentioned this issue Aug 20, 2024

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

gabyhelp mentioned this issue Nov 6, 2024

x/tools/gopls: Missing defaultLibrary semantic token modifier for builtin constants #70219

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/tools/gopls: semantic tokens: highlight control flow tokens #67663

x/tools/gopls: semantic tokens: highlight control flow tokens #67663

jmg-duarte commented May 27, 2024

hyangah commented May 28, 2024 •

edited

Loading

jmg-duarte commented May 29, 2024

adonovan commented May 29, 2024

jmg-duarte commented May 29, 2024

adonovan commented May 29, 2024 •

edited

Loading

x/tools/gopls: semantic tokens: highlight control flow tokens #67663

x/tools/gopls: semantic tokens: highlight control flow tokens #67663

Comments

jmg-duarte commented May 27, 2024

gopls version

Build info

go env

What did you do?

What did you see happen?

What did you expect to see?

Editor and settings

Logs

hyangah commented May 28, 2024 • edited Loading

jmg-duarte commented May 29, 2024

adonovan commented May 29, 2024

jmg-duarte commented May 29, 2024

adonovan commented May 29, 2024 • edited Loading

hyangah commented May 28, 2024 •

edited

Loading

adonovan commented May 29, 2024 •

edited

Loading