From 3dafbd1ba4c62a1d3b209101efe824e9eefb4d66 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 3 Mar 2016 11:32:24 -0500 Subject: [PATCH] go/types/typeutil: make IntuitiveMethodSet(*C) nonempty for concrete types C This fixes a bug in guru describe. Also, add a test of IntuitiveMethodSet. Change-Id: Ied3780807afd88e664fdb186619499670529fe33 Reviewed-on: https://go-review.googlesource.com/20166 Reviewed-by: Daniel Morsing --- cmd/guru/testdata/src/describe/main.go | 3 ++ cmd/guru/testdata/src/describe/main.golden | 8 +++ go/types/typeutil/ui.go | 26 ++++++--- go/types/typeutil/ui_test.go | 63 ++++++++++++++++++++++ 4 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 go/types/typeutil/ui_test.go diff --git a/cmd/guru/testdata/src/describe/main.go b/cmd/guru/testdata/src/describe/main.go index d519db17e3..8e56aca9f6 100644 --- a/cmd/guru/testdata/src/describe/main.go +++ b/cmd/guru/testdata/src/describe/main.go @@ -37,6 +37,9 @@ func main() { // @describe func-def-main "main" _ = d.f // @describe func-ref-d.f "d.f" _ = i.f // @describe func-ref-i.f "i.f" + var dptr *D // @describe ptr-with-nonptr-methods "dptr" + _ = dptr + // var objects anon := func() { _ = d // @describe ref-lexical-d "d" diff --git a/cmd/guru/testdata/src/describe/main.golden b/cmd/guru/testdata/src/describe/main.golden index f7ffdcbb75..aa8e603cdf 100644 --- a/cmd/guru/testdata/src/describe/main.golden +++ b/cmd/guru/testdata/src/describe/main.golden @@ -84,6 +84,14 @@ defined here reference to interface method func (I).f() defined here +-------- @describe ptr-with-nonptr-methods -------- +definition of var dptr *D +Method set: + method (*D) f() +Fields: + Field int + AnotherField string + -------- @describe ref-lexical-d -------- reference to var d D defined here diff --git a/go/types/typeutil/ui.go b/go/types/typeutil/ui.go index 945fb29719..a9e74473cd 100644 --- a/go/types/typeutil/ui.go +++ b/go/types/typeutil/ui.go @@ -10,23 +10,36 @@ package typeutil import "go/types" -// IntuitiveMethodSet returns the intuitive method set of a type, T. +// IntuitiveMethodSet returns the intuitive method set of a type T, +// which is the set of methods you can call on an addressable value of +// that type. // -// The result contains MethodSet(T) and additionally, if T is a -// concrete type, methods belonging to *T if there is no identically -// named method on T itself. This corresponds to user intuition about -// method sets; this function is intended only for user interfaces. +// The result always contains MethodSet(T), and is exactly MethodSet(T) +// for interface types and for pointer-to-concrete types. +// For all other concrete types T, the result additionally +// contains each method belonging to *T if there is no identically +// named method on T itself. +// +// This corresponds to user intuition about method sets; +// this function is intended only for user interfaces. // // The order of the result is as for types.MethodSet(T). // func IntuitiveMethodSet(T types.Type, msets *MethodSetCache) []*types.Selection { + isPointerToConcrete := func(T types.Type) bool { + ptr, ok := T.(*types.Pointer) + return ok && !types.IsInterface(ptr.Elem()) + } + var result []*types.Selection mset := msets.MethodSet(T) - if _, ok := T.Underlying().(*types.Interface); ok { + if types.IsInterface(T) || isPointerToConcrete(T) { for i, n := 0, mset.Len(); i < n; i++ { result = append(result, mset.At(i)) } } else { + // T is some other concrete type. + // Report methods of T and *T, preferring those of T. pmset := msets.MethodSet(types.NewPointer(T)) for i, n := 0, pmset.Len(); i < n; i++ { meth := pmset.At(i) @@ -35,6 +48,7 @@ func IntuitiveMethodSet(T types.Type, msets *MethodSetCache) []*types.Selection } result = append(result, meth) } + } return result } diff --git a/go/types/typeutil/ui_test.go b/go/types/typeutil/ui_test.go new file mode 100644 index 0000000000..4d5155eb42 --- /dev/null +++ b/go/types/typeutil/ui_test.go @@ -0,0 +1,63 @@ +// +build go1.5 + +package typeutil_test + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "go/types" + "strings" + "testing" + + "golang.org/x/tools/go/types/typeutil" +) + +func TestIntuitiveMethodSet(t *testing.T) { + const source = ` +package P +type A int +func (A) f() +func (*A) g() +` + + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "hello.go", source, 0) + if err != nil { + t.Fatal(err) + } + + var conf types.Config + pkg, err := conf.Check("P", fset, []*ast.File{f}, nil) + if err != nil { + t.Fatal(err) + } + qual := types.RelativeTo(pkg) + + for _, test := range []struct { + expr string // type expression + want string // intuitive method set + }{ + {"A", "(A).f (*A).g"}, + {"*A", "(*A).f (*A).g"}, + {"error", "(error).Error"}, + {"*error", ""}, + {"struct{A}", "(struct{A}).f (*struct{A}).g"}, + {"*struct{A}", "(*struct{A}).f (*struct{A}).g"}, + } { + tv, err := types.Eval(fset, pkg, 0, test.expr) + if err != nil { + t.Errorf("Eval(%s) failed: %v", test.expr, err) + } + var names []string + for _, m := range typeutil.IntuitiveMethodSet(tv.Type, nil) { + name := fmt.Sprintf("(%s).%s", types.TypeString(m.Recv(), qual), m.Obj().Name()) + names = append(names, name) + } + got := strings.Join(names, " ") + if got != test.want { + t.Errorf("IntuitiveMethodSet(%s) = %q, want %q", test.expr, got, test.want) + } + } +}