go/types/objectpath: canonical order for methods
The method ordering in types can depend on the parse order for types. In
order to make objectpath robust against this, simply encode all methods
with respect to a canonical lexicographical ordering.
Fixes golang/go#44195
Change-Id: I4177d9b4e094452f71d4db1813a5a36b54d0d70a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/354433
Run-TryBot: Tim King <taking@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Trust: Zvonimir Pavlinovic <zpavlinovic@google.com>
diff --git a/go/types/objectpath/objectpath.go b/go/types/objectpath/objectpath.go
index aa374c5..7e96fc2 100644
--- a/go/types/objectpath/objectpath.go
+++ b/go/types/objectpath/objectpath.go
@@ -23,11 +23,11 @@
import (
"fmt"
+ "go/types"
+ "sort"
"strconv"
"strings"
- "go/types"
-
"golang.org/x/tools/internal/typeparams"
)
@@ -67,6 +67,8 @@
// which is encoded as a string of decimal digits.
// These indices are stable across different representations
// of the same package, even source and export data.
+// The indices used are implementation specific and may not correspond to
+// the argument to the go/types function.
//
// In the example below,
//
@@ -287,8 +289,12 @@
// Inspect declared methods of defined types.
if T, ok := o.Type().(*types.Named); ok {
path = append(path, opType)
- for i := 0; i < T.NumMethods(); i++ {
- m := T.Method(i)
+ // Note that method index here is always with respect
+ // to canonical ordering of methods, regardless of how
+ // they appear in the underlying type.
+ canonical := canonicalize(T)
+ for i := 0; i < len(canonical); i++ {
+ m := canonical[i]
path2 := appendOpArg(path, opMethod, i)
if m == obj {
return Path(path2), nil // found declared method
@@ -421,11 +427,6 @@
type hasElem interface {
Elem() types.Type
}
- // abstraction of *types.{Interface,Named}
- type hasMethods interface {
- Method(int) *types.Func
- NumMethods() int
- }
// abstraction of *types.{Named,Signature}
type hasTypeParams interface {
TypeParams() *typeparams.TypeParamList
@@ -563,10 +564,11 @@
if !ok {
return nil, fmt.Errorf("cannot apply %q to %s (got %T, want interface or named)", code, t, t)
}
- if n := hasMethods.NumMethods(); index >= n {
+ canonical := canonicalize(hasMethods)
+ if n := len(canonical); index >= n {
return nil, fmt.Errorf("method index %d out of range [0-%d)", index, n)
}
- obj = hasMethods.Method(index)
+ obj = canonical[index]
t = nil
case opObj:
@@ -588,3 +590,28 @@
return obj, nil // success
}
+
+// hasMethods is an abstraction of *types.{Interface,Named}. This is pulled up
+// because it is used by methodOrdering, which is in turn used by both encoding
+// and decoding.
+type hasMethods interface {
+ Method(int) *types.Func
+ NumMethods() int
+}
+
+// canonicalize returns a canonical order for the methods in a hasMethod.
+func canonicalize(hm hasMethods) []*types.Func {
+ count := hm.NumMethods()
+ if count <= 0 {
+ return nil
+ }
+ canon := make([]*types.Func, count)
+ for i := 0; i < count; i++ {
+ canon[i] = hm.Method(i)
+ }
+ less := func(i, j int) bool {
+ return canon[i].Id() < canon[j].Id()
+ }
+ sort.Slice(canon, less)
+ return canon
+}
diff --git a/go/types/objectpath/objectpath_test.go b/go/types/objectpath/objectpath_test.go
index 1e335ef..39e7b1b 100644
--- a/go/types/objectpath/objectpath_test.go
+++ b/go/types/objectpath/objectpath_test.go
@@ -46,6 +46,10 @@
func unexportedFunc()
type unexportedType struct{}
+
+type S struct{t struct{x int}}
+type R []struct{y int}
+type Q [2]struct{z int}
`},
"a": {"a.go": `
package a
@@ -80,6 +84,9 @@
{"b", "T.M0.RA0", "var *interface{f()}", ""}, // parameter
{"b", "T.M0.RA0.EM0", "func (interface).f()", ""}, // interface method
{"b", "unexportedType", "type b.unexportedType struct{}", ""},
+ {"b", "S.UF0.F0", "field x int", ""},
+ {"b", "R.UEF0", "field y int", ""},
+ {"b", "Q.UEF0", "field z int", ""},
{"a", "T", "type a.T struct{x int; y int}", ""},
{"a", "T.UF0", "field x int", ""},
@@ -99,6 +106,13 @@
{"b", "A.UF0", "", "cannot apply 'U' to struct{x int} (got *types.Struct, want named)"},
{"b", "M.UPO", "", "cannot apply 'P' to map[struct{x int}]struct{y int} (got *types.Map, want signature)"},
{"b", "C.O", "", "path denotes type a.Int int, which belongs to a different package"},
+ {"b", "T.M9", "", "method index 9 out of range [0-1)"},
+ {"b", "M.UF0", "", "cannot apply 'F' to map[struct{x int}]struct{y int} (got *types.Map, want struct)"},
+ {"b", "V.KO", "", "cannot apply 'K' to []*a.T (got *types.Slice, want map)"},
+ {"b", "V.A4", "", "cannot apply 'A' to []*a.T (got *types.Slice, want tuple)"},
+ {"b", "V.RA0", "", "cannot apply 'R' to []*a.T (got *types.Slice, want signature)"},
+ {"b", "F.PA4", "", "tuple index 4 out of range [0-4)"},
+ {"b", "F.XO", "", "invalid path: unknown code 'X'"},
}
conf := loader.Config{Build: buildutil.FakeContext(pkgs)}
conf.Import("a")
@@ -310,3 +324,65 @@
return s
}
+
+// TestOrdering uses objectpath over two Named types with the same method
+// names but in a different source order and checks that objectpath is the
+// same for methods with the same name.
+func TestOrdering(t *testing.T) {
+ pkgs := map[string]map[string]string{
+ "p": {"p.go": `
+package p
+
+type T struct{ A int }
+
+func (T) M() { }
+func (T) N() { }
+func (T) X() { }
+func (T) Y() { }
+`},
+ "q": {"q.go": `
+package q
+
+type T struct{ A int }
+
+func (T) N() { }
+func (T) M() { }
+func (T) Y() { }
+func (T) X() { }
+`}}
+ conf := loader.Config{Build: buildutil.FakeContext(pkgs)}
+ conf.Import("p")
+ conf.Import("q")
+ prog, err := conf.Load()
+ if err != nil {
+ t.Fatal(err)
+ }
+ p := prog.Imported["p"].Pkg
+ q := prog.Imported["q"].Pkg
+
+ // From here, the objectpaths generated for p and q should be the
+ // same. If they are not, then we are generating an ordering that is
+ // dependent on the declaration of the types within the file.
+ for _, test := range []struct {
+ path objectpath.Path
+ }{
+ {"T.M0"},
+ {"T.M1"},
+ {"T.M2"},
+ {"T.M3"},
+ } {
+ pobj, err := objectpath.Object(p, test.path)
+ if err != nil {
+ t.Errorf("Object(%s) failed in a1: %v", test.path, err)
+ continue
+ }
+ qobj, err := objectpath.Object(q, test.path)
+ if err != nil {
+ t.Errorf("Object(%s) failed in a2: %v", test.path, err)
+ continue
+ }
+ if pobj.Name() != pobj.Name() {
+ t.Errorf("Objects(%s) not equal, got a1 = %v, a2 = %v", test.path, pobj.Name(), qobj.Name())
+ }
+ }
+}
