gopls/internal/lsp/source: put testing.T/B first when extracting
Put the testing.T/B second when extracting functions/methods.
It's next after context.Context.
Fixes golang/go#69341
Change-Id: Idcfc0e09e4174646a3f136dcc5badfda4af9938e
GitHub-Last-Rev: 99de9722e6856e13dc2bc4d069cee149c62e3631
GitHub-Pull-Request: golang/tools#517
Reviewed-on: https://go-review.googlesource.com/c/tools/+/610976
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Tim King <taking@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/gopls/internal/golang/extract.go b/gopls/internal/golang/extract.go
index cc82a53..3610dee 100644
--- a/gopls/internal/golang/extract.go
+++ b/gopls/internal/golang/extract.go
@@ -651,8 +651,12 @@
}, nil
}
-// isSelector reports if e is the selector expr <x>, <sel>.
+// isSelector reports if e is the selector expr <x>, <sel>. It works for pointer and non-pointer selector expressions.
func isSelector(e ast.Expr, x, sel string) bool {
+ unary, ok := e.(*ast.UnaryExpr)
+ if ok && unary.Op == token.MUL {
+ e = unary.X
+ }
selectorExpr, ok := e.(*ast.SelectorExpr)
if !ok {
return false
@@ -666,9 +670,15 @@
// reorderParams reorders the given parameters in-place to follow common Go conventions.
func reorderParams(params []ast.Expr, paramTypes []*ast.Field) {
+ moveParamToFrontIfFound(params, paramTypes, "testing", "T")
+ moveParamToFrontIfFound(params, paramTypes, "testing", "B")
+ moveParamToFrontIfFound(params, paramTypes, "context", "Context")
+}
+
+func moveParamToFrontIfFound(params []ast.Expr, paramTypes []*ast.Field, x, sel string) {
// Move Context parameter (if any) to front.
for i, t := range paramTypes {
- if isSelector(t.Type, "context", "Context") {
+ if isSelector(t.Type, x, sel) {
p, t := params[i], paramTypes[i]
copy(params[1:], params[:i])
copy(paramTypes[1:], paramTypes[:i])
diff --git a/gopls/internal/test/marker/testdata/codeaction/extract_method.txt b/gopls/internal/test/marker/testdata/codeaction/extract_method.txt
index 2b357be..7580050 100644
--- a/gopls/internal/test/marker/testdata/codeaction/extract_method.txt
+++ b/gopls/internal/test/marker/testdata/codeaction/extract_method.txt
@@ -157,12 +157,17 @@
-- context.go --
package extract
-import "context"
+import (
+ "context"
+ "testing"
+)
//@codeactionedit(B_AddP, "refactor.extract", contextMeth1, "Extract method")
//@codeactionedit(B_AddP, "refactor.extract", contextFunc1, "Extract function")
//@codeactionedit(B_LongList, "refactor.extract", contextMeth2, "Extract method")
//@codeactionedit(B_LongList, "refactor.extract", contextFunc2, "Extract function")
+//@codeactionedit(B_AddPWithB, "refactor.extract", contextFuncB, "Extract function")
+//@codeactionedit(B_LongListWithT, "refactor.extract", contextFuncT, "Extract function")
type B struct {
x int
@@ -180,39 +185,72 @@
p3 := 1
return p1 + p2 + p3, ctx.Err() //@loc(B_LongList, re`return.*ctx\.Err\(\)`)
}
+
+func (b *B) AddPWithB(ctx context.Context, tB *testing.B) (int, error) {
+ sum := b.x + b.y //@loc(B_AddPWithB, re`(?s:^.*?Err\(\))`)
+ tB.Skip()
+ return sum, ctx.Err()
+}
+
+func (b *B) LongListWithT(ctx context.Context, t *testing.T) (int, error) {
+ p1 := 1
+ p2 := 1
+ p3 := 1
+ p4 := p1 + p2 //@loc(B_LongListWithT, re`(?s:^.*?Err\(\))`)
+ t.Skip()
+ return p4 + p3, ctx.Err()
+}
-- @contextMeth1/context.go --
-@@ -17 +17 @@
+@@ -22 +22 @@
- return sum, ctx.Err() //@loc(B_AddP, re`return.*ctx\.Err\(\)`)
+ return b.newMethod(ctx, sum) //@loc(B_AddP, re`return.*ctx\.Err\(\)`)
-@@ -20 +20,4 @@
+@@ -25 +25,4 @@
+func (*B) newMethod(ctx context.Context, sum int) (int, error) {
+ return sum, ctx.Err()
+}
+
-- @contextMeth2/context.go --
-@@ -24 +24 @@
+@@ -29 +29 @@
- return p1 + p2 + p3, ctx.Err() //@loc(B_LongList, re`return.*ctx\.Err\(\)`)
+ return b.newMethod(ctx, p1, p2, p3) //@loc(B_LongList, re`return.*ctx\.Err\(\)`)
-@@ -26 +26,4 @@
-+
+@@ -32 +32,4 @@
+func (*B) newMethod(ctx context.Context, p1 int, p2 int, p3 int) (int, error) {
+ return p1 + p2 + p3, ctx.Err()
+}
++
-- @contextFunc2/context.go --
-@@ -24 +24 @@
+@@ -29 +29 @@
- return p1 + p2 + p3, ctx.Err() //@loc(B_LongList, re`return.*ctx\.Err\(\)`)
+ return newFunction(ctx, p1, p2, p3) //@loc(B_LongList, re`return.*ctx\.Err\(\)`)
-@@ -26 +26,4 @@
-+
+@@ -32 +32,4 @@
+func newFunction(ctx context.Context, p1 int, p2 int, p3 int) (int, error) {
+ return p1 + p2 + p3, ctx.Err()
+}
++
-- @contextFunc1/context.go --
-@@ -17 +17 @@
+@@ -22 +22 @@
- return sum, ctx.Err() //@loc(B_AddP, re`return.*ctx\.Err\(\)`)
+ return newFunction(ctx, sum) //@loc(B_AddP, re`return.*ctx\.Err\(\)`)
-@@ -20 +20,4 @@
+@@ -25 +25,4 @@
+func newFunction(ctx context.Context, sum int) (int, error) {
+ return sum, ctx.Err()
+}
+
+-- @contextFuncB/context.go --
+@@ -33 +33,6 @@
+- sum := b.x + b.y //@loc(B_AddPWithB, re`(?s:^.*?Err\(\))`)
++ //@loc(B_AddPWithB, re`(?s:^.*?Err\(\))`)
++ return newFunction(ctx, tB, b)
++}
++
++func newFunction(ctx context.Context, tB *testing.B, b *B) (int, error) {
++ sum := b.x + b.y
+-- @contextFuncT/context.go --
+@@ -42 +42,6 @@
+- p4 := p1 + p2 //@loc(B_LongListWithT, re`(?s:^.*?Err\(\))`)
++ //@loc(B_LongListWithT, re`(?s:^.*?Err\(\))`)
++ return newFunction(ctx, t, p1, p2, p3)
++}
++
++func newFunction(ctx context.Context, t *testing.T, p1 int, p2 int, p3 int) (int, error) {
++ p4 := p1 + p2
