diff --git a/go/types/objectpath/objectpath.go b/go/types/objectpath/objectpath.go index aa374c5363..7e96fc234e 100644 --- a/go/types/objectpath/objectpath.go +++ b/go/types/objectpath/objectpath.go @@ -23,11 +23,11 @@ package objectpath import ( "fmt" + "go/types" + "sort" "strconv" "strings" - "go/types" - "golang.org/x/tools/internal/typeparams" ) @@ -67,6 +67,8 @@ type Path string // 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 @@ func For(obj types.Object) (Path, error) { // 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 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { 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 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { 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 @@ func Object(pkg *types.Package, p Path) (types.Object, error) { 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 1e335ef7a3..39e7b1bcdf 100644 --- a/go/types/objectpath/objectpath_test.go +++ b/go/types/objectpath/objectpath_test.go @@ -46,6 +46,10 @@ type M map[struct{x int}]struct{y int} 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 @@ type T struct{x, y int} {"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 @@ type T struct{x, y int} {"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 @@ func objectString(obj types.Object) string { 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()) + } + } +}