gopls/internal/regtest/marker: use golden diffs for suggested fixes

Following the model of codeactionerr, use diffs to reduce the verbosity and redundancy of golden content, for the suggestedfix marker. Also, since all suggested fixes should be of kind 'quickfix', remove the unnecessary kind parameter. Finally, clean up some stale comments. For golang/go#54845 Change-Id: I2eb08e4415dcff4acba604bf97b16c0a82c0a658 Reviewed-on: https://go-review.googlesource.com/c/tools/+/539656 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Alan Donovan <adonovan@google.com>
golang · Nov 8, 2023 · bbf8380 · bbf8380
1 parent 51df92b
commit bbf8380
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 123 deletions.
diff --git a/gopls/internal/lsp/regtest/marker.go b/gopls/internal/lsp/regtest/marker.go
@@ -249,9 +249,6 @@ var update = flag.Bool("update", false, "if set, update test data during marker
 //     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.
-//     TODO(rfindley): we probably only need 'suggestedfix' for quick-fixes. All
-//     other actions should use codeaction markers. In that case, we can remove
-//     the 'kind' parameter.
 //
 //   - rank(location, ...completionItem): executes a textDocument/completion
 //     request at the given location, and verifies that each expected
@@ -387,15 +384,11 @@ var update = flag.Bool("update", false, "if set, update test data during marker
 //
 // Existing marker tests (in ../testdata) to port:
 //   - CallHierarchy
-//   - Completions
-//   - CompletionSnippets
-//   - CaseSensitiveCompletions
-//   - RankCompletions
 //   - SemanticTokens
-//   - FunctionExtractions
+//   - SuggestedFixes
 //   - MethodExtractions
-//   - Renames
 //   - InlayHints
+//   - Renames
 //   - SelectionRanges
 func RunMarkerTests(t *testing.T, dir string) {
 	// The marker tests must be able to run go/packages.Load.
@@ -407,7 +400,6 @@ func RunMarkerTests(t *testing.T, dir string) {
 	}
 
 	// Opt: use a shared cache.
-	// TODO(rfindley): opt: use a memoize store with no eviction.
 	cache := cache.New(nil)
 
 	for _, test := range tests {
@@ -2035,7 +2027,7 @@ func (mark marker) consumeExtraNotes(name string, f func(marker)) {
 // 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) {
+func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, golden *Golden) {
 	loc.Range.End = loc.Range.Start // diagnostics ignore end position.
 	// Find and remove the matching diagnostic.
 	diag, ok := removeDiagnostic(mark, loc, re)
@@ -2045,14 +2037,14 @@ func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, a
 	}
 
 	// Apply the fix it suggests.
-	changed, err := codeAction(mark.run.env, loc.URI, diag.Range, actionKind, &diag)
+	changed, err := codeAction(mark.run.env, loc.URI, diag.Range, "quickfix", &diag)
 	if err != nil {
 		mark.errorf("suggestedfix failed: %v. (Use @suggestedfixerr for expected errors.)", err)
 		return
 	}
 
 	// Check the file state.
-	checkChangedFiles(mark, changed, golden)
+	checkDiffs(mark, changed, golden)
 }
 
 // codeAction executes a textDocument/codeAction request for the specified

diff --git a/gopls/internal/regtest/marker/testdata/diagnostics/typeerr.txt b/gopls/internal/regtest/marker/testdata/diagnostics/typeerr.txt
@@ -19,15 +19,12 @@ package a
 func f(x int) {
 	append("") //@diag(re`""`, re"a slice")
 
-	x := 123 //@diag(re"x := 123", re"no new variables"), suggestedfix(re"():", re"no new variables", "quickfix", fix)
+	x := 123 //@diag(re"x := 123", re"no new variables"), suggestedfix(re"():", re"no new variables", fix)
 }
 
 -- @fix/typeerr.go --
-package a
-
-func f(x int) {
-	append("") //@diag(re`""`, re"a slice")
-
-	x = 123 //@diag(re"x := 123", re"no new variables"), suggestedfix(re"():", re"no new variables", "quickfix", fix)
-}
-
+--- before
++++ after
+@@ -6 +6 @@
+-	x := 123 //@diag(re"x := 123", re"no new variables"), suggestedfix(re"():", re"no new variables", fix)
++	x = 123 //@diag(re"x := 123", re"no new variables"), suggestedfix(re"():", re"no new variables", fix)
diff --git a/gopls/internal/regtest/marker/testdata/stubmethods/basic.txt b/gopls/internal/regtest/marker/testdata/stubmethods/basic.txt
@@ -9,16 +9,15 @@ package a
 
 type C int
 
-var _ error = C(0) //@suggestedfix(re"C.0.", re"missing method Error", "quickfix", stub)
-
+var _ error = C(0) //@suggestedfix(re"C.0.", re"missing method Error", 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", "quickfix", stub)
+--- before
++++ after
+@@ -3 +3,6 @@
+-type C int
++type C int
++
++// Error implements error.
++func (C) Error() string {
++	panic("unimplemented")
++}
diff --git a/gopls/internal/regtest/marker/testdata/stubmethods/issue61693.txt b/gopls/internal/regtest/marker/testdata/stubmethods/issue61693.txt
@@ -15,21 +15,16 @@ func F(err ...error) {}
 
 func _() {
 	var x error
-	F(x, C(0)) //@suggestedfix(re"C.0.", re"missing method Error", "quickfix", stub)
+	F(x, C(0)) //@suggestedfix(re"C.0.", re"missing method Error", stub)
 }
 -- @stub/main.go --
-package main
-
-type C int
-
-// Error implements error.
-func (C) Error() string {
-	panic("unimplemented")
-}
-
-func F(err ...error) {}
-
-func _() {
-	var x error
-	F(x, C(0)) //@suggestedfix(re"C.0.", re"missing method Error", "quickfix", stub)
-}
+--- before
++++ after
+@@ -3 +3,6 @@
+-type C int
++type C int
++
++// Error implements error.
++func (C) Error() string {
++	panic("unimplemented")
++}
diff --git a/gopls/internal/regtest/marker/testdata/stubmethods/issue61830.txt b/gopls/internal/regtest/marker/testdata/stubmethods/issue61830.txt
@@ -14,23 +14,15 @@ type I interface {
 
 type A struct{}
 
-var _ I = &A{} //@suggestedfix(re"&A..", re"missing method M", "quickfix", stub)
+var _ I = &A{} //@suggestedfix(re"&A..", re"missing method M", stub)
 -- @stub/p.go --
-package p
-
-import "io"
-
-type B struct{}
-
-type I interface {
-	M(io.Reader, B)
-}
-
-type A struct{}
-
-// M implements I.
-func (*A) M(io.Reader, B) {
-	panic("unimplemented")
-}
-
-var _ I = &A{} //@suggestedfix(re"&A..", re"missing method M", "quickfix", stub)
+--- before
++++ after
+@@ -11 +11,6 @@
+-type A struct{}
++type A struct{}
++
++// M implements I.
++func (*A) M(io.Reader, B) {
++	panic("unimplemented")
++}
diff --git a/gopls/internal/regtest/marker/testdata/suggestedfix/self_assignment.txt b/gopls/internal/regtest/marker/testdata/suggestedfix/self_assignment.txt
@@ -9,20 +9,13 @@ import (
 
 func goodbye() {
 	s := "hiiiiiii"
-	s = s //@suggestedfix("s = s", re"self-assignment", "quickfix", fix)
+	s = s //@suggestedfix("s = s", re"self-assignment", fix)
 	log.Print(s)
 }
 
 -- @fix/a.go --
-package suggestedfix
-
-import (
-	"log"
-)
-
-func goodbye() {
-	s := "hiiiiiii"
-	 //@suggestedfix("s = s", re"self-assignment", "quickfix", fix)
-	log.Print(s)
-}
-
+--- before
++++ after
+@@ -9 +9 @@
+-	s = s //@suggestedfix("s = s", re"self-assignment", fix)
++	 //@suggestedfix("s = s", re"self-assignment", fix)
diff --git a/gopls/internal/regtest/marker/testdata/suggestedfix/undeclared.txt b/gopls/internal/regtest/marker/testdata/suggestedfix/undeclared.txt
@@ -9,54 +9,46 @@ go 1.12
 package p
 
 func a() {
-	z, _ := 1+y, 11 //@suggestedfix("y", re"(undeclared name|undefined): y", "quickfix", a)
+	z, _ := 1+y, 11 //@suggestedfix("y", re"(undeclared name|undefined): y", a)
 	_ = z
 }
 
 -- @a/a.go --
-package p
-
-func a() {
-	y := 
-	z, _ := 1+y, 11 //@suggestedfix("y", re"(undeclared name|undefined): y", "quickfix", a)
-	_ = z
-}
-
+--- before
++++ after
+@@ -3 +3,2 @@
+-func a() {
++func a() {
++	y := 
 -- b.go --
 package p
 
 func b() {
 	if 100 < 90 {
-	} else if 100 > n+2 { //@suggestedfix("n", re"(undeclared name|undefined): n", "quickfix", b)
+	} else if 100 > n+2 { //@suggestedfix("n", re"(undeclared name|undefined): n", b)
 	}
 }
 
 -- @b/b.go --
-package p
-
-func b() {
-	n := 
-	if 100 < 90 {
-	} else if 100 > n+2 { //@suggestedfix("n", re"(undeclared name|undefined): n", "quickfix", b)
-	}
-}
-
+--- before
++++ after
+@@ -3 +3,2 @@
+-func b() {
++func b() {
++	n := 
 -- c.go --
 package p
 
 func c() {
-	for i < 200 { //@suggestedfix("i", re"(undeclared name|undefined): i", "quickfix", c)
+	for i < 200 { //@suggestedfix("i", re"(undeclared name|undefined): i", c)
 	}
 	r() //@diag("r", re"(undeclared name|undefined): r")
 }
 
 -- @c/c.go --
-package p
-
-func c() {
-	i := 
-	for i < 200 { //@suggestedfix("i", re"(undeclared name|undefined): i", "quickfix", c)
-	}
-	r() //@diag("r", re"(undeclared name|undefined): r")
-}
-
+--- before
++++ after
+@@ -3 +3,2 @@
+-func c() {
++func c() {
++	i := 
diff --git a/gopls/internal/regtest/marker/testdata/suggestedfix/unusedrequire.txt b/gopls/internal/regtest/marker/testdata/suggestedfix/unusedrequire.txt
@@ -13,12 +13,15 @@ module mod.com
 
 go 1.14
 
-require example.com v1.0.0 //@suggestedfix("require", re"not used", "quickfix", a)
+require example.com v1.0.0 //@suggestedfix("require", re"not used", a)
 
 -- @a/a/go.mod --
-module mod.com
-
-go 1.14
+--- before
++++ after
+@@ -4,3 +4 @@
+-
+-require example.com v1.0.0 //@suggestedfix("require", re"not used", a)
+-
 -- a/main.go --
 package main
 func main() {}
diff --git a/gopls/internal/regtest/marker/testdata/suggestedfix/unusedrequire_gowork.txt b/gopls/internal/regtest/marker/testdata/suggestedfix/unusedrequire_gowork.txt
@@ -23,12 +23,15 @@ module mod.com/a
 
 go 1.14
 
-require example.com v1.0.0 //@suggestedfix("require", re"not used", "quickfix", a)
+require example.com v1.0.0 //@suggestedfix("require", re"not used", a)
 
 -- @a/a/go.mod --
-module mod.com/a
-
-go 1.14
+--- before
++++ after
+@@ -4,3 +4 @@
+-
+-require example.com v1.0.0 //@suggestedfix("require", re"not used", a)
+-
 -- a/main.go --
 package main
 func main() {}
@@ -38,12 +41,15 @@ module mod.com/b
 
 go 1.14
 
-require example.com v1.0.0 //@suggestedfix("require", re"not used", "quickfix", b)
+require example.com v1.0.0 //@suggestedfix("require", re"not used", b)
 
 -- @b/b/go.mod --
-module mod.com/b
-
-go 1.14
+--- before
++++ after
+@@ -4,3 +4 @@
+-
+-require example.com v1.0.0 //@suggestedfix("require", re"not used", b)
+-
 -- b/main.go --
 package main
 func main() {}