From e9a61ba66bcdc23375397e38a8cd7e43b5be6b5b Mon Sep 17 00:00:00 2001 From: Ross Light Date: Tue, 29 May 2018 08:45:14 -0700 Subject: [PATCH] goose: require pointer for first argument to goose.Bind (google/go-cloud#31) Fixes google/go-cloud#15 --- README.md | 10 +++++----- goose.go | 5 +++-- internal/goose/parse.go | 16 ++++++++++------ .../testdata/ImportedInterfaceBinding/bar/bar.go | 4 ++-- .../goose/testdata/InterfaceBinding/foo/foo.go | 2 +- .../InterfaceBindingReuse/foo/foo_goose.go | 2 +- 6 files changed, 22 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 3742c5e..cecc85b 100644 --- a/README.md +++ b/README.md @@ -247,13 +247,13 @@ func ProvideBar() *Bar { var BarFooer = goose.NewSet( ProvideBar, - goose.Bind(Fooer(nil), (*Bar)(nil))) + goose.Bind(new(Fooer), new(Bar))) ``` -The first argument to `goose.Bind` is a nil value for the interface type and the -second argument is a zero value of the concrete type. An interface binding does -not necessarily need to have a provider in the same set that provides the -concrete type. +The first argument to `goose.Bind` is a pointer to a value of the desired +interface type and the second argument is a zero value of the concrete type. An +interface binding does not necessarily need to have a provider in the same set +that provides the concrete type. [type identity]: https://golang.org/ref/spec#Type_identity [return concrete types]: https://github.com/golang/go/wiki/CodeReviewComments#interfaces diff --git a/goose.go b/goose.go index cbc2618..61ff8d8 100644 --- a/goose.go +++ b/goose.go @@ -43,11 +43,12 @@ func Build(...interface{}) string { type Binding struct{} // Bind declares that a concrete type should be used to satisfy a -// dependency on iface. +// dependency on the type of iface, which must be a pointer to an +// interface type. // // Example: // -// var MySet = goose.NewSet(goose.Bind(MyInterface(nil), new(MyStruct))) +// var MySet = goose.NewSet(goose.Bind(new(MyInterface), new(MyStruct))) func Bind(iface, to interface{}) Binding { return Binding{} } diff --git a/internal/goose/parse.go b/internal/goose/parse.go index e6691e7..a6fc135 100644 --- a/internal/goose/parse.go +++ b/internal/goose/parse.go @@ -496,21 +496,25 @@ func processBind(fset *token.FileSet, info *types.Info, call *ast.CallExpr) (*If return nil, fmt.Errorf("%v: call to Bind takes exactly two arguments", fset.Position(call.Pos())) } // TODO(light): Verify that arguments are simple expressions. - iface := info.TypeOf(call.Args[0]) - methodSet, ok := iface.Underlying().(*types.Interface) + ifaceArgType := info.TypeOf(call.Args[0]) + ifacePtr, ok := ifaceArgType.(*types.Pointer) if !ok { - return nil, fmt.Errorf("%v: first argument to bind must be of interface type; found %s", fset.Position(call.Pos()), types.TypeString(iface, nil)) + return nil, fmt.Errorf("%v: first argument to bind must be a pointer to an interface type; found %s", fset.Position(call.Pos()), types.TypeString(ifaceArgType, nil)) + } + methodSet, ok := ifacePtr.Elem().Underlying().(*types.Interface) + if !ok { + return nil, fmt.Errorf("%v: first argument to bind must be a pointer to an interface type; found %s", fset.Position(call.Pos()), types.TypeString(ifaceArgType, nil)) } provided := info.TypeOf(call.Args[1]) - if types.Identical(iface, provided) { + if types.Identical(ifacePtr.Elem(), provided) { return nil, fmt.Errorf("%v: cannot bind interface to itself", fset.Position(call.Pos())) } if !types.Implements(provided, methodSet) { - return nil, fmt.Errorf("%v: %s does not implement %s", fset.Position(call.Pos()), types.TypeString(provided, nil), types.TypeString(iface, nil)) + return nil, fmt.Errorf("%v: %s does not implement %s", fset.Position(call.Pos()), types.TypeString(provided, nil), types.TypeString(ifaceArgType, nil)) } return &IfaceBinding{ Pos: call.Pos(), - Iface: iface, + Iface: ifacePtr.Elem(), Provided: provided, }, nil } diff --git a/internal/goose/testdata/ImportedInterfaceBinding/bar/bar.go b/internal/goose/testdata/ImportedInterfaceBinding/bar/bar.go index 7c1a18c..cbd9e16 100644 --- a/internal/goose/testdata/ImportedInterfaceBinding/bar/bar.go +++ b/internal/goose/testdata/ImportedInterfaceBinding/bar/bar.go @@ -17,8 +17,8 @@ package main import ( "fmt" - "github.com/google/go-cloud/goose" "foo" + "github.com/google/go-cloud/goose" ) func main() { @@ -39,4 +39,4 @@ func provideBar() *Bar { var Set = goose.NewSet( provideBar, - goose.Bind(foo.Fooer(nil), (*Bar)(nil))) + goose.Bind((*foo.Fooer)(nil), (*Bar)(nil))) diff --git a/internal/goose/testdata/InterfaceBinding/foo/foo.go b/internal/goose/testdata/InterfaceBinding/foo/foo.go index 413d1d0..5ff809f 100644 --- a/internal/goose/testdata/InterfaceBinding/foo/foo.go +++ b/internal/goose/testdata/InterfaceBinding/foo/foo.go @@ -42,4 +42,4 @@ func provideBar() *Bar { var Set = goose.NewSet( provideBar, - goose.Bind(Fooer(nil), (*Bar)(nil))) + goose.Bind((*Fooer)(nil), (*Bar)(nil))) diff --git a/internal/goose/testdata/InterfaceBindingReuse/foo/foo_goose.go b/internal/goose/testdata/InterfaceBindingReuse/foo/foo_goose.go index 85f08cc..7c00827 100644 --- a/internal/goose/testdata/InterfaceBindingReuse/foo/foo_goose.go +++ b/internal/goose/testdata/InterfaceBindingReuse/foo/foo_goose.go @@ -24,5 +24,5 @@ func injectFooBar() FooBar { panic(goose.Build( provideBar, provideFooBar, - goose.Bind(Fooer(nil), (*Bar)(nil)))) + goose.Bind((*Fooer)(nil), (*Bar)(nil)))) }