From cd32a686b165faed27eda8213d23a55bdb596ee8 Mon Sep 17 00:00:00 2001 From: Robert van Gent Date: Tue, 14 Aug 2018 14:55:28 -0700 Subject: [PATCH] wire: Add wire.InterfaceValue, required instead of wire.Value if the value is an interface value (google/go-cloud#322) * Add wire.InterfaceValue, required instead of wire.Value if the value is an interface value. * Update guestbook sample to use InterfaceValue where appropriate. * Remove unnecessary ok := true * Addressing comments from code review. --- README.md | 8 ++++ internal/wire/parse.go | 42 ++++++++++++++++++- .../wire/testdata/InterfaceValue/foo/foo.go | 26 ++++++++++++ .../wire/testdata/InterfaceValue/foo/wire.go | 29 +++++++++++++ internal/wire/testdata/InterfaceValue/pkg | 1 + .../InterfaceValue/want/program_out.txt | 1 + .../testdata/InterfaceValue/want/wire_gen.go | 22 ++++++++++ .../InterfaceValueInvalidArg0/foo/foo.go | 21 ++++++++++ .../InterfaceValueInvalidArg0/foo/wire.go | 26 ++++++++++++ .../testdata/InterfaceValueInvalidArg0/pkg | 1 + .../want/wire_errs.txt | 1 + .../InterfaceValueNotEnoughArgs/foo/foo.go | 21 ++++++++++ .../InterfaceValueNotEnoughArgs/foo/wire.go | 26 ++++++++++++ .../testdata/InterfaceValueNotEnoughArgs/pkg | 1 + .../want/wire_errs.txt | 1 + .../testdata/ValueIsInterfaceValue/foo/foo.go | 27 ++++++++++++ .../ValueIsInterfaceValue/foo/wire.go | 29 +++++++++++++ .../wire/testdata/ValueIsInterfaceValue/pkg | 1 + .../ValueIsInterfaceValue/want/wire_errs.txt | 1 + wire.go | 14 ++++++- 20 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 internal/wire/testdata/InterfaceValue/foo/foo.go create mode 100644 internal/wire/testdata/InterfaceValue/foo/wire.go create mode 100644 internal/wire/testdata/InterfaceValue/pkg create mode 100644 internal/wire/testdata/InterfaceValue/want/program_out.txt create mode 100644 internal/wire/testdata/InterfaceValue/want/wire_gen.go create mode 100644 internal/wire/testdata/InterfaceValueInvalidArg0/foo/foo.go create mode 100644 internal/wire/testdata/InterfaceValueInvalidArg0/foo/wire.go create mode 100644 internal/wire/testdata/InterfaceValueInvalidArg0/pkg create mode 100644 internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt create mode 100644 internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/foo.go create mode 100644 internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/wire.go create mode 100644 internal/wire/testdata/InterfaceValueNotEnoughArgs/pkg create mode 100644 internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt create mode 100644 internal/wire/testdata/ValueIsInterfaceValue/foo/foo.go create mode 100644 internal/wire/testdata/ValueIsInterfaceValue/foo/wire.go create mode 100644 internal/wire/testdata/ValueIsInterfaceValue/pkg create mode 100644 internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt diff --git a/README.md b/README.md index 5acefdd..42c87f4 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,14 @@ package; references to variables will be evaluated during the injector package's initialization. Wire will emit an error if the expression calls any functions or receives from any channels. +For interface values, use `InterfaceValue`: + +```go +func injectReader() io.Reader { + wire.Build(wire.InterfaceValue(new(io.Reader), os.Stdin)) + return Foo{} +} +``` ### Cleanup functions If a provider creates a value that needs to be cleaned up (e.g. closing a file), diff --git a/internal/wire/parse.go b/internal/wire/parse.go index 3bd614f..e73f713 100644 --- a/internal/wire/parse.go +++ b/internal/wire/parse.go @@ -454,6 +454,12 @@ func (oc *objectCache) processExpr(pkg *loader.PackageInfo, expr ast.Expr, varNa return nil, []error{notePosition(exprPos, err)} } return v, nil + case "InterfaceValue": + v, err := processInterfaceValue(oc.prog.Fset, &pkg.Info, call) + if err != nil { + return nil, []error{notePosition(exprPos, err)} + } + return v, nil default: return nil, []error{notePosition(exprPos, errors.New("unknown pattern"))} } @@ -739,6 +745,11 @@ func processValue(fset *token.FileSet, info *types.Info, call *ast.CallExpr) (*V if !ok { return nil, notePosition(fset.Position(call.Pos()), errors.New("argument to Value is too complex")) } + // Result type can't be an interface type; use wire.InterfaceValue for that. + argType := info.TypeOf(call.Args[0]) + if _, isInterfaceType := argType.Underlying().(*types.Interface); isInterfaceType { + return nil, notePosition(fset.Position(call.Pos()), fmt.Errorf("argument to Value may not be an interface value (found %s); use InterfaceValue instead", types.TypeString(argType, nil))) + } return &Value{ Pos: call.Args[0].Pos(), Out: info.TypeOf(call.Args[0]), @@ -747,6 +758,35 @@ func processValue(fset *token.FileSet, info *types.Info, call *ast.CallExpr) (*V }, nil } +// processInterfaceValue creates a value from a wire.InterfaceValue call. +func processInterfaceValue(fset *token.FileSet, info *types.Info, call *ast.CallExpr) (*Value, error) { + // Assumes that call.Fun is wire.InterfaceValue. + + if len(call.Args) != 2 { + return nil, notePosition(fset.Position(call.Pos()), errors.New("call to InterfaceValue takes exactly two arguments")) + } + ifaceArgType := info.TypeOf(call.Args[0]) + ifacePtr, ok := ifaceArgType.(*types.Pointer) + if !ok { + return nil, notePosition(fset.Position(call.Pos()), fmt.Errorf("first argument to InterfaceValue must be a pointer to an interface type; found %s", types.TypeString(ifaceArgType, nil))) + } + iface := ifacePtr.Elem() + methodSet, ok := iface.Underlying().(*types.Interface) + if !ok { + return nil, notePosition(fset.Position(call.Pos()), fmt.Errorf("first argument to InterfaceValue must be a pointer to an interface type; found %s", types.TypeString(ifaceArgType, nil))) + } + provided := info.TypeOf(call.Args[1]) + if !types.Implements(provided, methodSet) { + return nil, notePosition(fset.Position(call.Pos()), fmt.Errorf("%s does not implement %s", types.TypeString(provided, nil), types.TypeString(ifaceArgType, nil))) + } + return &Value{ + Pos: call.Args[1].Pos(), + Out: iface, + expr: call.Args[1], + info: info, + }, nil +} + // isInjector checks whether a given function declaration is an // injector template, returning the wire.Build call. It returns nil if // the function is not an injector template. @@ -845,7 +885,7 @@ func (pv ProviderOrValue) Provider() *Provider { return pv.p } -// Provider returns pv as a Value pointer. It panics if pv points to a +// Value returns pv as a Value pointer. It panics if pv points to a // Provider. func (pv ProviderOrValue) Value() *Value { if pv.p != nil { diff --git a/internal/wire/testdata/InterfaceValue/foo/foo.go b/internal/wire/testdata/InterfaceValue/foo/foo.go new file mode 100644 index 0000000..7ae3f61 --- /dev/null +++ b/internal/wire/testdata/InterfaceValue/foo/foo.go @@ -0,0 +1,26 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "fmt" + "io/ioutil" +) + +func main() { + r := injectedReader() + buf, _ := ioutil.ReadAll(r) + fmt.Println(string(buf)) +} diff --git a/internal/wire/testdata/InterfaceValue/foo/wire.go b/internal/wire/testdata/InterfaceValue/foo/wire.go new file mode 100644 index 0000000..6b127b2 --- /dev/null +++ b/internal/wire/testdata/InterfaceValue/foo/wire.go @@ -0,0 +1,29 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//+build wireinject + +package main + +import ( + "io" + "strings" + + "github.com/google/go-cloud/wire" +) + +func injectedReader() io.Reader { + wire.Build(wire.InterfaceValue(new(io.Reader), strings.NewReader("hello world"))) + return nil +} diff --git a/internal/wire/testdata/InterfaceValue/pkg b/internal/wire/testdata/InterfaceValue/pkg new file mode 100644 index 0000000..257cc56 --- /dev/null +++ b/internal/wire/testdata/InterfaceValue/pkg @@ -0,0 +1 @@ +foo diff --git a/internal/wire/testdata/InterfaceValue/want/program_out.txt b/internal/wire/testdata/InterfaceValue/want/program_out.txt new file mode 100644 index 0000000..3b18e51 --- /dev/null +++ b/internal/wire/testdata/InterfaceValue/want/program_out.txt @@ -0,0 +1 @@ +hello world diff --git a/internal/wire/testdata/InterfaceValue/want/wire_gen.go b/internal/wire/testdata/InterfaceValue/want/wire_gen.go new file mode 100644 index 0000000..6b69e22 --- /dev/null +++ b/internal/wire/testdata/InterfaceValue/want/wire_gen.go @@ -0,0 +1,22 @@ +// Code generated by Wire. DO NOT EDIT. + +//go:generate wire +//+build !wireinject + +package main + +import ( + io "io" + strings "strings" +) + +// Injectors from wire.go: + +func injectedReader() io.Reader { + reader := _wireReaderValue + return reader +} + +var ( + _wireReaderValue = strings.NewReader("hello world") +) diff --git a/internal/wire/testdata/InterfaceValueInvalidArg0/foo/foo.go b/internal/wire/testdata/InterfaceValueInvalidArg0/foo/foo.go new file mode 100644 index 0000000..4289d9f --- /dev/null +++ b/internal/wire/testdata/InterfaceValueInvalidArg0/foo/foo.go @@ -0,0 +1,21 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import "fmt" + +func main() { + fmt.Println(injectedMessage()) +} diff --git a/internal/wire/testdata/InterfaceValueInvalidArg0/foo/wire.go b/internal/wire/testdata/InterfaceValueInvalidArg0/foo/wire.go new file mode 100644 index 0000000..fa64bec --- /dev/null +++ b/internal/wire/testdata/InterfaceValueInvalidArg0/foo/wire.go @@ -0,0 +1,26 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//+build wireinject + +package main + +import ( + "github.com/google/go-cloud/wire" +) + +func injectedMessage() string { + wire.Build(wire.InterfaceValue("foo", "bar")) + return "" +} diff --git a/internal/wire/testdata/InterfaceValueInvalidArg0/pkg b/internal/wire/testdata/InterfaceValueInvalidArg0/pkg new file mode 100644 index 0000000..257cc56 --- /dev/null +++ b/internal/wire/testdata/InterfaceValueInvalidArg0/pkg @@ -0,0 +1 @@ +foo diff --git a/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt b/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt new file mode 100644 index 0000000..8c6768c --- /dev/null +++ b/internal/wire/testdata/InterfaceValueInvalidArg0/want/wire_errs.txt @@ -0,0 +1 @@ +first argument to InterfaceValue must be a pointer to an interface type; found string diff --git a/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/foo.go b/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/foo.go new file mode 100644 index 0000000..4289d9f --- /dev/null +++ b/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/foo.go @@ -0,0 +1,21 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import "fmt" + +func main() { + fmt.Println(injectedMessage()) +} diff --git a/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/wire.go b/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/wire.go new file mode 100644 index 0000000..17576ae --- /dev/null +++ b/internal/wire/testdata/InterfaceValueNotEnoughArgs/foo/wire.go @@ -0,0 +1,26 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//+build wireinject + +package main + +import ( + "github.com/google/go-cloud/wire" +) + +func injectedMessage() string { + wire.Build(wire.InterfaceValue("foo")) + return "" +} diff --git a/internal/wire/testdata/InterfaceValueNotEnoughArgs/pkg b/internal/wire/testdata/InterfaceValueNotEnoughArgs/pkg new file mode 100644 index 0000000..257cc56 --- /dev/null +++ b/internal/wire/testdata/InterfaceValueNotEnoughArgs/pkg @@ -0,0 +1 @@ +foo diff --git a/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt b/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt new file mode 100644 index 0000000..e26d19d --- /dev/null +++ b/internal/wire/testdata/InterfaceValueNotEnoughArgs/want/wire_errs.txt @@ -0,0 +1 @@ +too few arguments in call to wire.InterfaceValue diff --git a/internal/wire/testdata/ValueIsInterfaceValue/foo/foo.go b/internal/wire/testdata/ValueIsInterfaceValue/foo/foo.go new file mode 100644 index 0000000..9e7b5ee --- /dev/null +++ b/internal/wire/testdata/ValueIsInterfaceValue/foo/foo.go @@ -0,0 +1,27 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import ( + "fmt" + "io/ioutil" + "strings" +) + +func main() { + r := injectedReader(strings.NewReader("hello world")) + buf, _ := ioutil.ReadAll(r) + fmt.Println(string(buf)) +} diff --git a/internal/wire/testdata/ValueIsInterfaceValue/foo/wire.go b/internal/wire/testdata/ValueIsInterfaceValue/foo/wire.go new file mode 100644 index 0000000..3c407d0 --- /dev/null +++ b/internal/wire/testdata/ValueIsInterfaceValue/foo/wire.go @@ -0,0 +1,29 @@ +// Copyright 2018 The Go Cloud Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//+build wireinject + +package main + +import ( + "io" + "strings" + + "github.com/google/go-cloud/wire" +) + +func injectedReader(r *strings.Reader) io.Reader { + wire.Build(wire.Value(io.Reader(r))) + return nil +} diff --git a/internal/wire/testdata/ValueIsInterfaceValue/pkg b/internal/wire/testdata/ValueIsInterfaceValue/pkg new file mode 100644 index 0000000..257cc56 --- /dev/null +++ b/internal/wire/testdata/ValueIsInterfaceValue/pkg @@ -0,0 +1 @@ +foo diff --git a/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt b/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt new file mode 100644 index 0000000..022f9b8 --- /dev/null +++ b/internal/wire/testdata/ValueIsInterfaceValue/want/wire_errs.txt @@ -0,0 +1 @@ +argument to Value may not be an interface value (found io.Reader); use InterfaceValue instead diff --git a/wire.go b/wire.go index 8e52134..1c2e988 100644 --- a/wire.go +++ b/wire.go @@ -20,8 +20,8 @@ type ProviderSet struct{} // NewSet creates a new provider set that includes the providers in // its arguments. Each argument is either an exported function value, -// an exported struct (zero) value, a provider set, a call to Bind, or -// a call to Value. +// an exported struct (zero) value, a provider set, a call to Bind, a +// a call to Value, or a call to InterfaceValue. func NewSet(...interface{}) ProviderSet { return ProviderSet{} } @@ -72,6 +72,7 @@ func Bind(iface, to interface{}) Binding { type ProvidedValue struct{} // Value binds an expression to provide the type of the expression. +// The expression may not be an interface value; use InterfaceValue for that. // // Example: // @@ -79,3 +80,12 @@ type ProvidedValue struct{} func Value(interface{}) ProvidedValue { return ProvidedValue{} } + +// InterfaceValue binds an expression to provide a specific interface type. +// +// Example: +// +// var MySet = wire.NewSet(wire.InterfaceValue(new(io.Reader), os.Stdin)) +func InterfaceValue(typ interface{}, x interface{}) ProvidedValue { + return ProvidedValue{} +}