gopls/internal/golang: support removing unused parameters from methods

Make a minimal change to the removeparam algorithm to support removing
unused parameters of methods.

This doesn't address the problem of expanding the set of signatures or
calls based on interface satisfaction constraints: that is really a
separate concern, which also affects method renaming (golang/go#58461).
Were we to have a more general solution to that problem, it would be
relatively straightforward to also expand the change signature
algorithm.

For golang/go#38028

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

Author: findleyr

gopls/internal/golang/change_signature.go

@@ -59,9 +59,6 @@ func RemoveUnusedParameter(ctx context.Context, fh file.Handle, rng protocol.Ran
 	if err != nil {
 		return nil, err // e.g. invalid range
 	}
-	if info.Decl.Recv != nil {
-		return nil, fmt.Errorf("can't change signature of methods (yet)")
-	}
 	if info.Field == nil {
 		return nil, fmt.Errorf("failed to find field")
 	}
@@ -373,8 +370,50 @@ func rewriteCalls(ctx context.Context, rw signatureRewrite) (map[protocol.Docume
 
 		wrapper := internalastutil.CloneNode(rw.origDecl)
 		wrapper.Type.Params = rw.params
+
+		// Get the receiver name, creating it if necessary.
+		var recv string // nonempty => call is a method call with receiver recv
+		if wrapper.Recv.NumFields() > 0 {
+			if len(wrapper.Recv.List[0].Names) > 0 {
+				recv = wrapper.Recv.List[0].Names[0].Name
+			} else {
+				// Create unique name for the temporary receiver, which will be inlined away.
+				//
+				// We use the lexical scope of the original function to avoid conflicts
+				// with (e.g.) named result variables. However, since the parameter syntax
+				// may have been modified/renamed from the original function, we must
+				// reject those names too.
+				usedParams := make(map[string]bool)
+				for _, fld := range wrapper.Type.Params.List {
+					for _, name := range fld.Names {
+						usedParams[name.Name] = true
+					}
+				}
+				scope := rw.pkg.TypesInfo().Scopes[rw.origDecl.Type]
+				if scope == nil {
+					return nil, bug.Errorf("missing function scope for %v", rw.origDecl.Name.Name)
+				}
+				for i := 0; ; i++ {
+					recv = fmt.Sprintf("r%d", i)
+					_, obj := scope.LookupParent(recv, token.NoPos)
+					if obj == nil && !usedParams[recv] {
+						break
+					}
+				}
+				wrapper.Recv.List[0].Names = []*ast.Ident{{Name: recv}}
+			}
+		}
+
+		name := &ast.Ident{Name: delegate.Name.Name}
+		var fun ast.Expr = name
+		if recv != "" {
+			fun = &ast.SelectorExpr{
+				X:   &ast.Ident{Name: recv},
+				Sel: name,
+			}
+		}
 		call := &ast.CallExpr{
-			Fun:  &ast.Ident{Name: delegate.Name.Name},
+			Fun:  fun,
 			Args: rw.callArgs,
 		}
 		if rw.variadic {

gopls/internal/test/marker/testdata/codeaction/removeparam_method.txt

@@ -0,0 +1,123 @@
+This test verifies that gopls can remove unused parameters from methods.
+
+Specifically, check
+1. basic removal of unused parameters, when the receiver is named, locally and
+   accross package boundaries
+2. handling of unnamed receivers
+
+-- go.mod --
+module example.com/rm
+
+go 1.20
+
+-- basic.go --
+package rm
+
+type Basic int
+
+func (t Basic) Foo(x int) { //@codeaction("x", "x", "refactor.rewrite", basic)
+}
+
+func _(b Basic) {
+	b.Foo(1)
+	// TODO(rfindley): methodexprs should not get rewritten as methods.
+	Basic.Foo(1, 2)
+}
+
+-- basicuse/p.go --
+package basicuse
+
+import "example.com/rm"
+
+func _() {
+	x := new(rm.Basic)
+	x.Foo(sideEffects())
+	rm.Basic.Foo(1,2)
+}
+
+func sideEffects() int
+
+-- @basic/basic.go --
+package rm
+
+type Basic int
+
+func (t Basic) Foo() { //@codeaction("x", "x", "refactor.rewrite", basic)
+}
+
+func _(b Basic) {
+	b.Foo()
+	// TODO(rfindley): methodexprs should not get rewritten as methods.
+	Basic(1).Foo()
+}
+-- @basic/basicuse/p.go --
+package basicuse
+
+import "example.com/rm"
+
+func _() {
+	x := new(rm.Basic)
+	var (
+		t rm.Basic = *x
+		_ int      = sideEffects()
+	)
+	t.Foo()
+	rm.Basic(1).Foo()
+}
+
+func sideEffects() int
+-- missingrecv.go --
+package rm
+
+type Missing struct{}
+
+var r2 int
+
+func (Missing) M(a, b, c, r0 int) (r1 int) { //@codeaction("b", "b", "refactor.rewrite", missingrecv)
+	return a + c
+}
+
+func _() {
+	m := &Missing{}
+	_ = m.M(1, 2, 3, 4)
+}
+
+-- missingrecvuse/p.go --
+package missingrecvuse
+
+import "example.com/rm"
+
+func _() {
+	x := rm.Missing{}
+	x.M(1, sideEffects(), 3, 4)
+}
+
+func sideEffects() int
+
+-- @missingrecv/missingrecv.go --
+package rm
+
+type Missing struct{}
+
+var r2 int
+
+func (Missing) M(a, c, r0 int) (r1 int) { //@codeaction("b", "b", "refactor.rewrite", missingrecv)
+	return a + c
+}
+
+func _() {
+	m := &Missing{}
+	_ = (*m).M(1, 3, 4)
+}
+-- @missingrecv/missingrecvuse/p.go --
+package missingrecvuse
+
+import "example.com/rm"
+
+func _() {
+	x := rm.Missing{}
+	var _ int = sideEffects()
+	x.M(1, 3, 4)
+}
+
+func sideEffects() int

gopls/internal/test/marker/testdata/codeaction/removeparam_satisfies.txt

@@ -0,0 +1,62 @@
+This test verifies that gopls can remove unused parameters from methods,
+when that method satisfies an interface.
+
+For now, we just update static calls. In the future, we should compute the set
+of dynamic calls that must change (and therefore, the set of concrete functions
+that must be modified), in order to produce the desired outcome for our users.
+
+Doing so would be more complicated, so for now this test simply records the
+current behavior.
+
+-- go.mod --
+module example.com/rm
+
+go 1.20
+
+-- p.go --
+package rm
+
+type T int
+
+func (t T) Foo(x int) { //@codeaction("x", "x", "refactor.rewrite", basic)
+}
+
+-- use/use.go --
+package use
+
+import "example.com/rm"
+
+type Fooer interface{
+	Foo(int)
+}
+
+var _ Fooer = rm.T(0)
+
+func _() {
+	var x rm.T
+	x.Foo(1)
+}
+-- @basic/p.go --
+package rm
+
+type T int
+
+func (t T) Foo() { //@codeaction("x", "x", "refactor.rewrite", basic)
+}
+
+-- @basic/use/use.go --
+package use
+
+import "example.com/rm"
+
+type Fooer interface {
+	Foo(int)
+}
+
+var _ Fooer = rm.T(0)
+
+func _() {
+	var x rm.T
+	var t rm.T = x
+	t.Foo()
+}