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 <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/refactor/rename/check.go b/refactor/rename/check.go
index f2bd880..8350ad7 100644
--- a/refactor/rename/check.go
+++ b/refactor/rename/check.go
@@ -13,7 +13,6 @@
"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 @@
}
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 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
// 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 @@
//
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 @@
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 @@
}
return nil
}
-
-// -- Plundered from golang.org/x/tools/go/ssa -----------------
-
-func isInterface(T types.Type) bool { return types.IsInterface(T) }
diff --git a/refactor/rename/rename.go b/refactor/rename/rename.go
index 56603cd..a5a59e9 100644
--- a/refactor/rename/rename.go
+++ b/refactor/rename/rename.go
@@ -323,7 +323,7 @@
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
}
diff --git a/refactor/rename/rename_test.go b/refactor/rename/rename_test.go
index 38c59c9..58b4f5e 100644
--- a/refactor/rename/rename_test.go
+++ b/refactor/rename/rename_test.go
@@ -467,6 +467,7 @@
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.
{
@@ -767,6 +768,78 @@
`,
},
},
+ // 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)
+}
+`,
+ },
+ },
// Lexical scope tests.
{
@@ -1247,6 +1320,14 @@
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 == "" {
diff --git a/refactor/rename/spec.go b/refactor/rename/spec.go
index 69198cb..ab7dbc3 100644
--- a/refactor/rename/spec.go
+++ b/refactor/rename/spec.go
@@ -25,7 +25,7 @@
"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 @@
// 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 @@
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())
}
diff --git a/refactor/rename/util.go b/refactor/rename/util.go
index 26926a8..bc6dc10 100644
--- a/refactor/rename/util.go
+++ b/refactor/rename/util.go
@@ -22,7 +22,7 @@
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 @@
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 == "_" {
