gopls/internal/golang: Rename all receiver variables

This CL causes the Rename operation, when applied to a
method receiver variable, to rename all receiver variables
of methods of the same named type, where possible.
(Errors in secondary receivers, for example due to shadowing
conflicts, are silently ignored.)

+ test, doc, relnote

Fixes golang/go#41892

Change-Id: Ie5755508afc9ebdcfe0578cbd9cac71ee122fc38
Reviewed-on: https://go-review.googlesource.com/c/tools/+/665935
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Jonathan Amsterdam <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>

Author: adonovan

gopls/doc/features/transformation.md

@@ -315,11 +315,18 @@ Similar problems may arise with packages that use reflection, such as
 `encoding/json` or `text/template`. There is no substitute for good
 judgment and testing.
 
+Special cases:
+
+- When renaming the receiver of a method, the tool also attempts to
+  rename the receivers of all other methods associated with the same
+  named type. Each other receiver that cannot be fully renamed is
+  quietly skipped.
+
+- Renaming a package declaration additionally causes the package's
+  directory to be renamed.
+
 Some tips for best results:
 
-- There is currently no special support for renaming all receivers of
-  a family of methods at once, so you will need to rename one receiver
-  one at a time (golang/go#41892).
 - The safety checks performed by the Rename algorithm require type
   information. If the program is grossly malformed, there may be
   insufficient information for it to run (golang/go#41870),

gopls/doc/release/v0.19.0.md

@@ -7,6 +7,13 @@
 
 # New features
 
+##  "Rename" of method receivers
+
+The Rename operation, when applied to the receiver of a method, now
+also attempts to rename the receivers of all other methods associated
+with the same named type. Each other receiver that cannot be fully
+renamed is quietly skipped.
+
 ##  "Implementations" supports signature types
 
 The Implementations query reports the correspondence between abstract

gopls/internal/golang/rename.go

@@ -69,8 +69,10 @@ import (
 	"golang.org/x/tools/gopls/internal/protocol"
 	goplsastutil "golang.org/x/tools/gopls/internal/util/astutil"
 	"golang.org/x/tools/gopls/internal/util/bug"
+	"golang.org/x/tools/gopls/internal/util/moreiters"
 	"golang.org/x/tools/gopls/internal/util/safetoken"
 	internalastutil "golang.org/x/tools/internal/astutil"
+	"golang.org/x/tools/internal/astutil/cursor"
 	"golang.org/x/tools/internal/diff"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/typesinternal"
@@ -482,6 +484,7 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
 	// computes the union across all variants.)
 	var targets map[types.Object]ast.Node
 	var pkg *cache.Package
+	var cur cursor.Cursor // of selected Ident or ImportSpec
 	{
 		mps, err := snapshot.MetadataForFile(ctx, f.URI())
 		if err != nil {
@@ -505,6 +508,11 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
 		if err != nil {
 			return nil, err
 		}
+		var ok bool
+		cur, ok = pgf.Cursor.FindPos(pos, pos)
+		if !ok {
+			return nil, fmt.Errorf("can't find cursor for selection")
+		}
 		objects, _, err := objectsAt(pkg.TypesInfo(), pgf.File, pos)
 		if err != nil {
 			return nil, err
@@ -571,8 +579,34 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
 		for obj := range targets {
 			objects = append(objects, obj)
 		}
+
 		editMap, _, err := renameObjects(newName, pkg, objects...)
-		return editMap, err
+		if err != nil {
+			return nil, err
+		}
+
+		// If target is a receiver, also rename receivers of
+		// other methods of the same type that don't already
+		// have the target name. Quietly discard edits from
+		// any that can't be renamed.
+		//
+		// TODO(adonovan): UX question: require that the
+		// selection be the declaration of the receiver before
+		// we broaden the renaming?
+		if curDecl, ok := moreiters.First(cur.Enclosing((*ast.FuncDecl)(nil))); ok {
+			decl := curDecl.Node().(*ast.FuncDecl) // enclosing func
+			if decl.Recv != nil &&
+				len(decl.Recv.List) > 0 &&
+				len(decl.Recv.List[0].Names) > 0 {
+				recv := pkg.TypesInfo().Defs[decl.Recv.List[0].Names[0]]
+				if recv == obj {
+					// TODO(adonovan): simplify the above 7 lines to
+					// to "if obj.(*Var).Kind==Recv" in go1.25.
+					renameReceivers(pkg, recv.(*types.Var), newName, editMap)
+				}
+			}
+		}
+		return editMap, nil
 	}
 
 	// Exported: search globally.
@@ -632,6 +666,39 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
 	return renameExported(pkgs, declPkgPath, declObjPath, newName)
 }
 
+// renameReceivers renames all receivers of methods of the same named
+// type as recv. The edits of each successful renaming are added to
+// editMap; the failed ones are quietly discarded.
+func renameReceivers(pkg *cache.Package, recv *types.Var, newName string, editMap map[protocol.DocumentURI][]diff.Edit) {
+	_, named := typesinternal.ReceiverNamed(recv)
+	if named == nil {
+		return
+	}
+
+	// Find receivers of other methods of the same named type.
+	for m := range named.Origin().Methods() {
+		recv2 := m.Signature().Recv()
+		if recv2 == recv {
+			continue // don't re-rename original receiver
+		}
+		if recv2.Name() == newName {
+			continue // no renaming needed
+		}
+		editMap2, _, err := renameObjects(newName, pkg, recv2)
+		if err != nil {
+			continue // ignore secondary failures
+		}
+
+		// Since all methods (and their comments)
+		// are disjoint, and don't affect imports,
+		// we can safely assume that all edits are
+		// nonconflicting and disjoint.
+		for uri, edits := range editMap2 {
+			editMap[uri] = append(editMap[uri], edits...)
+		}
+	}
+}
+
 // typeCheckReverseDependencies returns the type-checked packages for
 // the reverse dependencies of all packages variants containing
 // file declURI. The packages are in some topological order.

gopls/internal/test/marker/testdata/rename/recv.txt

@@ -0,0 +1,68 @@
+This test exercises renaming of method receivers (golang/go#41892).
+
+Notes:
+- x to print fails for A.J because it would shadow the built-in print;
+  that renaming is quietly skipped.
+- various combinations of named, aliases, and pointers are tested.
+- package b exercises generics.
+
+-- a/a.go --
+package a
+
+type T int
+type A = T
+
+func (T) F()   {}
+func (t T) G() {} //@rename("t", "x", tx)
+func (U T) H() {} //@rename("U", "v", Uv)
+func (_ T) I() {}
+func (v A) J() { print(v) }
+func (w *T) K() {}
+func (x *A) L() {} //@rename("x", "print", xprint)
+
+-- @tx/a/a.go --
+@@ -7,2 +7,2 @@
+-func (t T) G() {} //@rename("t", "x", tx)
+-func (U T) H() {} //@rename("U", "v", Uv)
++func (x T) G() {} //@rename("t", "x", tx)
++func (x T) H() {} //@rename("U", "v", Uv)
+@@ -10,2 +10,2 @@
+-func (v A) J() { print(v) }
+-func (w *T) K() {}
++func (x A) J() { print(x) }
++func (x *T) K() {}
+-- @Uv/a/a.go --
+@@ -7,2 +7,2 @@
+-func (t T) G() {} //@rename("t", "x", tx)
+-func (U T) H() {} //@rename("U", "v", Uv)
++func (v T) G() {} //@rename("t", "x", tx)
++func (v T) H() {} //@rename("U", "v", Uv)
+@@ -11,2 +11,2 @@
+-func (w *T) K() {}
+-func (x *A) L() {} //@rename("x", "print", xprint)
++func (v *T) K() {}
++func (v *A) L() {} //@rename("x", "print", xprint)
+-- @xprint/a/a.go --
+@@ -7,2 +7,2 @@
+-func (t T) G() {} //@rename("t", "x", tx)
+-func (U T) H() {} //@rename("U", "v", Uv)
++func (print T) G() {} //@rename("t", "x", tx)
++func (print T) H() {} //@rename("U", "v", Uv)
+@@ -11,2 +11,2 @@
+-func (w *T) K() {}
+-func (x *A) L() {} //@rename("x", "print", xprint)
++func (print *T) K() {}
++func (print *A) L() {} //@rename("x", "print", xprint)
+-- b/b.go --
+package b
+
+type C[T any] int
+func (r C[T]) F() {} //@rename("r", "c", rc)
+func (r C[T]) G() {}
+
+-- @rc/b/b.go --
+@@ -4,2 +4,2 @@
+-func (r C[T]) F() {} //@rename("r", "c", rc)
+-func (r C[T]) G() {}
++func (c C[T]) F() {} //@rename("r", "c", rc)
++func (c C[T]) G() {}