internal/lsp/source: don't crash requesting gc_details for an empty file

Requesting gc_details for an empty file never worked, but in the past it would inadvertently get handled by our forgiving position helpers. With https://go.dev/cl/409935, it became a crasher. Fix the crash by adding handling when the package name position is invalid. Also move gc_details related codelens to their own file. Fixes golang/go#54199 Change-Id: I7339b3903abd1315f1fea3dd9929d81855a8264a Reviewed-on: https://go-review.googlesource.com/c/tools/+/420715 gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Suzy Mueller <suzmue@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-08-02 10:32:11 -04:00 · 2022-08-02 10:32:11 -04:00 · d025cced83
parent 10cb4353f9
commit d025cced83
3 changed files with 141 additions and 71 deletions
--- a/gopls/internal/regtest/codelens/codelens_test.go
+++ b/gopls/internal/regtest/codelens/codelens_test.go
@ -6,8 +6,6 @@ package codelens

 import (
 	"fmt"
-	"runtime"
-	"strings"
 	"testing"

 	"golang.org/x/tools/gopls/internal/hooks"
@ -15,7 +13,6 @@ import (
 	. "golang.org/x/tools/internal/lsp/regtest"

 	"golang.org/x/tools/internal/lsp/command"
-	"golang.org/x/tools/internal/lsp/fake"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/tests"
 	"golang.org/x/tools/internal/testenv"
@ -284,71 +281,3 @@ func Foo() {
 		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() {
-	fmt.Println(42)
-}
-`
-	WithOptions(
-		Settings{
-			"codelenses": map[string]bool{
-				"gc_details": true,
-			},
-		},
-	).Run(t, mod, func(t *testing.T, env *Env) {
-		env.OpenFile("main.go")
-		env.ExecuteCodeLensCommand("main.go", command.GCDetails)
-		d := &protocol.PublishDiagnosticsParams{}
-		env.Await(
-			OnceMet(
-				DiagnosticAt("main.go", 5, 13),
-				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, "42 escapes") {
-				found = true
-			}
-		}
-		if !found {
-			t.Fatalf(`expected to find diagnostic with message "escape(42 escapes to heap)", found none`)
-		}
-
-		// Editing a buffer should cause gc_details diagnostics to disappear, since
-		// they only apply to saved buffers.
-		env.EditBuffer("main.go", fake.NewEdit(0, 0, 0, 0, "\n\n"))
-		env.Await(EmptyDiagnostics("main.go"))
-
-		// Saving a buffer should re-format back to the original state, and
-		// re-enable the gc_details diagnostics.
-		env.SaveBuffer("main.go")
-		env.Await(DiagnosticAt("main.go", 5, 13))
-
-		// Toggle the GC details code lens again so now it should be off.
-		env.ExecuteCodeLensCommand("main.go", command.GCDetails)
-		env.Await(
-			EmptyDiagnostics("main.go"),
-		)
-	})
-}
--- a/gopls/internal/regtest/codelens/gcdetails_test.go
+++ b/gopls/internal/regtest/codelens/gcdetails_test.go
@ -0,0 +1,137 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package codelens
+
+import (
+	"runtime"
+	"strings"
+	"testing"
+
+	"golang.org/x/tools/internal/testenv"
+
+	"golang.org/x/tools/internal/lsp/bug"
+	"golang.org/x/tools/internal/lsp/command"
+	"golang.org/x/tools/internal/lsp/fake"
+	"golang.org/x/tools/internal/lsp/protocol"
+	. "golang.org/x/tools/internal/lsp/regtest"
+)
+
+func TestGCDetails_Toggle(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() {
+	fmt.Println(42)
+}
+`
+	WithOptions(
+		Settings{
+			"codelenses": map[string]bool{
+				"gc_details": true,
+			},
+		},
+	).Run(t, mod, func(t *testing.T, env *Env) {
+		env.OpenFile("main.go")
+		env.ExecuteCodeLensCommand("main.go", command.GCDetails)
+		d := &protocol.PublishDiagnosticsParams{}
+		env.Await(
+			OnceMet(
+				DiagnosticAt("main.go", 5, 13),
+				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, "42 escapes") {
+				found = true
+			}
+		}
+		if !found {
+			t.Fatalf(`expected to find diagnostic with message "escape(42 escapes to heap)", found none`)
+		}
+
+		// Editing a buffer should cause gc_details diagnostics to disappear, since
+		// they only apply to saved buffers.
+		env.EditBuffer("main.go", fake.NewEdit(0, 0, 0, 0, "\n\n"))
+		env.Await(EmptyDiagnostics("main.go"))
+
+		// Saving a buffer should re-format back to the original state, and
+		// re-enable the gc_details diagnostics.
+		env.SaveBuffer("main.go")
+		env.Await(DiagnosticAt("main.go", 5, 13))
+
+		// Toggle the GC details code lens again so now it should be off.
+		env.ExecuteCodeLensCommand("main.go", command.GCDetails)
+		env.Await(
+			EmptyDiagnostics("main.go"),
+		)
+	})
+}
+
+// Test for the crasher in golang/go#54199
+func TestGCDetails_NewFile(t *testing.T) {
+	bug.PanicOnBugs = false
+	// It appears that older Go versions don't even see p.go from the initial
+	// workspace load.
+	testenv.NeedsGo1Point(t, 15)
+	const src = `
+-- go.mod --
+module mod.test
+
+go 1.12
+`
+
+	WithOptions(
+		Settings{
+			"codelenses": map[string]bool{
+				"gc_details": true,
+			},
+		},
+	).Run(t, src, func(t *testing.T, env *Env) {
+		env.CreateBuffer("p_test.go", "")
+
+		const gcDetailsCommand = "gopls." + string(command.GCDetails)
+
+		hasGCDetails := func() bool {
+			lenses := env.CodeLens("p_test.go") // should not crash
+			for _, lens := range lenses {
+				if lens.Command.Command == gcDetailsCommand {
+					return true
+				}
+			}
+			return false
+		}
+
+		// With an empty file, we shouldn't get the gc_details codelens because
+		// there is nowhere to position it (it needs a package name).
+		if hasGCDetails() {
+			t.Errorf("got the gc_details codelens for an empty file")
+		}
+
+		// Edit to provide a package name.
+		env.EditBuffer("p_test.go", fake.NewEdit(0, 0, 0, 0, "package p"))
+
+		// Now we should get the gc_details codelens.
+		if !hasGCDetails() {
+			t.Errorf("didn't get the gc_details codelens for a valid non-empty Go file")
+		}
+	})
+}
--- a/internal/lsp/source/code_lens.go
+++ b/internal/lsp/source/code_lens.go
@ -231,6 +231,10 @@ func toggleDetailsCodeLens(ctx context.Context, snapshot Snapshot, fh FileHandle
 	if err != nil {
 		return nil, err
 	}
+	if !pgf.File.Package.IsValid() {
+		// Without a package name we have nowhere to put the codelens, so give up.
+		return nil, nil
+	}
 	rng, err := NewMappedRange(pgf.Tok, pgf.Mapper, pgf.File.Package, pgf.File.Package).Range()
 	if err != nil {
 		return nil, err