gopls/internal/lsp/regtest: add @suggestedfix marker

This change adds a suggestedfix marker type similar to the one
in the LSP tests. Like @diag, it creates an expectation of a
diagnostic, which it then consumes; but it goes on to apply
the suggested fix of the specified kind and assert (like a
@rename marker) that the files were edited correctly.

Porting the existing test cases (and adding suggestedfixerr) will
be done in a follow-up.

Also:
- define an OnApplyEdit client hook that intercepts server downcalls.
  The marker test uses this to capture edits that occur as a side
  effect of an RPC (such as the ExecuteCommand of a suggestedfix)
  rather than the result of an RPC (such as rename).
  The existing main regtest runner does not use the hook.
  (Can we avoid this subtlety?)
- fix two latent "loopclosure" bugs

Change-Id: I5b352f8108f630b4be46aa1d25fd9a88938448ad
Reviewed-on: https://go-review.googlesource.com/c/tools/+/470677
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>

13 files changed, 232 insertions, 58 deletions

diff --git a/gopls/internal/lsp/code_action.go b/gopls/internal/lsp/code_action.go
index 230cdd830..3864648bc 100644
--- a/gopls/internal/lsp/code_action.go
+++ b/gopls/internal/lsp/code_action.go
@@ -418,17 +418,7 @@ func codeActionsForDiagnostic(ctx context.Context, snapshot source.Snapshot, sd
 			if err != nil {
 				return nil, err
 			}
-			changes = append(changes, protocol.DocumentChanges{
-				TextDocumentEdit: &protocol.TextDocumentEdit{
-					TextDocument: protocol.OptionalVersionedTextDocumentIdentifier{
-						Version: fh.Version(),
-						TextDocumentIdentifier: protocol.TextDocumentIdentifier{
-							URI: protocol.URIFromSpanURI(fh.URI()),
-						},
-					},
-					Edits: edits,
-				},
-			})
+			changes = append(changes, documentChanges(fh, edits)...)
 		}
 		action := protocol.CodeAction{
 			Title: fix.Title,
diff --git a/gopls/internal/lsp/command.go b/gopls/internal/lsp/command.go
index e6ec67f6a..75e5ef8b5 100644
--- a/gopls/internal/lsp/command.go
+++ b/gopls/internal/lsp/command.go
@@ -157,6 +157,7 @@ func (c *commandHandler) ApplyFix(ctx context.Context, args command.ApplyFixArgs
 		}
 		var changes []protocol.DocumentChanges
 		for _, edit := range edits {
+			edit := edit
 			changes = append(changes, protocol.DocumentChanges{
 				TextDocumentEdit: &edit,
 			})
@@ -588,6 +589,7 @@ func (s *Server) runGoModUpdateCommands(ctx context.Context, snapshot source.Sna
 	}
 	var documentChanges []protocol.DocumentChanges
 	for _, change := range changes {
+		change := change
 		documentChanges = append(documentChanges, protocol.DocumentChanges{
 			TextDocumentEdit: &change,
 		})
diff --git a/gopls/internal/lsp/fake/client.go b/gopls/internal/lsp/fake/client.go
index d6c886f17..bb9bda0fa 100644
--- a/gopls/internal/lsp/fake/client.go
+++ b/gopls/internal/lsp/fake/client.go
@@ -13,7 +13,10 @@ import (
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 )
 
-// ClientHooks are called to handle the corresponding client LSP method.
+// ClientHooks are a set of optional hooks called during handling of
+// the corresponding client method (see protocol.Client for the the
+// LSP server-to-client RPCs) in order to make test expectations
+// awaitable.
 type ClientHooks struct {
 	OnLogMessage             func(context.Context, *protocol.LogMessageParams) error
 	OnDiagnostics            func(context.Context, *protocol.PublishDiagnosticsParams) error
@@ -21,15 +24,17 @@ type ClientHooks struct {
 	OnProgress               func(context.Context, *protocol.ProgressParams) error
 	OnShowMessage            func(context.Context, *protocol.ShowMessageParams) error
 	OnShowMessageRequest     func(context.Context, *protocol.ShowMessageRequestParams) error
-	OnRegistration           func(context.Context, *protocol.RegistrationParams) error
-	OnUnregistration         func(context.Context, *protocol.UnregistrationParams) error
+	OnRegisterCapability     func(context.Context, *protocol.RegistrationParams) error
+	OnUnregisterCapability   func(context.Context, *protocol.UnregistrationParams) error
+	OnApplyEdit              func(context.Context, *protocol.ApplyWorkspaceEditParams) error
 }
 
-// Client is an adapter that converts an *Editor into an LSP Client. It mosly
+// Client is an adapter that converts an *Editor into an LSP Client. It mostly
 // delegates functionality to hooks that can be configured by tests.
 type Client struct {
-	editor *Editor
-	hooks  ClientHooks
+	editor         *Editor
+	hooks          ClientHooks
+	skipApplyEdits bool // don't apply edits from ApplyEdit downcalls to Editor
 }
 
 func (c *Client) CodeLensRefresh(context.Context) error { return nil }
@@ -98,8 +103,8 @@ func (c *Client) Configuration(_ context.Context, p *protocol.ParamConfiguration
 }
 
 func (c *Client) RegisterCapability(ctx context.Context, params *protocol.RegistrationParams) error {
-	if c.hooks.OnRegistration != nil {
-		if err := c.hooks.OnRegistration(ctx, params); err != nil {
+	if c.hooks.OnRegisterCapability != nil {
+		if err := c.hooks.OnRegisterCapability(ctx, params); err != nil {
 			return err
 		}
 	}
@@ -138,8 +143,8 @@ func (c *Client) RegisterCapability(ctx context.Context, params *protocol.Regist
 }
 
 func (c *Client) UnregisterCapability(ctx context.Context, params *protocol.UnregistrationParams) error {
-	if c.hooks.OnUnregistration != nil {
-		return c.hooks.OnUnregistration(ctx, params)
+	if c.hooks.OnUnregisterCapability != nil {
+		return c.hooks.OnUnregisterCapability(ctx, params)
 	}
 	return nil
 }
@@ -162,15 +167,21 @@ func (c *Client) ShowDocument(context.Context, *protocol.ShowDocumentParams) (*p
 	return nil, nil
 }
 
-// ApplyEdit applies edits sent from the server.
 func (c *Client) ApplyEdit(ctx context.Context, params *protocol.ApplyWorkspaceEditParams) (*protocol.ApplyWorkspaceEditResult, error) {
 	if len(params.Edit.Changes) != 0 {
 		return &protocol.ApplyWorkspaceEditResult{FailureReason: "Edit.Changes is unsupported"}, nil
 	}
-	for _, change := range params.Edit.DocumentChanges {
-		if err := c.editor.applyDocumentChange(ctx, change); err != nil {
+	if c.hooks.OnApplyEdit != nil {
+		if err := c.hooks.OnApplyEdit(ctx, params); err != nil {
 			return nil, err
 		}
 	}
+	if !c.skipApplyEdits {
+		for _, change := range params.Edit.DocumentChanges {
+			if err := c.editor.applyDocumentChange(ctx, change); err != nil {
+				return nil, err
+			}
+		}
+	}
 	return &protocol.ApplyWorkspaceEditResult{Applied: true}, nil
 }
diff --git a/gopls/internal/lsp/fake/editor.go b/gopls/internal/lsp/fake/editor.go
index 1a0134445..aed7b8daf 100644
--- a/gopls/internal/lsp/fake/editor.go
+++ b/gopls/internal/lsp/fake/editor.go
@@ -123,14 +123,14 @@ func NewEditor(sandbox *Sandbox, config EditorConfig) *Editor {
 // It returns the editor, so that it may be called as follows:
 //
 //	editor, err := NewEditor(s).Connect(ctx, conn, hooks)
-func (e *Editor) Connect(ctx context.Context, connector servertest.Connector, hooks ClientHooks) (*Editor, error) {
+func (e *Editor) Connect(ctx context.Context, connector servertest.Connector, hooks ClientHooks, skipApplyEdits bool) (*Editor, error) {
 	bgCtx, cancelConn := context.WithCancel(xcontext.Detach(ctx))
 	conn := connector.Connect(bgCtx)
 	e.cancelConn = cancelConn
 
 	e.serverConn = conn
 	e.Server = protocol.ServerDispatcher(conn)
-	e.client = &Client{editor: e, hooks: hooks}
+	e.client = &Client{editor: e, hooks: hooks, skipApplyEdits: skipApplyEdits}
 	conn.Go(bgCtx,
 		protocol.Handlers(
 			protocol.ClientHandler(e.client,
diff --git a/gopls/internal/lsp/lsprpc/lsprpc_test.go b/gopls/internal/lsp/lsprpc/lsprpc_test.go
index 6fb8372c5..3ec1a13d2 100644
--- a/gopls/internal/lsp/lsprpc/lsprpc_test.go
+++ b/gopls/internal/lsp/lsprpc/lsprpc_test.go
@@ -225,12 +225,13 @@ func TestDebugInfoLifecycle(t *testing.T) {
 	}
 	tsForwarder := servertest.NewPipeServer(forwarder, nil)
 
-	ed1, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(clientCtx, tsForwarder, fake.ClientHooks{})
+	const skipApplyEdits = false
+	ed1, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(clientCtx, tsForwarder, fake.ClientHooks{}, skipApplyEdits)
 	if err != nil {
 		t.Fatal(err)
 	}
 	defer ed1.Close(clientCtx)
-	ed2, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(baseCtx, tsBackend, fake.ClientHooks{})
+	ed2, err := fake.NewEditor(sb, fake.EditorConfig{}).Connect(baseCtx, tsBackend, fake.ClientHooks{}, skipApplyEdits)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/gopls/internal/lsp/regtest/env.go b/gopls/internal/lsp/regtest/env.go
index 67e720724..91335bdab 100644
--- a/gopls/internal/lsp/regtest/env.go
+++ b/gopls/internal/lsp/regtest/env.go
@@ -62,6 +62,7 @@ func NewAwaiter(workdir *fake.Workdir) *Awaiter {
 	}
 }
 
+// Hooks returns LSP client hooks required for awaiting asynchronous expectations.
 func (a *Awaiter) Hooks() fake.ClientHooks {
 	return fake.ClientHooks{
 		OnDiagnostics:            a.onDiagnostics,
@@ -70,8 +71,9 @@ func (a *Awaiter) Hooks() fake.ClientHooks {
 		OnProgress:               a.onProgress,
 		OnShowMessage:            a.onShowMessage,
 		OnShowMessageRequest:     a.onShowMessageRequest,
-		OnRegistration:           a.onRegistration,
-		OnUnregistration:         a.onUnregistration,
+		OnRegisterCapability:     a.onRegisterCapability,
+		OnUnregisterCapability:   a.onUnregisterCapability,
+		OnApplyEdit:              a.onApplyEdit,
 	}
 }
 
@@ -86,6 +88,7 @@ type State struct {
 	registrations          []*protocol.RegistrationParams
 	registeredCapabilities map[string]protocol.Registration
 	unregistrations        []*protocol.UnregistrationParams
+	documentChanges        []protocol.DocumentChanges // collected from ApplyEdit downcalls
 
 	// outstandingWork is a map of token->work summary. All tokens are assumed to
 	// be string, though the spec allows for numeric tokens as well.  When work
@@ -179,6 +182,15 @@ type condition struct {
 	verdict      chan Verdict
 }
 
+func (a *Awaiter) onApplyEdit(_ context.Context, params *protocol.ApplyWorkspaceEditParams) error {
+	a.mu.Lock()
+	defer a.mu.Unlock()
+
+	a.state.documentChanges = append(a.state.documentChanges, params.Edit.DocumentChanges...)
+	a.checkConditionsLocked()
+	return nil
+}
+
 func (a *Awaiter) onDiagnostics(_ context.Context, d *protocol.PublishDiagnosticsParams) error {
 	a.mu.Lock()
 	defer a.mu.Unlock()
@@ -255,7 +267,7 @@ func (a *Awaiter) onProgress(_ context.Context, m *protocol.ProgressParams) erro
 	return nil
 }
 
-func (a *Awaiter) onRegistration(_ context.Context, m *protocol.RegistrationParams) error {
+func (a *Awaiter) onRegisterCapability(_ context.Context, m *protocol.RegistrationParams) error {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 
@@ -270,7 +282,7 @@ func (a *Awaiter) onRegistration(_ context.Context, m *protocol.RegistrationPara
 	return nil
 }
 
-func (a *Awaiter) onUnregistration(_ context.Context, m *protocol.UnregistrationParams) error {
+func (a *Awaiter) onUnregisterCapability(_ context.Context, m *protocol.UnregistrationParams) error {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 
@@ -288,6 +300,17 @@ func (a *Awaiter) checkConditionsLocked() {
 	}
 }
 
+// takeDocumentChanges returns any accumulated document changes (from
+// server ApplyEdit RPC downcalls) and resets the list.
+func (a *Awaiter) takeDocumentChanges() []protocol.DocumentChanges {
+	a.mu.Lock()
+	defer a.mu.Unlock()
+
+	res := a.state.documentChanges
+	a.state.documentChanges = nil
+	return res
+}
+
 // checkExpectations reports whether s meets all expectations.
 func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
 	finalVerdict := Met
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
index 6a6e616ff..ba0e388fa 100644
--- a/gopls/internal/lsp/regtest/marker.go
+++ b/gopls/internal/lsp/regtest/marker.go
@@ -111,22 +111,34 @@ var update = flag.Bool("update", false, "if set, update test data during marker
 // # Marker types
 //
 // The following markers are supported within marker tests:
+//
 //   - diag(location, regexp): specifies an expected diagnostic matching the
 //     given regexp at the given location. The test runner requires
 //     a 1:1 correspondence between observed diagnostics and diag annotations
+//
 //   - def(src, dst location): perform a textDocument/definition request at
 //     the src location, and check the the result points to the dst location.
+//
 //   - hover(src, dst location, g Golden): perform a textDocument/hover at the
 //     src location, and checks that the result is the dst location, with hover
 //     content matching "hover.md" in the golden data g.
+//
 //   - loc(name, location): specifies the name for a location in the source. These
 //     locations may be referenced by other markers.
+//
 //   - rename(location, new, golden): specifies a renaming of the
 //     identifier at the specified location to the new name.
 //     The golden directory contains the transformed files.
+//
 //   - renameerr(location, new, wantError): specifies a renaming that
 //     fails with an error that matches the expectation.
 //
+//   - suggestedfix(location, regexp, kind, golden): like diag, the location and
+//     regexp identify an expected diagnostic. This diagnostic must
+//     to have exactly one associated code action of the specified kind.
+//     This action is executed for its editing effects on the source files.
+//     Like rename, the golden directory contains the expected transformed files.
+//
 // # Argument conversion
 //
 // Marker arguments are first parsed by the go/expect package, which accepts
@@ -422,12 +434,13 @@ func (mark marker) execute() {
 // Marker funcs should not mutate the test environment (e.g. via opening files
 // or applying edits in the editor).
 var markerFuncs = map[string]markerFunc{
-	"def":       makeMarkerFunc(defMarker),
-	"diag":      makeMarkerFunc(diagMarker),
-	"hover":     makeMarkerFunc(hoverMarker),
-	"loc":       makeMarkerFunc(locMarker),
-	"rename":    makeMarkerFunc(renameMarker),
-	"renameerr": makeMarkerFunc(renameErrMarker),
+	"def":          makeMarkerFunc(defMarker),
+	"diag":         makeMarkerFunc(diagMarker),
+	"hover":        makeMarkerFunc(hoverMarker),
+	"loc":          makeMarkerFunc(locMarker),
+	"rename":       makeMarkerFunc(renameMarker),
+	"renameerr":    makeMarkerFunc(renameErrMarker),
+	"suggestedfix": makeMarkerFunc(suggestedfixMarker),
 }
 
 // markerTest holds all the test data extracted from a test txtar archive.
@@ -683,7 +696,8 @@ func newEnv(t *testing.T, cache *cache.Cache, files map[string][]byte, config fa
 	awaiter := NewAwaiter(sandbox.Workdir)
 	ss := lsprpc.NewStreamServer(cache, false, hooks.Options)
 	server := servertest.NewPipeServer(ss, jsonrpc2.NewRawStream)
-	editor, err := fake.NewEditor(sandbox, config).Connect(ctx, server, awaiter.Hooks())
+	const skipApplyEdits = true // capture edits but don't apply them
+	editor, err := fake.NewEditor(sandbox, config).Connect(ctx, server, awaiter.Hooks(), skipApplyEdits)
 	if err != nil {
 		sandbox.Close() // ignore error
 		t.Fatal(err)
@@ -714,6 +728,7 @@ type markerTestRun struct {
 	env  *Env
 
 	// Collected information.
+	// Each @diag/@suggestedfix marker eliminates an entry from diags.
 	locations map[expect.Identifier]protocol.Location
 	diags     map[protocol.Location][]protocol.Diagnostic
 }
@@ -1074,22 +1089,23 @@ func locMarker(mark marker, name expect.Identifier, loc protocol.Location) {
 	mark.run.locations[name] = loc
 }
 
-// diagMarker implements the @diag hover marker. It eliminates diagnostics from
+// diagMarker implements the @diag marker. It eliminates diagnostics from
 // the observed set in mark.test.
 func diagMarker(mark marker, loc protocol.Location, re *regexp.Regexp) {
-	idx := -1
+	if _, err := removeDiagnostic(mark, loc, re); err != nil {
+		mark.errorf("%v", err)
+	}
+}
+
+func removeDiagnostic(mark marker, loc protocol.Location, re *regexp.Regexp) (protocol.Diagnostic, error) {
 	diags := mark.run.diags[loc]
 	for i, diag := range diags {
 		if re.MatchString(diag.Message) {
-			idx = i
-			break
+			mark.run.diags[loc] = append(diags[:i], diags[i+1:]...)
+			return diag, nil
 		}
 	}
-	if idx >= 0 {
-		mark.run.diags[loc] = append(diags[:idx], diags[idx+1:]...)
-	} else {
-		mark.errorf("no diagnostic matches %q", re)
-	}
+	return protocol.Diagnostic{}, fmt.Errorf("no diagnostic matches %q", re)
 }
 
 // renameMarker implements the @rename(location, new, golden) marker.
@@ -1126,10 +1142,15 @@ func rename(env *Env, loc protocol.Location, newName string) (map[string][]byte,
 		return nil, err
 	}
 
-	// Apply the edits to the Editor buffers
-	// and return the contents of the changed files.
+	return applyDocumentChanges(env, editMap.DocumentChanges)
+}
+
+// applyDocumentChanges returns the effect of applying the document
+// changes to the contents of the Editor buffers. The actual editor
+// buffers are unchanged.
+func applyDocumentChanges(env *Env, changes []protocol.DocumentChanges) (map[string][]byte, error) {
 	result := make(map[string][]byte)
-	for _, change := range editMap.DocumentChanges {
+	for _, change := range changes {
 		if change.RenameFile != nil {
 			// rename
 			oldFile := env.Sandbox.Workdir.URIToPath(change.RenameFile.OldURI)
@@ -1157,3 +1178,96 @@ func rename(env *Env, loc protocol.Location, newName string) (map[string][]byte,
 
 	return result, nil
 }
+
+// suggestedfixMarker implements the @suggestedfix(location, regexp,
+// kind, golden) marker. It acts like @diag(location, regexp), to set
+// the expectation of a diagnostic, but then it applies the first code
+// action of the specified kind suggested by the matched diagnostic.
+func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, actionKind string, golden *Golden) {
+	// Find and remove the matching diagnostic.
+	diag, err := removeDiagnostic(mark, loc, re)
+	if err != nil {
+		mark.errorf("%v", err)
+		return
+	}
+
+	// Apply the fix it suggests.
+	changed, err := suggestedfix(mark.run.env, loc, diag, actionKind)
+	if err != nil {
+		mark.errorf("suggestedfix failed: %v. (Use @suggestedfixerr for expected errors.)", err)
+		return
+	}
+
+	// Check the file state.
+	checkChangedFiles(mark, changed, golden)
+}
+
+func suggestedfix(env *Env, loc protocol.Location, diag protocol.Diagnostic, actionKind string) (map[string][]byte, error) {
+
+	// Request all code actions that apply to the diagnostic.
+	// (The protocol supports filtering using Context.Only={actionKind}
+	// but we can give a better error if we don't filter.)
+	actions, err := env.Editor.Server.CodeAction(env.Ctx, &protocol.CodeActionParams{
+		TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI},
+		Range:        diag.Range,
+		Context: protocol.CodeActionContext{
+			Only:        nil, // => all kinds
+			Diagnostics: []protocol.Diagnostic{diag},
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	// Find the sole candidates CodeAction of the specified kind (e.g. refactor.rewrite).
+	var candidates []protocol.CodeAction
+	for _, act := range actions {
+		if act.Kind == protocol.CodeActionKind(actionKind) {
+			candidates = append(candidates, act)
+		}
+	}
+	if len(candidates) != 1 {
+		for _, act := range actions {
+			env.T.Logf("found CodeAction Kind=%s Title=%q", act.Kind, act.Title)
+		}
+		return nil, fmt.Errorf("found %d CodeActions of kind %s for this diagnostic, want 1", len(candidates), actionKind)
+	}
+	action := candidates[0]
+
+	// An action may specify an edit and/or a command, to be
+	// applied in that order. But since applyDocumentChanges(env,
+	// action.Edit.DocumentChanges) doesn't compose, for now we
+	// assert that all commands used in the @suggestedfix tests
+	// return only a command.
+	if action.Edit.DocumentChanges != nil {
+		env.T.Errorf("internal error: discarding unexpected CodeAction{Kind=%s, Title=%q}.Edit.DocumentChanges", action.Kind, action.Title)
+	}
+	if action.Command == nil {
+		return nil, fmt.Errorf("missing CodeAction{Kind=%s, Title=%q}.Command", action.Kind, action.Title)
+	}
+
+	// This is a typical CodeAction command:
+	//
+	//   Title:     "Implement error"
+	//   Command:   gopls.apply_fix
+	//   Arguments: [{"Fix":"stub_methods","URI":".../a.go","Range":...}}]
+	//
+	// The client makes an ExecuteCommand RPC to the server,
+	// which dispatches it to the ApplyFix handler.
+	// ApplyFix dispatches to the "stub_methods" suggestedfix hook (the meat).
+	// The server then makes an ApplyEdit RPC to the client,
+	// whose Awaiter hook gathers the edits instead of applying them.
+
+	_ = env.Awaiter.takeDocumentChanges() // reset (assuming Env is confined to this thread)
+
+	if _, err := env.Editor.Server.ExecuteCommand(env.Ctx, &protocol.ExecuteCommandParams{
+		Command:   action.Command.Command,
+		Arguments: action.Command.Arguments,
+	}); err != nil {
+		env.T.Fatalf("error converting command %q to edits: %v", action.Command.Command, err)
+	}
+
+	return applyDocumentChanges(env, env.Awaiter.takeDocumentChanges())
+}
+
+// TODO(adonovan): suggestedfixerr
diff --git a/gopls/internal/lsp/regtest/runner.go b/gopls/internal/lsp/regtest/runner.go
index 5f556b282..57f541c63 100644
--- a/gopls/internal/lsp/regtest/runner.go
+++ b/gopls/internal/lsp/regtest/runner.go
@@ -220,7 +220,8 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
 			ts := servertest.NewPipeServer(ss, framer)
 
 			awaiter := NewAwaiter(sandbox.Workdir)
-			editor, err := fake.NewEditor(sandbox, config.editor).Connect(ctx, ts, awaiter.Hooks())
+			const skipApplyEdits = false
+			editor, err := fake.NewEditor(sandbox, config.editor).Connect(ctx, ts, awaiter.Hooks(), skipApplyEdits)
 			if err != nil {
 				t.Fatal(err)
 			}
diff --git a/gopls/internal/regtest/bench/bench_test.go b/gopls/internal/regtest/bench/bench_test.go
index 76b3dcfd4..29e02ce6e 100644
--- a/gopls/internal/regtest/bench/bench_test.go
+++ b/gopls/internal/regtest/bench/bench_test.go
@@ -114,11 +114,13 @@ func connectEditor(dir string, config fake.EditorConfig, ts servertest.Connector
 	}
 
 	a := NewAwaiter(s.Workdir)
-	e, err := fake.NewEditor(s, config).Connect(context.Background(), ts, a.Hooks())
+	const skipApplyEdits = false
+	editor, err := fake.NewEditor(s, config).Connect(context.Background(), ts, a.Hooks(), skipApplyEdits)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	return s, e, a, nil
+
+	return s, editor, a, nil
 }
 
 // newGoplsServer returns a connector that connects to a new gopls process.
diff --git a/gopls/internal/regtest/bench/stress_test.go b/gopls/internal/regtest/bench/stress_test.go
index 471e2e192..15a2c9081 100644
--- a/gopls/internal/regtest/bench/stress_test.go
+++ b/gopls/internal/regtest/bench/stress_test.go
@@ -49,7 +49,8 @@ func TestPilosaStress(t *testing.T) {
 	ts := servertest.NewPipeServer(server, jsonrpc2.NewRawStream)
 	ctx := context.Background()
 
-	editor, err := fake.NewEditor(sandbox, fake.EditorConfig{}).Connect(ctx, ts, fake.ClientHooks{})
+	const skipApplyEdits = false
+	editor, err := fake.NewEditor(sandbox, fake.EditorConfig{}).Connect(ctx, ts, fake.ClientHooks{}, skipApplyEdits)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/gopls/internal/regtest/marker/testdata/stubmethods/basic.txt b/gopls/internal/regtest/marker/testdata/stubmethods/basic.txt
new file mode 100644
index 000000000..bb53e6767
--- /dev/null
+++ b/gopls/internal/regtest/marker/testdata/stubmethods/basic.txt
@@ -0,0 +1,24 @@
+This test exercises basic 'stub methods' functionality.
+
+-- go.mod --
+module example.com
+go 1.12
+
+-- a/a.go --
+package a
+
+type C int
+
+var _ error = C(0) //@suggestedfix(re"C.0.", re"missing method Error", "refactor.rewrite", stub)
+
+-- @stub/a/a.go --
+package a
+
+type C int
+
+// Error implements error
+func (C) Error() string {
+	panic("unimplemented")
+}
+
+var _ error = C(0) //@suggestedfix(re"C.0.", re"missing method Error", "refactor.rewrite", stub)
diff --git a/gopls/internal/regtest/misc/leak_test.go b/gopls/internal/regtest/misc/leak_test.go
index 88d2f0113..586ffcc41 100644
--- a/gopls/internal/regtest/misc/leak_test.go
+++ b/gopls/internal/regtest/misc/leak_test.go
@@ -73,12 +73,16 @@ func setupEnv(t *testing.T, files string, c *cache.Cache) *Env {
 	}
 
 	a := NewAwaiter(s.Workdir)
-	e, err := fake.NewEditor(s, fake.EditorConfig{}).Connect(ctx, ts, a.Hooks())
+	const skipApplyEdits = false
+	editor, err := fake.NewEditor(s, fake.EditorConfig{}).Connect(ctx, ts, a.Hooks(), skipApplyEdits)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	return &Env{
 		T:       t,
 		Ctx:     ctx,
-		Editor:  e,
+		Editor:  editor,
 		Sandbox: s,
 		Awaiter: a,
 	}
diff --git a/gopls/internal/regtest/misc/shared_test.go b/gopls/internal/regtest/misc/shared_test.go
index eaa4c7e2f..410a8d327 100644
--- a/gopls/internal/regtest/misc/shared_test.go
+++ b/gopls/internal/regtest/misc/shared_test.go
@@ -33,7 +33,8 @@ func main() {
 		// Create a second test session connected to the same workspace and server
 		// as the first.
 		awaiter := NewAwaiter(env1.Sandbox.Workdir)
-		editor, err := fake.NewEditor(env1.Sandbox, env1.Editor.Config()).Connect(env1.Ctx, env1.Server, awaiter.Hooks())
+		const skipApplyEdits = false
+		editor, err := fake.NewEditor(env1.Sandbox, env1.Editor.Config()).Connect(env1.Ctx, env1.Server, awaiter.Hooks(), skipApplyEdits)
 		if err != nil {
 			t.Fatal(err)
 		}