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

[jmg-duarte]

gopls version

Build info

golang.org/x/tools/gopls v0.15.3
golang.org/x/tools/gopls@v0.15.3 h1:zbdOidFrPTc8Bx0YrN5QKgJ0zCjyGi0L27sKQ/bDG5o=
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/mod@v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8=
golang.org/x/sync@v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
golang.org/x/telemetry@v0.0.0-20240209200032-7b892fcb8a78 h1:vcVnuftN4J4UKLRcgetjzfU9FjjgXUUYUc3JhFplgV4=
golang.org/x/text@v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
golang.org/x/tools@v0.18.1-0.20240412183611-d92ae0781217 h1:uH9jJYgeLCvblH0S+03kFO0qUDxRkbLRLFiKVVDl7ak=
golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
honnef.co/go/tools@v0.4.6 h1:oFEHCKeID7to/3autwsWfnuv69j3NsfcXbvJKuIcep8=
mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/v2@v2.5.0 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

[hyangah]

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]

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]

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]

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]

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).