gopls/internal/lsp/cmd/test: new integration test for gopls command

This change creates an entirely new set of integration tests
for the client-side logic of the gopls command and its subcommands.

Each test fork+execs the gopls command and makes assertions about
its exit code and stdout/stderr. The tests run in parallel (~3s).
This is not intended as a test of server-side behavior.

By decoupling the client tests from the server tests, we'll
be able to delete all the other files in this directory (in
a follow-up), allowing us to reorganize the marker-based
tests of the LSP protocol so that they can be used in
regtests.

Updates golang/go#54845

Change-Id: I5fe11849079f7cc5fe44fc50cfcfd6bbff384014
Reviewed-on: https://go-review.googlesource.com/c/tools/+/463515
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/lsp/cmd/call_hierarchy.go b/gopls/internal/lsp/cmd/call_hierarchy.go
index 0220fd2..9892fdf 100644
--- a/gopls/internal/lsp/cmd/call_hierarchy.go
+++ b/gopls/internal/lsp/cmd/call_hierarchy.go
@@ -129,13 +129,12 @@
 	}
 	var callRanges []string
 	for _, rng := range calls {
-		callSpan, err := callsFile.mapper.LocationSpan(protocol.Location{URI: item.URI, Range: rng})
+		call, err := callsFile.mapper.RangeSpan(rng)
 		if err != nil {
 			return "", err
 		}
-
-		spn := fmt.Sprint(callSpan)
-		callRanges = append(callRanges, fmt.Sprint(spn[strings.Index(spn, ":")+1:]))
+		callRange := fmt.Sprintf("%d:%d-%d", call.Start().Line(), call.Start().Column(), call.End().Column())
+		callRanges = append(callRanges, callRange)
 	}
 
 	printString := fmt.Sprintf("function %s in %v", item.Name, itemSpan)
diff --git a/gopls/internal/lsp/cmd/folding_range.go b/gopls/internal/lsp/cmd/folding_range.go
index 7a9cbf9..68d93a3 100644
--- a/gopls/internal/lsp/cmd/folding_range.go
+++ b/gopls/internal/lsp/cmd/folding_range.go
@@ -65,7 +65,7 @@
 			r.StartLine+1,
 			r.StartCharacter+1,
 			r.EndLine+1,
-			r.EndCharacter,
+			r.EndCharacter+1,
 		)
 	}
 
