refactor/rename: fix renaming of aliases

This change gets the renaming of aliases right when
they are materialized by GODEBUG=gotypesaliases=1.
(Obviously there's no way to get it right without
that feature.) And it adds a (conditional) test.

Updates golang/go#65294

Change-Id: Ie242e3a0a9d012218067bdc61a901f6618fecbb5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/567842
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: adonovan

refactor/rename/check.go

@@ -13,7 +13,6 @@ import (
 	"go/types"
 
 	"golang.org/x/tools/go/loader"
-	"golang.org/x/tools/internal/aliases"
 	"golang.org/x/tools/internal/typeparams"
 	"golang.org/x/tools/internal/typesinternal"
 	"golang.org/x/tools/refactor/satisfy"
@@ -438,8 +437,8 @@ func (r *renamer) checkStructField(from *types.Var) {
 		}
 		i++
 	}
-	if spec, ok := path[i].(*ast.TypeSpec); ok {
-		// This struct is also a named type.
+	if spec, ok := path[i].(*ast.TypeSpec); ok && !spec.Assign.IsValid() {
+		// This struct is also a defined type.
 		// We must check for direct (non-promoted) field/field
 		// and method/field conflicts.
 		named := info.Defs[spec.Name].Type()
@@ -452,7 +451,7 @@ func (r *renamer) checkStructField(from *types.Var) {
 			return // skip checkSelections to avoid redundant errors
 		}
 	} else {
-		// This struct is not a named type.
+		// This struct is not a defined type. (It may be an alias.)
 		// We need only check for direct (non-promoted) field/field conflicts.
 		T := info.Types[tStruct].Type.Underlying().(*types.Struct)
 		for i := 0; i < T.NumFields(); i++ {
@@ -470,10 +469,7 @@ func (r *renamer) checkStructField(from *types.Var) {
 	// 	type T int       // this and
 	// 	var s struct {T} // this must change too.
 	if from.Anonymous() {
-		// hasTypeName = {Named,Alias,TypeParam}, though
-		// a TypeParam cannot appear as an anonymous field.
-		// TODO(adonovan): add test of aliases.
-		type hasTypeName interface{ Obj() *types.TypeName }
+		// A TypeParam cannot appear as an anonymous field.
 		if t, ok := typesinternal.Unpointer(from.Type()).(hasTypeName); ok {
 			r.check(t.Obj())
 		}
@@ -483,6 +479,9 @@ func (r *renamer) checkStructField(from *types.Var) {
 	r.checkSelections(from)
 }
 
+// hasTypeName abstracts the named types, *types.{Named,Alias,TypeParam}.
+type hasTypeName interface{ Obj() *types.TypeName }
+
 // checkSelections checks that all uses and selections that resolve to
 // the specified object would continue to do so after the renaming.
 func (r *renamer) checkSelections(from types.Object) {
@@ -596,7 +595,7 @@ func (r *renamer) checkMethod(from *types.Func) {
 	// Check for conflict at point of declaration.
 	// Check to ensure preservation of assignability requirements.
 	R := recv(from).Type()
-	if isInterface(R) {
+	if types.IsInterface(R) {
 		// Abstract method
 
 		// declaration
@@ -613,7 +612,7 @@ func (r *renamer) checkMethod(from *types.Func) {
 		for _, info := range r.packages {
 			// Start with named interface types (better errors)
 			for _, obj := range info.Defs {
-				if obj, ok := obj.(*types.TypeName); ok && isInterface(obj.Type()) {
+				if obj, ok := obj.(*types.TypeName); ok && types.IsInterface(obj.Type()) {
 					f, _, _ := types.LookupFieldOrMethod(
 						obj.Type(), false, from.Pkg(), from.Name())
 					if f == nil {
@@ -685,7 +684,7 @@ func (r *renamer) checkMethod(from *types.Func) {
 			// yields abstract method I.f.  This can make error
 			// messages less than obvious.
 			//
-			if !isInterface(key.RHS) {
+			if !types.IsInterface(key.RHS) {
 				// The logic below was derived from checkSelections.
 
 				rtosel := rmethods.Lookup(from.Pkg(), r.to)
@@ -760,7 +759,7 @@ func (r *renamer) checkMethod(from *types.Func) {
 		//
 		for key := range r.satisfy() {
 			// key = (lhs, rhs) where lhs is always an interface.
-			if isInterface(key.RHS) {
+			if types.IsInterface(key.RHS) {
 				continue
 			}
 			rsel := r.msets.MethodSet(key.RHS).Lookup(from.Pkg(), from.Name())
@@ -782,7 +781,7 @@ func (r *renamer) checkMethod(from *types.Func) {
 				var iface string
 
 				I := recv(imeth).Type()
-				if named, ok := aliases.Unalias(I).(*types.Named); ok {
+				if named, ok := I.(hasTypeName); ok {
 					pos = named.Obj().Pos()
 					iface = "interface " + named.Obj().Name()
 				} else {
@@ -850,7 +849,3 @@ func someUse(info *loader.PackageInfo, obj types.Object) *ast.Ident {
 	}
 	return nil
 }
-
-// -- Plundered from golang.org/x/tools/go/ssa -----------------
-
-func isInterface(T types.Type) bool { return types.IsInterface(T) }

refactor/rename/rename.go

@@ -323,7 +323,7 @@ func Main(ctxt *build.Context, offsetFlag, fromFlag, to string) error {
 	for _, obj := range fromObjects {
 		if obj, ok := obj.(*types.Func); ok {
 			recv := obj.Type().(*types.Signature).Recv()
-			if recv != nil && isInterface(recv.Type().Underlying()) {
+			if recv != nil && types.IsInterface(recv.Type()) {
 				r.changeMethods = true
 				break
 			}

refactor/rename/rename_test.go

@@ -467,6 +467,7 @@ func TestRewrites(t *testing.T) {
 		ctxt             *build.Context    // nil => use previous
 		offset, from, to string            // values of the -from/-offset and -to flags
 		want             map[string]string // contents of updated files
+		alias            bool              // requires materialized aliases
 	}{
 		// Elimination of renaming import.
 		{
@@ -764,6 +765,78 @@ type T2 int
 type U struct{ *T2 }
 
 var _ = U{}.T2
+`,
+			},
+		},
+		// Renaming of embedded field alias.
+		{
+			alias: true,
+			ctxt: main(`package main
+
+type T int
+type A = T
+type U struct{ A }
+
+var _ = U{}.A
+var a A
+`),
+			offset: "/go/src/main/0.go:#68", to: "A2", // A in "U{}.A"
+			want: map[string]string{
+				"/go/src/main/0.go": `package main
+
+type T int
+type A2 = T
+type U struct{ A2 }
+
+var _ = U{}.A2
+var a A2
+`,
+			},
+		},
+		// Renaming of embedded field pointer to alias.
+		{
+			alias: true,
+			ctxt: main(`package main
+
+type T int
+type A = T
+type U struct{ *A }
+
+var _ = U{}.A
+var a A
+`),
+			offset: "/go/src/main/0.go:#69", to: "A2", // A in "U{}.A"
+			want: map[string]string{
+				"/go/src/main/0.go": `package main
+
+type T int
+type A2 = T
+type U struct{ *A2 }
+
+var _ = U{}.A2
+var a A2
+`,
+			},
+		},
+		// Renaming of alias
+		{
+			ctxt: main(`package main
+
+type A = int
+
+func _() A {
+	return A(0)
+}
+`),
+			offset: "/go/src/main/0.go:#49", to: "A2", // A in "A(0)"
+			want: map[string]string{
+				"/go/src/main/0.go": `package main
+
+type A2 = int
+
+func _() A2 {
+	return A2(0)
+}
 `,
 			},
 		},
@@ -1247,6 +1320,14 @@ func main() {
 			return nil
 		}
 
+		if !test.alias {
+			t.Setenv("GODEBUG", "gotypesalias=0")
+		} else if !strings.Contains(fmt.Sprint(build.Default.ReleaseTags), " go1.22") {
+			t.Skip("skipping test that requires materialized type aliases")
+		} else {
+			t.Setenv("GODEBUG", "gotypesalias=1")
+		}
+
 		err := Main(ctxt, test.offset, test.from, test.to)
 		var prefix string
 		if test.offset == "" {

refactor/rename/spec.go

@@ -25,7 +25,7 @@ import (
 
 	"golang.org/x/tools/go/buildutil"
 	"golang.org/x/tools/go/loader"
-	"golang.org/x/tools/internal/aliases"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 // A spec specifies an entity to rename.
@@ -459,17 +459,14 @@ func findObjects(info *loader.PackageInfo, spec *spec) ([]types.Object, error) {
 		// search within named type.
 		obj, _, _ := types.LookupFieldOrMethod(tName.Type(), true, info.Pkg, spec.typeMember)
 		if obj == nil {
-			return nil, fmt.Errorf("cannot find field or method %q of %s %s.%s",
-				spec.typeMember, typeKind(tName.Type()), info.Pkg.Path(), tName.Name())
+			return nil, fmt.Errorf("cannot find field or method %q of %s.%s",
+				spec.typeMember, info.Pkg.Path(), tName.Name())
 		}
 
 		if spec.searchFor == "" {
 			// If it is an embedded field, return the type of the field.
 			if v, ok := obj.(*types.Var); ok && v.Anonymous() {
-				switch t := aliases.Unalias(v.Type()).(type) {
-				case *types.Pointer:
-					return []types.Object{aliases.Unalias(t.Elem()).(*types.Named).Obj()}, nil
-				case *types.Named:
+				if t, ok := typesinternal.Unpointer(v.Type()).(hasTypeName); ok {
 					return []types.Object{t.Obj()}, nil
 				}
 			}
@@ -482,7 +479,7 @@ func findObjects(info *loader.PackageInfo, spec *spec) ([]types.Object, error) {
 				spec.searchFor, objectKind(obj), info.Pkg.Path(), tName.Name(),
 				obj.Name())
 		}
-		if isInterface(tName.Type()) {
+		if types.IsInterface(tName.Type()) {
 			return nil, fmt.Errorf("cannot search for local name %q within abstract method (%s.%s).%s",
 				spec.searchFor, info.Pkg.Path(), tName.Name(), searchFunc.Name())
 		}

refactor/rename/util.go

@@ -22,7 +22,7 @@ func objectKind(obj types.Object) string {
 	switch obj := obj.(type) {
 	case *types.PkgName:
 		return "imported package name"
-	case *types.TypeName:
+	case *types.TypeName: // defined type | alias | type parameter
 		return "type"
 	case *types.Var:
 		if obj.IsField() {
@@ -37,10 +37,6 @@ func objectKind(obj types.Object) string {
 	return strings.ToLower(strings.TrimPrefix(reflect.TypeOf(obj).String(), "*types."))
 }
 
-func typeKind(T types.Type) string {
-	return strings.ToLower(strings.TrimPrefix(reflect.TypeOf(T.Underlying()).String(), "*types."))
-}
-
 // NB: for renamings, blank is not considered valid.
 func isValidIdentifier(id string) bool {
 	if id == "" || id == "_" {