gopls/internal/regtest/marker: porting extract tests

Port the extraction tests, which involved a mixture of suggestedfix,
extractmethod, and extractfunc markers (notably, nothing was running the
extractfunc markers!). To do this, extend the new codeaction markers to
accept an option slice of titles to use for filtering.

Modify the method/func extraction tests to reduce verbosity and long
lines.

For golang/go#54845

Change-Id: I319d1c731a4cdb4ad952944667bc57d781ad176d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/539481
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

gopls/internal/lsp/lsp_test.go

@@ -345,58 +345,6 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []tests.S
 	}
 }
 
-func (r *runner) MethodExtraction(t *testing.T, start span.Span, end span.Span) {
-	uri := start.URI()
-	m, err := r.data.Mapper(uri)
-	if err != nil {
-		t.Fatal(err)
-	}
-	spn := span.New(start.URI(), start.Start(), end.End())
-	rng, err := m.SpanRange(spn)
-	if err != nil {
-		t.Fatal(err)
-	}
-	actionsRaw, err := r.server.CodeAction(r.ctx, &protocol.CodeActionParams{
-		TextDocument: protocol.TextDocumentIdentifier{
-			URI: protocol.URIFromSpanURI(uri),
-		},
-		Range: rng,
-		Context: protocol.CodeActionContext{
-			Only: []protocol.CodeActionKind{"refactor.extract"},
-		},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	var actions []protocol.CodeAction
-	for _, action := range actionsRaw {
-		if action.Command.Title == "Extract method" {
-			actions = append(actions, action)
-		}
-	}
-	// Hack: We assume that we only get one matching code action per range.
-	// TODO(rstambler): Support multiple code actions per test.
-	if len(actions) == 0 || len(actions) > 1 {
-		t.Fatalf("unexpected number of code actions, want 1, got %v", len(actions))
-	}
-	_, err = r.server.ExecuteCommand(r.ctx, &protocol.ExecuteCommandParams{
-		Command:   actions[0].Command.Command,
-		Arguments: actions[0].Command.Arguments,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	res := <-r.editRecv
-	for u, got := range res {
-		want := r.data.Golden(t, "methodextraction_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
-			return got, nil
-		})
-		if diff := compare.Bytes(want, got); diff != "" {
-			t.Errorf("method extraction failed for %s:\n%s", u.Filename(), diff)
-		}
-	}
-}
-
 func (r *runner) InlayHints(t *testing.T, spn span.Span) {
 	uri := spn.URI()
 	filename := uri.Filename()

gopls/internal/lsp/regtest/marker.go

@@ -153,15 +153,20 @@ var update = flag.Bool("update", false, "if set, update test data during marker
 //     completion candidate produced at the given location with provided label
 //     results in the given golden state.
 //
-//   - codeaction(start, end, kind, golden): specifies a code action to request
-//     for the given range. To support multi-line ranges, the range is defined
-//     to be between start.Start and end.End. The golden directory contains
-//     changed file content after the code action is applied.
+//   - codeaction(start, end, kind, golden, ...titles): specifies a code action
+//     to request for the given range. To support multi-line ranges, the range
+//     is defined to be between start.Start and end.End. The golden directory
+//     contains changed file content after the code action is applied.
+//     If titles are provided, they are used to filter the matching code
+//     action.
 //
-//   - codeactionedit(range, kind, golden): a shorter form of codeaction.
-//     Invokes a code action of the given kind for the given in-line range, and
-//     compares the resulting formatted unified *edits* (notably, not the full
-//     file content) with the golden directory.
+//     TODO(rfindley): consolidate with codeactionedit, via a @loc2 marker that
+//     allows binding multi-line locations.
+//
+//   - codeactionedit(range, kind, golden, ...titles): a shorter form of
+//     codeaction. Invokes a code action of the given kind for the given
+//     in-line range, and compares the resulting formatted unified *edits*
+//     (notably, not the full file content) with the golden directory.
 //
 //   - codeactionerr(start, end, kind, wantError): specifies a codeaction that
 //     fails with an error that matches the expectation.
@@ -381,12 +386,15 @@ var update = flag.Bool("update", false, "if set, update test data during marker
 //   - Provide some means by which locations in the standard library
 //     (or builtin.go) can be named, so that, for example, we can we
 //     can assert that MyError implements the built-in error type.
+//   - If possible, improve handling for optional arguments. Rather than have
+//     multiple variations of a marker, it would be nice to support a more
+//     flexible signature: can codeaction, codeactionedit, codeactionerr, and
+//     suggestedfix be consolidated?
 //
 // Existing marker tests (in ../testdata) to port:
 //   - CallHierarchy
 //   - SemanticTokens
 //   - SuggestedFixes
-//   - MethodExtractions
 //   - InlayHints
 //   - Renames
 //   - SelectionRanges
@@ -1924,13 +1932,13 @@ func applyDocumentChanges(env *Env, changes []protocol.DocumentChanges, fileChan
 	return nil
 }
 
-func codeActionMarker(mark marker, start, end protocol.Location, actionKind string, g *Golden) {
+func codeActionMarker(mark marker, start, end protocol.Location, actionKind string, g *Golden, titles ...string) {
 	// Request the range from start.Start to end.End.
 	loc := start
 	loc.Range.End = end.Range.End
 
 	// Apply the fix it suggests.
-	changed, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil)
+	changed, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil, titles)
 	if err != nil {
 		mark.errorf("codeAction failed: %v", err)
 		return
@@ -1940,8 +1948,8 @@ func codeActionMarker(mark marker, start, end protocol.Location, actionKind stri
 	checkChangedFiles(mark, changed, g)
 }
 
-func codeActionEditMarker(mark marker, loc protocol.Location, actionKind string, g *Golden) {
-	changed, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil)
+func codeActionEditMarker(mark marker, loc protocol.Location, actionKind string, g *Golden, titles ...string) {
+	changed, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil, titles)
 	if err != nil {
 		mark.errorf("codeAction failed: %v", err)
 		return
@@ -1953,7 +1961,7 @@ func codeActionEditMarker(mark marker, loc protocol.Location, actionKind string,
 func codeActionErrMarker(mark marker, start, end protocol.Location, actionKind string, wantErr wantError) {
 	loc := start
 	loc.Range.End = end.Range.End
-	_, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil)
+	_, err := codeAction(mark.run.env, loc.URI, loc.Range, actionKind, nil, nil)
 	wantErr.check(mark, err)
 }
 
@@ -2037,7 +2045,7 @@ func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, g
 	}
 
 	// Apply the fix it suggests.
-	changed, err := codeAction(mark.run.env, loc.URI, diag.Range, "quickfix", &diag)
+	changed, err := codeAction(mark.run.env, loc.URI, diag.Range, "quickfix", &diag, nil)
 	if err != nil {
 		mark.errorf("suggestedfix failed: %v. (Use @suggestedfixerr for expected errors.)", err)
 		return
@@ -2054,8 +2062,8 @@ func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, g
 // The resulting map contains resulting file contents after the code action is
 // applied. Currently, this function does not support code actions that return
 // edits directly; it only supports code action commands.
-func codeAction(env *Env, uri protocol.DocumentURI, rng protocol.Range, actionKind string, diag *protocol.Diagnostic) (map[string][]byte, error) {
-	changes, err := codeActionChanges(env, uri, rng, actionKind, diag)
+func codeAction(env *Env, uri protocol.DocumentURI, rng protocol.Range, actionKind string, diag *protocol.Diagnostic, titles []string) (map[string][]byte, error) {
+	changes, err := codeActionChanges(env, uri, rng, actionKind, diag, titles)
 	if err != nil {
 		return nil, err
 	}
@@ -2069,7 +2077,8 @@ func codeAction(env *Env, uri protocol.DocumentURI, rng protocol.Range, actionKi
 // codeActionChanges executes a textDocument/codeAction request for the
 // specified location and kind, and captures the resulting document changes.
 // If diag is non-nil, it is used as the code action context.
-func codeActionChanges(env *Env, uri protocol.DocumentURI, rng protocol.Range, actionKind string, diag *protocol.Diagnostic) ([]protocol.DocumentChanges, error) {
+// If titles is non-empty, the code action title must be present among the provided titles.
+func codeActionChanges(env *Env, uri protocol.DocumentURI, rng protocol.Range, actionKind string, diag *protocol.Diagnostic, titles []string) ([]protocol.DocumentChanges, 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.)
@@ -2093,14 +2102,23 @@ func codeActionChanges(env *Env, uri protocol.DocumentURI, rng protocol.Range, a
 	var candidates []protocol.CodeAction
 	for _, act := range actions {
 		if act.Kind == protocol.CodeActionKind(actionKind) {
-			candidates = append(candidates, act)
+			if len(titles) > 0 {
+				for _, f := range titles {
+					if act.Title == f {
+						candidates = append(candidates, act)
+						break
+					}
+				}
+			} else {
+				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)
+		return nil, fmt.Errorf("found %d CodeActions of kind %s matching filters %v for this diagnostic, want 1", len(candidates), actionKind, titles)
 	}
 	action := candidates[0]