diff --git a/gopls/internal/lsp/cmd/test/integration_test.go b/gopls/internal/lsp/cmd/test/integration_test.go
new file mode 100644
index 0000000..7b0f852
--- /dev/null
+++ b/gopls/internal/lsp/cmd/test/integration_test.go
@@ -0,0 +1,898 @@
+// Copyright 2023 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 cmdtest
+
+// This file defines integration tests of each gopls subcommand that
+// fork+exec the command in a separate process.
+//
+// (Rather than execute 'go build gopls' during the test, we reproduce
+// the main entrypoint in the test executable.)
+//
+// The purpose of this test is to exercise client-side logic such as
+// argument parsing and formatting of LSP RPC responses, not server
+// behavior; see lsp_test for that.
+//
+// All tests run in parallel.
+//
+// TODO(adonovan):
+// - Use markers to represent positions in the input and in assertions.
+// - Coverage of cross-cutting things like cwd, enviro, span parsing, etc.
+// - Subcommands that accept -write and -diff flags should implement
+//   them consistently wrt the default behavior; factor their tests.
+// - Add missing test for 'vulncheck' subcommand.
+// - Add tests for client-only commands: serve, bug, help, api-json, licenses.
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	exec "golang.org/x/sys/execabs"
+	"golang.org/x/tools/gopls/internal/hooks"
+	"golang.org/x/tools/gopls/internal/lsp/cmd"
+	"golang.org/x/tools/gopls/internal/lsp/debug"
+	"golang.org/x/tools/gopls/internal/lsp/protocol"
+	"golang.org/x/tools/internal/bug"
+	"golang.org/x/tools/internal/testenv"
+	"golang.org/x/tools/internal/tool"
+	"golang.org/x/tools/txtar"
+)
+
+// TestVersion tests the 'version' subcommand (../info.go).
+func TestVersion(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, "")
+
+	// There's not much we can robustly assert about the actual version.
+	const want = debug.Version // e.g. "master"
+
+	// basic
+	{
+		res := gopls(t, tree, "version")
+		res.checkExit(0)
+		res.checkStdout(want)
+	}
+
+	// -json flag
+	{
+		res := gopls(t, tree, "version", "-json")
+		res.checkExit(0)
+		var v debug.ServerVersion
+		if res.toJSON(&v) {
+			if v.Version != want {
+				t.Errorf("expected Version %q, got %q (%v)", want, v.Version, res)
+			}
+		}
+	}
+}
+
+// TestCheck tests the 'check' subcommand (../check.go).
+func TestCheck(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+import "fmt"
+var _ = fmt.Sprintf("%s", 123)
+
+-- b.go --
+package a
+import "fmt"
+var _ = fmt.Sprintf("%d", "123")
+`)
+
+	// no files
+	{
+		res := gopls(t, tree, "check")
+		res.checkExit(0)
+		if res.stdout != "" {
+			t.Errorf("unexpected output: %v", res)
+		}
+	}
+
+	// one file
+	{
+		res := gopls(t, tree, "check", "./a.go")
+		res.checkExit(0)
+		res.checkStdout("fmt.Sprintf format %s has arg 123 of wrong type int")
+	}
+
+	// two files
+	{
+		res := gopls(t, tree, "check", "./a.go", "./b.go")
+		res.checkExit(0)
+		res.checkStdout(`a.go:.* fmt.Sprintf format %s has arg 123 of wrong type int`)
+		res.checkStdout(`b.go:.* fmt.Sprintf format %d has arg "123" of wrong type string`)
+	}
+}
+
+// TestCallHierarchy tests the 'call_hierarchy' subcommand (../call_hierarchy.go).
+func TestCallHierarchy(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func f() {}
+func g() {
+	f()
+}
+func h() {
+	f()
+	f()
+}
+`)
+	// missing position
+	{
+		res := gopls(t, tree, "call_hierarchy")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// wrong place
+	{
+		res := gopls(t, tree, "call_hierarchy", "a.go:1")
+		res.checkExit(2)
+		res.checkStderr("identifier not found")
+	}
+	// f is called once from g and twice from h.
+	{
+		res := gopls(t, tree, "call_hierarchy", "a.go:2:6")
+		res.checkExit(0)
+		// We use regexp '.' as an OS-agnostic path separator.
+		res.checkStdout("ranges 7:2-3, 8:2-3 in ..a.go from/to function h in ..a.go:6:6-7")
+		res.checkStdout("ranges 4:2-3 in ..a.go from/to function g in ..a.go:3:6-7")
+		res.checkStdout("identifier: function f in ..a.go:2:6-7")
+	}
+}
+
+// TestDefinition tests the 'definition' subcommand (../definition.go).
+func TestDefinition(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+import "fmt"
+func f() {
+	fmt.Println()
+}
+func g() {
+	f()
+}
+`)
+	// missing position
+	{
+		res := gopls(t, tree, "definition")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// intra-package
+	{
+		res := gopls(t, tree, "definition", "a.go:7:2") // "f()"
+		res.checkExit(0)
+		res.checkStdout("a.go:3:6-7: defined here as func f")
+	}
+	// cross-package
+	{
+		res := gopls(t, tree, "definition", "a.go:4:7") // "Println"
+		res.checkExit(0)
+		res.checkStdout("print.go.* defined here as func fmt.Println")
+		res.checkStdout("Println formats using the default formats for its operands")
+	}
+	// -json and -markdown
+	{
+		res := gopls(t, tree, "definition", "-json", "-markdown", "a.go:4:7")
+		res.checkExit(0)
+		var defn cmd.Definition
+		if res.toJSON(&defn) {
+			if !strings.HasPrefix(defn.Description, "```go\nfunc fmt.Println") {
+				t.Errorf("Description does not start with markdown code block. Got: %s", defn.Description)
+			}
+		}
+	}
+}
+
+// TestFoldingRanges tests the 'folding_ranges' subcommand (../folding_range.go).
+func TestFoldingRanges(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func f(x int) {
+	// hello
+}
+`)
+	// missing filename
+	{
+		res := gopls(t, tree, "folding_ranges")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// success
+	{
+		res := gopls(t, tree, "folding_ranges", "a.go")
+		res.checkExit(0)
+		res.checkStdout("2:8-2:13") // params (x int)
+		res.checkStdout("2:16-4:1") //   body { ... }
+	}
+}
+
+// TestFormat tests the 'format' subcommand (../format.go).
+func TestFormat(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- a.go --
+package a ;  func f ( ) { }
+`)
+	const want = `package a
+
+func f() {}
+`
+
+	// no files => nop
+	{
+		res := gopls(t, tree, "format")
+		res.checkExit(0)
+	}
+	// default => print formatted result
+	{
+		res := gopls(t, tree, "format", "a.go")
+		res.checkExit(0)
+		if res.stdout != want {
+			t.Errorf("format: got <<%s>>, want <<%s>>", res.stdout, want)
+		}
+	}
+	// start/end position not supported (unless equal to start/end of file)
+	{
+		res := gopls(t, tree, "format", "a.go:1-2")
+		res.checkExit(2)
+		res.checkStderr("only full file formatting supported")
+	}
+	// -list: show only file names
+	{
+		res := gopls(t, tree, "format", "-list", "a.go")
+		res.checkExit(0)
+		res.checkStdout("a.go")
+	}
+	// -diff prints a unified diff
+	{
+		res := gopls(t, tree, "format", "-diff", "a.go")
+		res.checkExit(0)
+		// We omit the filenames as they vary by OS.
+		want := `
+-package a ;  func f ( ) { }
++package a
++
++func f() {}
+`
+		res.checkStdout(regexp.QuoteMeta(want))
+	}
+	// -write updates the file
+	{
+		res := gopls(t, tree, "format", "-write", "a.go")
+		res.checkExit(0)
+		res.checkStdout("^$") // empty
+		checkContent(t, filepath.Join(tree, "a.go"), want)
+	}
+}
+
+// TestHighlight tests the 'highlight' subcommand (../highlight.go).
+func TestHighlight(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- a.go --
+package a
+import "fmt"
+func f() {
+	fmt.Println()
+	fmt.Println()
+}
+`)
+
+	// no arguments
+	{
+		res := gopls(t, tree, "highlight")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// all occurrences of Println
+	{
+		res := gopls(t, tree, "highlight", "a.go:4:7")
+		res.checkExit(0)
+		res.checkStdout("a.go:4:6-13")
+		res.checkStdout("a.go:5:6-13")
+	}
+}
+
+// TestImplementations tests the 'implementation' subcommand (../implementation.go).
+func TestImplementations(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- a.go --
+package a
+import "fmt"
+type T int
+func (T) String() string { return "" }
+`)
+
+	// no arguments
+	{
+		res := gopls(t, tree, "implementation")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// T.String
+	{
+		res := gopls(t, tree, "implementation", "a.go:4:10")
+		res.checkExit(0)
+		// TODO(adonovan): extract and check the content of the reported ranges?
+		// We use regexp '.' as an OS-agnostic path separator.
+		res.checkStdout("fmt.print.go:")     // fmt.Stringer.String
+		res.checkStdout("runtime.error.go:") // runtime.stringer.String
+	}
+}
+
+// TestImports tests the 'imports' subcommand (../imports.go).
+func TestImports(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- a.go --
+package a
+func _() {
+	fmt.Println()
+}
+`)
+
+	want := `
+package a
+
+import "fmt"
+func _() {
+	fmt.Println()
+}
+`[1:]
+
+	// no arguments
+	{
+		res := gopls(t, tree, "imports")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// default: print with imports
+	{
+		res := gopls(t, tree, "imports", "a.go")
+		res.checkExit(0)
+		if res.stdout != want {
+			t.Errorf("format: got <<%s>>, want <<%s>>", res.stdout, want)
+		}
+	}
+	// -diff: show a unified diff
+	{
+		res := gopls(t, tree, "imports", "-diff", "a.go")
+		res.checkExit(0)
+		res.checkStdout(regexp.QuoteMeta(`+import "fmt"`))
+	}
+	// -write: update file
+	{
+		res := gopls(t, tree, "imports", "-write", "a.go")
+		res.checkExit(0)
+		checkContent(t, filepath.Join(tree, "a.go"), want)
+	}
+}
+
+// TestLinks tests the 'links' subcommand (../links.go).
+func TestLinks(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- a.go --
+// Link in package doc: https://pkg.go.dev/
+package a
+
+// Link in internal comment: https://go.dev/cl
+
+// Doc comment link: https://blog.go.dev/
+func f() {}
+`)
+	// no arguments
+	{
+		res := gopls(t, tree, "links")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// success
+	{
+		res := gopls(t, tree, "links", "a.go")
+		res.checkExit(0)
+		res.checkStdout("https://go.dev/cl")
+		res.checkStdout("https://pkg.go.dev")
+		res.checkStdout("https://blog.go.dev/")
+	}
+	// -json
+	{
+		res := gopls(t, tree, "links", "-json", "a.go")
+		res.checkExit(0)
+		res.checkStdout("https://pkg.go.dev")
+		res.checkStdout("https://go.dev/cl")
+		res.checkStdout("https://blog.go.dev/") // at 5:21-5:41
+		var links []protocol.DocumentLink
+		if res.toJSON(&links) {
+			// Check just one of the three locations.
+			if got, want := fmt.Sprint(links[2].Range), "5:21-5:41"; got != want {
+				t.Errorf("wrong link location: got %v, want %v", got, want)
+			}
+		}
+	}
+}
+
+// TestReferences tests the 'references' subcommand (../references.go).
+func TestReferences(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+import "fmt"
+func f() {
+	fmt.Println()
+}
+
+-- b.go --
+package a
+import "fmt"
+func g() {
+	fmt.Println()
+}
+`)
+	// no arguments
+	{
+		res := gopls(t, tree, "references")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// fmt.Println
+	{
+		res := gopls(t, tree, "references", "a.go:4:10")
+		res.checkExit(0)
+		res.checkStdout("a.go:4:6-13")
+		res.checkStdout("b.go:4:6-13")
+	}
+}
+
+// TestSignature tests the 'signature' subcommand (../signature.go).
+func TestSignature(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+import "fmt"
+func f() {
+	fmt.Println(123)
+}
+`)
+	// no arguments
+	{
+		res := gopls(t, tree, "signature")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// at 123 inside fmt.Println() call
+	{
+		res := gopls(t, tree, "signature", "a.go:4:15")
+		res.checkExit(0)
+		res.checkStdout("Println\\(a ...")
+		res.checkStdout("Println formats using the default formats...")
+	}
+}
+
+// TestPrepareRename tests the 'prepare_rename' subcommand (../prepare_rename.go).
+func TestPrepareRename(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func oldname() {}
+`)
+	// no arguments
+	{
+		res := gopls(t, tree, "prepare_rename")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// in 'package' keyword
+	{
+		res := gopls(t, tree, "prepare_rename", "a.go:1:3")
+		res.checkExit(2)
+		res.checkStderr("request is not valid at the given position")
+	}
+	// in 'package' identifier (not supported by client)
+	{
+		res := gopls(t, tree, "prepare_rename", "a.go:1:9")
+		res.checkExit(2)
+		res.checkStderr("can't rename package")
+	}
+	// in func oldname
+	{
+		res := gopls(t, tree, "prepare_rename", "a.go:2:9")
+		res.checkExit(0)
+		res.checkStdout("a.go:2:6-13") // all of "oldname"
+	}
+}
+
+// TestRename tests the 'rename' subcommand (../rename.go).
+func TestRename(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func oldname() {}
+`)
+	// no arguments
+	{
+		res := gopls(t, tree, "rename")
+		res.checkExit(2)
+		res.checkStderr("expects 2 arguments")
+	}
+	// missing newname
+	{
+		res := gopls(t, tree, "rename", "a.go:1:3")
+		res.checkExit(2)
+		res.checkStderr("expects 2 arguments")
+	}
+	// in 'package' keyword
+	{
+		res := gopls(t, tree, "rename", "a.go:1:3", "newname")
+		res.checkExit(2)
+		res.checkStderr("no object found")
+	}
+	// in 'package' identifier
+	{
+		res := gopls(t, tree, "rename", "a.go:1:9", "newname")
+		res.checkExit(2)
+		res.checkStderr(`cannot rename package: module path .* same as the package path, so .* no effect`)
+	}
+	// success, func oldname (and -diff)
+	{
+		res := gopls(t, tree, "rename", "-diff", "a.go:2:9", "newname")
+		res.checkExit(0)
+		res.checkStdout(regexp.QuoteMeta("-func oldname() {}"))
+		res.checkStdout(regexp.QuoteMeta("+func newname() {}"))
+	}
+}
+
+// TestSymbols tests the 'symbols' subcommand (../symbols.go).
+func TestSymbols(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func f()
+var v int
+const c = 0
+`)
+	// no files
+	{
+		res := gopls(t, tree, "symbols")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// success
+	{
+		res := gopls(t, tree, "symbols", "a.go:123:456") // (line/col ignored)
+		res.checkExit(0)
+		res.checkStdout("f Function 2:6-2:7")
+		res.checkStdout("v Variable 3:5-3:6")
+		res.checkStdout("c Constant 4:7-4:8")
+	}
+}
+
+// TestSemtok tests the 'semtok' subcommand (../semantictokens.go).
+func TestSemtok(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func f()
+var v int
+const c = 0
+`)
+	// no files
+	{
+		res := gopls(t, tree, "semtok")
+		res.checkExit(2)
+		res.checkStderr("expected one file name")
+	}
+	// success
+	{
+		res := gopls(t, tree, "semtok", "a.go")
+		res.checkExit(0)
+		got := res.stdout
+		want := `
+/*⇒7,keyword,[]*/package /*⇒1,namespace,[]*/a
+/*⇒4,keyword,[]*/func /*⇒1,function,[definition]*/f()
+/*⇒3,keyword,[]*/var /*⇒1,variable,[definition]*/v /*⇒3,type,[defaultLibrary]*/int
+/*⇒5,keyword,[]*/const /*⇒1,variable,[definition readonly]*/c = /*⇒1,number,[]*/0
+`[1:]
+		if got != want {
+			t.Errorf("semtok: got <<%s>>, want <<%s>>", got, want)
+		}
+	}
+}
+
+// TestFix tests the 'fix' subcommand (../suggested_fix.go).
+func TestFix(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+var _ error = T(0)
+type T int
+func f() (int, string) { return }
+`)
+	want := `
+package a
+var _ error = T(0)
+type T int
+func f() (int, string) { return 0, "" }
+`[1:]
+
+	// no arguments
+	{
+		res := gopls(t, tree, "fix")
+		res.checkExit(2)
+		res.checkStderr("expects at least 1 argument")
+	}
+	// success (-a enables fillreturns)
+	{
+		res := gopls(t, tree, "fix", "-a", "a.go")
+		res.checkExit(0)
+		got := res.stdout
+		if got != want {
+			t.Errorf("fix: got <<%s>>, want <<%s>>", got, want)
+		}
+	}
+	// TODO(adonovan): more tests:
+	// - -write, -diff: factor with imports, format, rename.
+	// - without -all flag
+	// - args[2:] is an optional list of protocol.CodeActionKind enum values.
+	// - a span argument with a range causes filtering.
+}
+
+// TestWorkspaceSymbol tests the 'workspace_symbol' subcommand (../workspace_symbol.go).
+func TestWorkspaceSymbol(t *testing.T) {
+	t.Parallel()
+
+	tree := writeTree(t, `
+-- go.mod --
+module example.com
+go 1.18
+
+-- a.go --
+package a
+func someFunctionName()
+`)
+	// no files
+	{
+		res := gopls(t, tree, "workspace_symbol")
+		res.checkExit(2)
+		res.checkStderr("expects 1 argument")
+	}
+	// success
+	{
+		res := gopls(t, tree, "workspace_symbol", "meFun")
+		res.checkExit(0)
+		res.checkStdout("a.go:2:6-22 someFunctionName Function")
+	}
+}
+
+// -- test framework --
+
+func TestMain(m *testing.M) {
+	switch os.Getenv("ENTRYPOINT") {
+	case "goplsMain":
+		goplsMain()
+	default:
+		os.Exit(m.Run())
+	}
+}
+
+// This function is a stand-in for gopls.main in ../../../../main.go.
+func goplsMain() {
+	bug.PanicOnBugs = true // (not in the production command)
+	tool.Main(context.Background(), cmd.New("gopls", "", nil, hooks.Options), os.Args[1:])
+}
+
+// writeTree extracts a txtar archive into a new directory and returns its path.
+func writeTree(t *testing.T, archive string) string {
+	root := t.TempDir()
+
+	// This unfortunate step is required because gopls output
+	// expands symbolic links it its input file names (arguably it
+	// should not), and on macOS the temp dir is in /var -> private/var.
+	root, err := filepath.EvalSymlinks(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, f := range txtar.Parse([]byte(archive)).Files {
+		filename := filepath.Join(root, f.Name)
+		if err := os.MkdirAll(filepath.Dir(filename), 0777); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filename, f.Data, 0666); err != nil {
+			t.Fatal(err)
+		}
+	}
+	return root
+}
+
+// gopls executes gopls in a child process.
+func gopls(t *testing.T, dir string, args ...string) *result {
+	testenv.NeedsTool(t, "go")
+
+	// Catch inadvertent use of dir=".", which would make
+	// the ReplaceAll below unpredictable.
+	if !filepath.IsAbs(dir) {
+		t.Fatalf("dir is not absolute: %s", dir)
+	}
+
+	cmd := exec.Command(os.Args[0], args...)
+	cmd.Env = append(os.Environ(), "ENTRYPOINT=goplsMain")
+	cmd.Dir = dir
+	cmd.Stdout = new(bytes.Buffer)
+	cmd.Stderr = new(bytes.Buffer)
+
+	cmdErr := cmd.Run()
+
+	stdout := strings.ReplaceAll(fmt.Sprint(cmd.Stdout), dir, ".")
+	stderr := strings.ReplaceAll(fmt.Sprint(cmd.Stderr), dir, ".")
+	exitcode := 0
+	if cmdErr != nil {
+		if exitErr, ok := cmdErr.(*exec.ExitError); ok {
+			exitcode = exitErr.ExitCode()
+		} else {
+			stderr = cmdErr.Error() // (execve failure)
+			exitcode = -1
+		}
+	}
+	res := &result{
+		t:        t,
+		command:  "gopls " + strings.Join(args, " "),
+		exitcode: exitcode,
+		stdout:   stdout,
+		stderr:   stderr,
+	}
+	if false {
+		t.Log(res)
+	}
+	return res
+}
+
+// A result holds the result of a gopls invocation, and provides assertion helpers.
+type result struct {
+	t              *testing.T
+	command        string
+	exitcode       int
+	stdout, stderr string
+}
+
+func (res *result) String() string {
+	return fmt.Sprintf("%s: exit=%d stdout=<<%s>> stderr=<<%s>>",
+		res.command, res.exitcode, res.stdout, res.stderr)
+}
+
+// checkExit asserts that gopls returned the expected exit code.
+func (res *result) checkExit(code int) {
+	res.t.Helper()
+	if res.exitcode != code {
+		res.t.Errorf("%s: exited with code %d, want %d (%s)",
+			res.command, res.exitcode, code, res)
+	}
+}
+
+// checkStdout asserts that the gopls standard output matches the pattern.
+func (res *result) checkStdout(pattern string) {
+	res.t.Helper()
+	res.checkOutput(pattern, "stdout", res.stdout)
+}
+
+// checkStderr asserts that the gopls standard error matches the pattern.
+func (res *result) checkStderr(pattern string) {
+	res.t.Helper()
+	res.checkOutput(pattern, "stderr", res.stderr)
+}
+
+func (res *result) checkOutput(pattern, name, content string) {
+	res.t.Helper()
+	if match, err := regexp.MatchString(pattern, content); err != nil {
+		res.t.Errorf("invalid regexp: %v", err)
+	} else if !match {
+		res.t.Errorf("%s: %s does not match [%s]; got <<%s>>",
+			res.command, name, pattern, content)
+	}
+}
+
+// toJSON decodes res.stdout as JSON into to *ptr and reports its success.
+func (res *result) toJSON(ptr interface{}) bool {
+	if err := json.Unmarshal([]byte(res.stdout), ptr); err != nil {
+		res.t.Errorf("invalid JSON %v", err)
+		return false
+	}
+	return true
+}
+
+// checkContent checks that the contents of the file are as expected.
+func checkContent(t *testing.T, filename, want string) {
+	data, err := os.ReadFile(filename)
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	if got := string(data); got != want {
+		t.Errorf("content of %s is <<%s>>, want <<%s>>", filename, got, want)
+	}
+}
diff --git a/gopls/internal/lsp/signature_help.go b/gopls/internal/lsp/signature_help.go
index 8a343fb..b623f78 100644
--- a/gopls/internal/lsp/signature_help.go
+++ b/gopls/internal/lsp/signature_help.go
@@ -7,10 +7,10 @@
 import (
 	"context"
 
-	"golang.org/x/tools/internal/event"
-	"golang.org/x/tools/internal/event/tag"
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 	"golang.org/x/tools/gopls/internal/lsp/source"
+	"golang.org/x/tools/internal/event"
+	"golang.org/x/tools/internal/event/tag"
 )
 
 func (s *Server) signatureHelp(ctx context.Context, params *protocol.SignatureHelpParams) (*protocol.SignatureHelp, error) {
@@ -22,7 +22,7 @@
 	info, activeParameter, err := source.SignatureHelp(ctx, snapshot, fh, params.Position)
 	if err != nil {
 		event.Error(ctx, "no signature help", err, tag.Position.Of(params.Position))
-		return nil, nil
+		return nil, nil // sic? There could be many reasons for failure.
 	}
 	return &protocol.SignatureHelp{
 		Signatures:      []protocol.SignatureInformation{*info},
diff --git a/gopls/internal/lsp/testdata/folding/a.go.golden b/gopls/internal/lsp/testdata/folding/a.go.golden
index ce69102..23befa7 100644
--- a/gopls/internal/lsp/testdata/folding/a.go.golden
+++ b/gopls/internal/lsp/testdata/folding/a.go.golden
@@ -247,41 +247,41 @@
 }
 
 -- foldingRange-cmd --
-3:9-6:0
-10:22-11:32
-12:10-12:9
-12:20-75:0
-14:10-25:1
-15:12-20:3
-16:12-18:2
-17:16-17:21
-18:11-20:2
-19:16-19:22
-21:13-22:22
-22:15-22:21
-23:10-24:24
-24:15-24:23
-26:24-28:11
-30:24-33:32
-34:12-38:1
-39:16-41:1
-42:21-46:1
-47:17-51:1
-52:8-56:1
-57:15-57:23
-57:32-57:40
-58:10-69:1
-59:18-64:3
-60:11-62:2
-61:16-61:28
-62:11-64:2
-63:16-63:29
-65:11-66:18
-66:15-66:17
-67:10-68:24
-68:15-68:23
-70:32-71:30
-72:9-74:16
+3:9-6:1
+10:22-11:33
+12:10-12:10
+12:20-75:1
+14:10-25:2
+15:12-20:4
+16:12-18:3
+17:16-17:22
+18:11-20:3
+19:16-19:23
+21:13-22:23
+22:15-22:22
+23:10-24:25
+24:15-24:24
+26:24-28:12
+30:24-33:33
+34:12-38:2
+39:16-41:2
+42:21-46:2
+47:17-51:2
+52:8-56:2
+57:15-57:24
+57:32-57:41
+58:10-69:2
+59:18-64:4
+60:11-62:3
+61:16-61:29
+62:11-64:3
+63:16-63:30
+65:11-66:19
+66:15-66:18
+67:10-68:25
+68:15-68:24
+70:32-71:31
+72:9-74:17
 
 -- foldingRange-comment-0 --
 package folding //@fold("package")
diff --git a/gopls/internal/lsp/testdata/folding/bad.go.golden b/gopls/internal/lsp/testdata/folding/bad.go.golden
index d1bdfec..6db6982 100644
--- a/gopls/internal/lsp/testdata/folding/bad.go.golden
+++ b/gopls/internal/lsp/testdata/folding/bad.go.golden
@@ -45,14 +45,14 @@
 }
 
 -- foldingRange-cmd --
-3:9-5:0
-7:9-8:8
-11:13-11:12
-11:23-18:0
-12:8-15:1
-14:15-14:20
-15:10-16:23
-16:15-16:21
+3:9-5:1
+7:9-8:9
+11:13-11:13
+11:23-18:1
+12:8-15:2
+14:15-14:21
+15:10-16:24
+16:15-16:22
 
 -- foldingRange-imports-0 --
 package folding //@fold("package")