internal/lsp: revise some semantic token details
There was a bug and inconsistency in how const identifiers were
categorized when being used. The new code always makes them readonly
variables.
The predeclared identifiers (true, false, nil, iota) were treated
inconsistently. The new code makes them all readonly variables.
Under rare conditions the output from gopls semtok was not syntactic
Go code because is would put a /* comment immediately after an
operator /, thereby generating a spurious line comment.
Fixes golang/go#42715
Change-Id: I5b1ccfaf607d6baa58c132e460c5617db6634a72
Reviewed-on: https://go-review.googlesource.com/c/tools/+/272208
Trust: Peter Weinberger <pjw@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cmd/semantictokens.go b/internal/lsp/cmd/semantictokens.go
index f3c4b12..7716d14 100644
--- a/internal/lsp/cmd/semantictokens.go
+++ b/internal/lsp/cmd/semantictokens.go
@@ -152,7 +152,12 @@
insert = fmt.Sprintf("%s%d,namespace,[]*/", SemanticLeft, length)
splitAt = m.offset + m.len
} else {
- insert = fmt.Sprintf("%s%d,%s,%v*/", SemanticRight, length, m.typ, m.mods)
+ // be careful not to generate //*
+ spacer := ""
+ if splitAt-1 >= 0 && l[splitAt-1] == '/' {
+ spacer = " "
+ }
+ insert = fmt.Sprintf("%s%s%d,%s,%v*/", spacer, SemanticRight, length, m.typ, m.mods)
}
x := append([]byte(insert), l[splitAt:]...)
l = append(l[:splitAt], x...)
diff --git a/internal/lsp/semantic.go b/internal/lsp/semantic.go
index 48700d2..2123f2f 100644
--- a/internal/lsp/semantic.go
+++ b/internal/lsp/semantic.go
@@ -196,7 +196,7 @@
if len(e.stack) > 0 {
loc := e.stack[len(e.stack)-1].Pos()
add := e.pgf.Tok.PositionFor(loc, false)
- msg = append(msg, fmt.Sprintf("(%d:%d)", add.Line, add.Column))
+ msg = append(msg, fmt.Sprintf("(line:%d,col:%d)", add.Line, add.Column))
}
msg = append(msg, "]")
return strings.Join(msg, " ")
@@ -372,25 +372,8 @@
case *types.Const:
mods := []string{"readonly"}
tt := y.Type()
- if ttx, ok := tt.(*types.Basic); ok {
- switch bi := ttx.Info(); {
- case bi&types.IsNumeric != 0:
- me := tokVariable
- if x.String() == "iota" {
- me = tokKeyword
- mods = []string{}
- }
- e.token(x.Pos(), len(x.String()), me, mods)
- case bi&types.IsString != 0:
- e.token(x.Pos(), len(x.String()), tokString, mods)
- case bi&types.IsBoolean != 0:
- e.token(x.Pos(), len(x.Name), tokKeyword, nil)
- case bi == 0:
- e.token(x.Pos(), len(x.String()), tokVariable, mods)
- default:
- msg := fmt.Sprintf("unexpected %x at %s", bi, e.pgf.Tok.PositionFor(x.Pos(), false))
- e.unexpected(msg)
- }
+ if _, ok := tt.(*types.Basic); ok {
+ e.token(x.Pos(), len(x.String()), tokVariable, mods)
break
}
if ttx, ok := tt.(*types.Named); ok {
@@ -411,7 +394,7 @@
// nothing to map it to
case *types.Nil:
// nil is a predeclared identifier
- e.token(x.Pos(), len("nil"), tokKeyword, []string{"readonly"})
+ e.token(x.Pos(), len("nil"), tokVariable, []string{"readonly"})
case *types.PkgName:
e.token(x.Pos(), len(x.Name), tokNamespace, nil)
case *types.TypeName:
diff --git a/internal/lsp/testdata/semantic/a.go b/internal/lsp/testdata/semantic/a.go
index a8c7d99..756c56e 100644
--- a/internal/lsp/testdata/semantic/a.go
+++ b/internal/lsp/testdata/semantic/a.go
@@ -1,4 +1,4 @@
-package semantictokens
+package semantictokens //@ semantic("")
import (
_ "encoding/utf8"
diff --git a/internal/lsp/testdata/semantic/a.go.golden b/internal/lsp/testdata/semantic/a.go.golden
index 6721726..5bb276c 100644
--- a/internal/lsp/testdata/semantic/a.go.golden
+++ b/internal/lsp/testdata/semantic/a.go.golden
@@ -1,5 +1,5 @@
-- semantic --
-/*⇒7,keyword,[]*/package /*⇒14,namespace,[]*/semantictokens
+/*⇒7,keyword,[]*/package /*⇒14,namespace,[]*/semantictokens //@ semantic("")
/*⇒6,keyword,[]*/import (
_ "encoding/utf8"/*⇐4,namespace,[]*/
@@ -20,10 +20,10 @@
)
/*⇒5,keyword,[]*/const (
- /*⇒2,variable,[definition readonly]*/xx /*⇒1,type,[]*/F = /*⇒4,keyword,[]*/iota
+ /*⇒2,variable,[definition readonly]*/xx /*⇒1,type,[]*/F = /*⇒4,variable,[readonly]*/iota
/*⇒2,variable,[definition readonly]*/yy = /*⇒2,variable,[readonly]*/xx /*⇒1,operator,[]*/+ /*⇒1,number,[]*/3
/*⇒2,variable,[definition readonly]*/zz = /*⇒2,string,[]*/""
- /*⇒2,variable,[definition readonly]*/ww = /*⇒6,string,[]*/"not " /*⇒1,operator,[]*/+ /*⇒2,string,[readonly]*/zz
+ /*⇒2,variable,[definition readonly]*/ww = /*⇒6,string,[]*/"not " /*⇒1,operator,[]*/+ /*⇒2,variable,[readonly]*/zz
)
/*⇒4,keyword,[]*/type /*⇒1,type,[definition]*/A /*⇒6,keyword,[]*/struct {
@@ -50,13 +50,13 @@
/*⇒7,keyword,[]*/default:
}
/*⇒3,keyword,[]*/for /*⇒1,variable,[definition]*/k, /*⇒1,variable,[definition]*/v := /*⇒5,keyword,[]*/range /*⇒1,variable,[]*/m {
- /*⇒6,keyword,[]*/return (/*⇒1,operator,[]*/!/*⇒1,variable,[]*/k) /*⇒2,operator,[]*/&& /*⇒1,variable,[]*/v[/*⇒1,number,[]*/0] /*⇒2,operator,[]*/== /*⇒3,keyword,[readonly]*/nil
+ /*⇒6,keyword,[]*/return (/*⇒1,operator,[]*/!/*⇒1,variable,[]*/k) /*⇒2,operator,[]*/&& /*⇒1,variable,[]*/v[/*⇒1,number,[]*/0] /*⇒2,operator,[]*/== /*⇒3,variable,[readonly]*/nil
}
/*⇒2,variable,[]*/c2 /*⇒2,operator,[]*/<- /*⇒1,type,[]*/A./*⇒1,variable,[definition]*/X
/*⇒1,variable,[definition]*/w /*⇒2,operator,[]*/:= /*⇒1,variable,[]*/b[/*⇒1,number,[]*/4:]
/*⇒1,variable,[definition]*/j /*⇒2,operator,[]*/:= /*⇒3,function,[defaultLibrary]*/len(/*⇒1,variable,[]*/x)
/*⇒1,variable,[]*/j/*⇒2,operator,[]*/--
- /*⇒6,keyword,[]*/return /*⇒4,keyword,[]*/true
+ /*⇒6,keyword,[]*/return /*⇒4,variable,[readonly]*/true
}
/*⇒4,keyword,[]*/func /*⇒1,function,[definition]*/g(/*⇒2,parameter,[definition]*/vv /*⇒3,operator,[]*/.../*⇒9,keyword,[]*/interface{}) {
@@ -64,7 +64,7 @@
/*⇒5,keyword,[]*/defer /*⇒2,variable,[]*/ff()
/*⇒2,keyword,[]*/go /*⇒3,namespace,[]*/utf./*⇒9,variable,[definition]*/RuneCount(/*⇒2,string,[]*/"")
/*⇒2,keyword,[]*/go /*⇒4,namespace,[]*/utf8./*⇒9,function,[]*/RuneCount(/*⇒2,variable,[]*/vv.(/*⇒6,variable,[definition]*/string))
- /*⇒2,keyword,[]*/if /*⇒4,keyword,[]*/true {
+ /*⇒2,keyword,[]*/if /*⇒4,variable,[readonly]*/true {
} /*⇒4,keyword,[]*/else {
}
/*⇒5,parameter,[definition]*/Never:
diff --git a/internal/lsp/testdata/semantic/b.go b/internal/lsp/testdata/semantic/b.go
index 8c10eb7..7d3ab39 100644
--- a/internal/lsp/testdata/semantic/b.go
+++ b/internal/lsp/testdata/semantic/b.go
@@ -1,10 +1,19 @@
package semantictokens //@ semantic("")
+func f(x ...interface{}) {
+}
+
func weirⰀd() {
const (
- snil = nil
- nil = true
- true = false
- false = snil
+ snil = nil
+ nil = true
+ true = false
+ false = snil
+ cmd = `foof`
+ double = iota
+ iota = copy
+ four = (len(cmd)/2 < 5)
+ five = four
)
+ f(cmd, nil, double, iota)
}
diff --git a/internal/lsp/testdata/semantic/b.go.golden b/internal/lsp/testdata/semantic/b.go.golden
index 90d4977..b56bb11 100644
--- a/internal/lsp/testdata/semantic/b.go.golden
+++ b/internal/lsp/testdata/semantic/b.go.golden
@@ -1,12 +1,21 @@
-- semantic --
/*⇒7,keyword,[]*/package /*⇒14,namespace,[]*/semantictokens //@ semantic("")
+/*⇒4,keyword,[]*/func /*⇒1,function,[definition]*/f(/*⇒1,parameter,[definition]*/x /*⇒3,operator,[]*/.../*⇒9,keyword,[]*/interface{}) {
+}
+
/*⇒4,keyword,[]*/func /*⇒6,function,[definition]*/weirⰀd() {
/*⇒5,keyword,[]*/const (
- /*⇒4,variable,[definition readonly]*/snil = /*⇒3,keyword,[readonly]*/nil
- /*⇒3,variable,[definition readonly]*/nil = /*⇒4,keyword,[]*/true
- /*⇒4,variable,[definition readonly]*/true = /*⇒5,keyword,[]*/false
- /*⇒5,variable,[definition readonly]*/false = /*⇒4,variable,[readonly]*/snil
+ /*⇒4,variable,[definition readonly]*/snil = /*⇒3,variable,[readonly]*/nil
+ /*⇒3,variable,[definition readonly]*/nil = /*⇒4,variable,[readonly]*/true
+ /*⇒4,variable,[definition readonly]*/true = /*⇒5,variable,[readonly]*/false
+ /*⇒5,variable,[definition readonly]*/false = /*⇒4,variable,[readonly]*/snil
+ /*⇒3,variable,[definition readonly]*/cmd = /*⇒6,string,[]*/`foof`
+ /*⇒6,variable,[definition readonly]*/double = /*⇒4,variable,[readonly]*/iota
+ /*⇒4,variable,[definition readonly]*/iota = /*⇒4,function,[defaultLibrary]*/copy
+ /*⇒4,variable,[definition readonly]*/four = (/*⇒3,function,[defaultLibrary]*/len(/*⇒3,variable,[readonly]*/cmd)/*⇒1,operator,[]*// /*⇒1,number,[]*/2 /*⇒1,operator,[]*/< /*⇒1,number,[]*/5)
+ /*⇒4,variable,[definition readonly]*/five = /*⇒4,variable,[readonly]*/four
)
+ /*⇒1,function,[]*/f(/*⇒3,variable,[readonly]*/cmd, /*⇒3,variable,[readonly]*/nil, /*⇒6,variable,[readonly]*/double, /*⇒4,variable,[readonly]*/iota)
}
diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden
index e7af0bd..a1daa7f 100644
--- a/internal/lsp/testdata/summary.txt.golden
+++ b/internal/lsp/testdata/summary.txt.golden
@@ -12,7 +12,7 @@
FoldingRangesCount = 2
FormatCount = 6
ImportCount = 8
-SemanticTokenCount = 2
+SemanticTokenCount = 3
SuggestedFixCount = 38
FunctionExtractionCount = 12
DefinitionsCount = 64
