internal/lsp: add a test for gc annotation details code lens
This change adds a test to check the functionality of the GC details
code lens, including the behavior of toggling it on and off. This
exposed a Windows bug that was mentioned on Slack, which can be fixed by
adjusting the URI.
I also refactored a bit of the code to use a JSON decoder, which
simplifies things a little bit.
Change-Id: I7737107cf020fa35bca245485a3b1a1416920fd2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/256940
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
diff --git a/gopls/internal/regtest/codelens_test.go b/gopls/internal/regtest/codelens_test.go
index 3fcfdce..5dd3afb 100644
--- a/gopls/internal/regtest/codelens_test.go
+++ b/gopls/internal/regtest/codelens_test.go
@@ -5,6 +5,8 @@
package regtest
import (
+ "runtime"
+ "strings"
"testing"
"golang.org/x/tools/internal/lsp/protocol"
@@ -177,24 +179,7 @@
`
runner.Run(t, shouldRemoveDep, func(t *testing.T, env *Env) {
env.OpenFile("go.mod")
- lenses := env.CodeLens("go.mod")
- want := "Tidy module"
- var found protocol.CodeLens
- for _, lens := range lenses {
- if lens.Command.Title == want {
- found = lens
- break
- }
- }
- if found.Command.Command == "" {
- t.Fatalf("did not find lens %q, got %v", want, found.Command)
- }
- if _, err := env.Editor.Server.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{
- Command: found.Command.Command,
- Arguments: found.Command.Arguments,
- }); err != nil {
- t.Fatal(err)
- }
+ env.ExecuteCodeLensCommand("go.mod", source.CommandTidy)
env.Await(NoOutstandingWork())
got := env.ReadWorkspaceFile("go.mod")
const wantGoMod = `module mod.com
@@ -239,19 +224,62 @@
env.Await(env.DiagnosticAtRegexp("cgo.go", `C\.(fortytwo)`))
// Regenerate cgo, fixing the diagnostic.
- lenses := env.CodeLens("cgo.go")
- var lens protocol.CodeLens
- for _, l := range lenses {
- if l.Command.Command == source.CommandRegenerateCgo.Name {
- lens = l
+ env.ExecuteCodeLensCommand("cgo.go", source.CommandRegenerateCgo)
+ env.Await(EmptyDiagnostics("cgo.go"))
+ })
+}
+
+func TestGCDetails(t *testing.T) {
+ testenv.NeedsGo1Point(t, 15)
+ if runtime.GOOS == "android" {
+ t.Skipf("the gc details code lens doesn't work on Android")
+ }
+
+ const mod = `
+-- go.mod --
+module mod.com
+
+go 1.15
+-- main.go --
+package main
+
+import "fmt"
+
+func main() {
+ var x string
+ fmt.Println(x)
+}
+`
+ withOptions(
+ EditorConfig{CodeLens: map[string]bool{"gc_details": true}},
+ ).run(t, mod, func(t *testing.T, env *Env) {
+ env.Await(InitialWorkspaceLoad)
+ env.OpenFile("main.go")
+ env.ExecuteCodeLensCommand("main.go", source.CommandToggleDetails)
+ d := &protocol.PublishDiagnosticsParams{}
+ env.Await(
+ OnceMet(
+ DiagnosticAt("main.go", 6, 12),
+ ReadDiagnostics("main.go", d),
+ ),
+ )
+ // Confirm that the diagnostics come from the gc details code lens.
+ var found bool
+ for _, d := range d.Diagnostics {
+ if d.Severity != protocol.SeverityInformation {
+ t.Fatalf("unexpected diagnostic severity %v, wanted Information", d.Severity)
+ }
+ if strings.Contains(d.Message, "x escapes") {
+ found = true
}
}
- if _, err := env.Editor.Server.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{
- Command: lens.Command.Command,
- Arguments: lens.Command.Arguments,
- }); err != nil {
- t.Fatal(err)
+ if !found {
+ t.Fatalf(`expected to find diagnostic with message "escape(x escapes to heap)", found none`)
}
- env.Await(EmptyDiagnostics("cgo.go"))
+ // Toggle the GC details code lens again so now it should be off.
+ env.ExecuteCodeLensCommand("main.go", source.CommandToggleDetails)
+ env.Await(
+ EmptyDiagnostics("main.go"),
+ )
})
}
diff --git a/gopls/internal/regtest/wrappers.go b/gopls/internal/regtest/wrappers.go
index 8d8136f..294d303 100644
--- a/gopls/internal/regtest/wrappers.go
+++ b/gopls/internal/regtest/wrappers.go
@@ -10,6 +10,7 @@
"golang.org/x/tools/internal/lsp/fake"
"golang.org/x/tools/internal/lsp/protocol"
+ "golang.org/x/tools/internal/lsp/source"
errors "golang.org/x/xerrors"
)
@@ -259,6 +260,29 @@
return lens
}
+// ExecuteCodeLensCommand executes the command for the code lens matching the
+// given command name.
+func (e *Env) ExecuteCodeLensCommand(path string, cmd *source.Command) {
+ lenses := e.CodeLens(path)
+ var lens protocol.CodeLens
+ var found bool
+ for _, l := range lenses {
+ if l.Command.Command == cmd.Name {
+ lens = l
+ found = true
+ }
+ }
+ if !found {
+ e.T.Fatalf("found no command with the title %s", cmd.Name)
+ }
+ if _, err := e.Editor.Server.ExecuteCommand(e.Ctx, &protocol.ExecuteCommandParams{
+ Command: lens.Command.Command,
+ Arguments: lens.Command.Arguments,
+ }); err != nil {
+ e.T.Fatal(err)
+ }
+}
+
// References calls textDocument/references for the given path at the given
// position.
func (e *Env) References(path string, pos fake.Pos) []protocol.Location {
diff --git a/internal/lsp/command.go b/internal/lsp/command.go
index 2f0f4d5..e6937ef 100644
--- a/internal/lsp/command.go
+++ b/internal/lsp/command.go
@@ -11,8 +11,6 @@
"fmt"
"io"
"io/ioutil"
- "log"
- "path"
"path/filepath"
"golang.org/x/tools/internal/event"
@@ -203,7 +201,7 @@
if err := source.UnmarshalArgs(args, &fileURI); err != nil {
return err
}
- pkgDir := span.URIFromPath(path.Dir(fileURI.Filename()))
+ pkgDir := span.URIFromPath(filepath.Dir(fileURI.Filename()))
s.gcOptimizationDetailsMu.Lock()
if _, ok := s.gcOptimizatonDetails[pkgDir]; ok {
delete(s.gcOptimizatonDetails, pkgDir)
@@ -211,8 +209,6 @@
s.gcOptimizatonDetails[pkgDir] = struct{}{}
}
s.gcOptimizationDetailsMu.Unlock()
- event.Log(ctx, fmt.Sprintf("gc_details %s now %v %v", pkgDir, s.gcOptimizatonDetails[pkgDir],
- s.gcOptimizatonDetails))
// need to recompute diagnostics.
// so find the snapshot
sv, err := s.session.ViewOf(fileURI)
@@ -290,7 +286,6 @@
var failedTests int
for _, funcName := range tests {
args := []string{pkgPath, "-v", "-count=1", "-run", fmt.Sprintf("^%s$", funcName)}
- log.Printf("running with these args: %v", args)
if err := snapshot.RunGoCommandPiped(ctx, "test", args, out, out); err != nil {
if errors.Is(err, context.Canceled) {
return err
diff --git a/internal/lsp/source/code_lens.go b/internal/lsp/source/code_lens.go
index 427b548..2aee889 100644
--- a/internal/lsp/source/code_lens.go
+++ b/internal/lsp/source/code_lens.go
@@ -242,14 +242,12 @@
if err != nil {
return nil, err
}
- return []protocol.CodeLens{
- {
- Range: rng,
- Command: protocol.Command{
- Title: "Toggle gc annotation details",
- Command: CommandToggleDetails.Name,
- Arguments: jsonArgs,
- },
+ return []protocol.CodeLens{{
+ Range: rng,
+ Command: protocol.Command{
+ Title: "Toggle gc annotation details",
+ Command: CommandToggleDetails.Name,
+ Arguments: jsonArgs,
},
- }, nil
+ }}, nil
}
diff --git a/internal/lsp/source/gc_annotations.go b/internal/lsp/source/gc_annotations.go
index 2d85026..8aa13cb 100644
--- a/internal/lsp/source/gc_annotations.go
+++ b/internal/lsp/source/gc_annotations.go
@@ -20,6 +20,7 @@
func GCOptimizationDetails(ctx context.Context, snapshot Snapshot, pkgDir span.URI) (map[VersionedFileIdentity][]*Diagnostic, error) {
outDir := filepath.Join(os.TempDir(), fmt.Sprintf("gopls-%d.details", os.Getpid()))
+
if err := os.MkdirAll(outDir, 0700); err != nil {
return nil, err
}
@@ -28,7 +29,15 @@
return nil, err
}
defer os.Remove(tmpFile.Name())
- args := []string{fmt.Sprintf("-gcflags=-json=0,%s", outDir),
+
+ outDirURI := span.URIFromPath(outDir)
+ // GC details doesn't handle Windows URIs in the form of "file:///C:/...",
+ // so rewrite them to "file://C:/...". See golang/go#41614.
+ if !strings.HasPrefix(outDir, "/") {
+ outDirURI = span.URI(strings.Replace(string(outDirURI), "file:///", "file://", 1))
+ }
+ args := []string{
+ fmt.Sprintf("-gcflags=-json=0,%s", outDirURI),
fmt.Sprintf("-o=%s", tmpFile.Name()),
pkgDir.Filename(),
}
@@ -44,100 +53,114 @@
opts := snapshot.View().Options()
var parseError error
for _, fn := range files {
- fname, v, err := parseDetailsFile(fn)
+ uri, diagnostics, err := parseDetailsFile(fn, opts)
if err != nil {
// expect errors for all the files, save 1
parseError = err
}
- if !strings.HasSuffix(fname, ".go") {
- continue // <autogenerated>
- }
- uri := span.URIFromPath(fname)
- x := snapshot.FindFile(uri)
- if x == nil {
+ fh := snapshot.FindFile(uri)
+ if fh == nil {
continue
}
- v = filterDiagnostics(v, opts)
- reports[x.VersionedFileIdentity()] = v
+ reports[fh.VersionedFileIdentity()] = diagnostics
}
return reports, parseError
}
-func filterDiagnostics(v []*Diagnostic, o *Options) []*Diagnostic {
- var ans []*Diagnostic
- for _, x := range v {
- if x.Source != "go compiler" {
- continue
- }
- if o.Annotations["noInline"] &&
- (strings.HasPrefix(x.Message, "canInline") ||
- strings.HasPrefix(x.Message, "cannotInline") ||
- strings.HasPrefix(x.Message, "inlineCall")) {
- continue
- } else if o.Annotations["noEscape"] &&
- (strings.HasPrefix(x.Message, "escape") || x.Message == "leak") {
- continue
- } else if o.Annotations["noNilcheck"] && strings.HasPrefix(x.Message, "nilcheck") {
- continue
- } else if o.Annotations["noBounds"] &&
- (strings.HasPrefix(x.Message, "isInBounds") ||
- strings.HasPrefix(x.Message, "isSliceInBounds")) {
- continue
- }
- ans = append(ans, x)
- }
- return ans
-}
-
-func parseDetailsFile(fn string) (string, []*Diagnostic, error) {
- buf, err := ioutil.ReadFile(fn)
+func parseDetailsFile(filename string, options *Options) (span.URI, []*Diagnostic, error) {
+ buf, err := ioutil.ReadFile(filename)
if err != nil {
- return "", nil, err // This is an internal error. Likely ever file will fail.
+ return "", nil, err
}
- var fname string
- var ans []*Diagnostic
- lines := bytes.Split(buf, []byte{'\n'})
- for i, l := range lines {
- if len(l) == 0 {
- continue
- }
+ var (
+ uri span.URI
+ i int
+ diagnostics []*Diagnostic
+ )
+ type metadata struct {
+ File string `json:"file,omitempty"`
+ }
+ for dec := json.NewDecoder(bytes.NewReader(buf)); dec.More(); {
+ // The first element always contains metadata.
if i == 0 {
- x := make(map[string]interface{})
- if err := json.Unmarshal(l, &x); err != nil {
- return "", nil, fmt.Errorf("internal error (%v) parsing first line of json file %s",
- err, fn)
+ i++
+ m := new(metadata)
+ if err := dec.Decode(m); err != nil {
+ return "", nil, err
}
- fname = x["file"].(string)
+ if !strings.HasSuffix(m.File, ".go") {
+ continue // <autogenerated>
+ }
+ uri = span.URIFromPath(m.File)
continue
}
- y := protocol.Diagnostic{}
- if err := json.Unmarshal(l, &y); err != nil {
- return "", nil, fmt.Errorf("internal error (%#v) parsing json file for %s", err, fname)
+ d := new(protocol.Diagnostic)
+ if err := dec.Decode(d); err != nil {
+ return "", nil, err
}
- y.Range.Start.Line-- // change from 1-based to 0-based
- y.Range.Start.Character--
- y.Range.End.Line--
- y.Range.End.Character--
- msg := y.Code.(string)
- if y.Message != "" {
- msg = fmt.Sprintf("%s(%s)", msg, y.Message)
+ msg := d.Code.(string)
+ if msg != "" {
+ msg = fmt.Sprintf("%s(%s)", msg, d.Message)
}
- x := Diagnostic{
- Range: y.Range,
- Message: msg,
- Source: y.Source,
- Severity: y.Severity,
+ if skipDiagnostic(msg, d.Source, options) {
+ continue
}
- for _, ri := range y.RelatedInformation {
- x.Related = append(x.Related, RelatedInformation{
+ var related []RelatedInformation
+ for _, ri := range d.RelatedInformation {
+ related = append(related, RelatedInformation{
URI: ri.Location.URI.SpanURI(),
- Range: ri.Location.Range,
+ Range: zeroIndexedRange(ri.Location.Range),
Message: ri.Message,
})
}
- ans = append(ans, &x)
+ diagnostic := &Diagnostic{
+ Range: zeroIndexedRange(d.Range),
+ Message: msg,
+ Severity: d.Severity,
+ Source: d.Source,
+ Tags: d.Tags,
+ Related: related,
+ }
+ diagnostics = append(diagnostics, diagnostic)
+ i++
}
- return fname, ans, nil
+ return uri, diagnostics, nil
+}
+
+// skipDiagnostic reports whether a given diagnostic should be shown to the end
+// user, given the current options.
+func skipDiagnostic(msg, source string, o *Options) bool {
+ if source != "go compiler" {
+ return false
+ }
+ switch {
+ case o.Annotations["noInline"]:
+ return strings.HasPrefix(msg, "canInline") ||
+ strings.HasPrefix(msg, "cannotInline") ||
+ strings.HasPrefix(msg, "inlineCall")
+ case o.Annotations["noEscape"]:
+ return strings.HasPrefix(msg, "escape") || msg == "leak"
+ case o.Annotations["noNilcheck"]:
+ return strings.HasPrefix(msg, "nilcheck")
+ case o.Annotations["noBounds"]:
+ return strings.HasPrefix(msg, "isInBounds") ||
+ strings.HasPrefix(msg, "isSliceInBounds")
+ }
+ return false
+}
+
+// The range produced by the compiler is 1-indexed, so subtract range by 1.
+func zeroIndexedRange(rng protocol.Range) protocol.Range {
+ return protocol.Range{
+ Start: protocol.Position{
+ Line: rng.Start.Line - 1,
+ Character: rng.Start.Character - 1,
+ },
+ End: protocol.Position{
+ Line: rng.End.Line - 1,
+ Character: rng.End.Character - 1,
+ },
+ }
}
func findJSONFiles(dir string) ([]string, error) {
