From 415d5904369e96540b6d2be223b4f541a2d13e21 Mon Sep 17 00:00:00 2001 From: Robert van Gent Date: Mon, 26 Aug 2019 15:13:56 -0700 Subject: [PATCH 1/2] wire.FieldsOf now provides a pointer to the field type as well as the actual field type --- docs/guide.md | 23 ++++++++++--- internal/wire/analyze.go | 32 ++++++++++++------- internal/wire/parse.go | 13 ++++---- .../wire/testdata/FieldsOfStruct/foo/foo.go | 1 + .../wire/testdata/FieldsOfStruct/foo/wire.go | 7 ++++ .../FieldsOfStruct/want/program_out.txt | 1 + .../testdata/FieldsOfStruct/want/wire_gen.go | 6 ++++ internal/wire/wire.go | 8 +++-- wire.go | 3 +- 9 files changed, 69 insertions(+), 25 deletions(-) diff --git a/docs/guide.md b/docs/guide.md index c1a15af8..9f59a155 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -231,12 +231,13 @@ 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 -### Struct Providers +### Injected Structs -Structs can also be marked as providers. Use the `wire.Struct` function to -inject a struct type and tell the injector which field(s) should be injected. +Structs can be constructed using provided types. Use the `wire.Struct` function +to inject a struct type and tell the injector which field(s) should be injected. The injector will fill in each field using the provider for the field's type. -For a given struct type `S`, this would provide both `S` and `*S`. For example, + +For a given struct type `S`, `wire.Struct` provides both `S` and `*S`. For example, given the following providers: ```go @@ -298,7 +299,18 @@ func injectFooBar() FooBar { } ``` -And similarly if the injector needed a `*FooBar`. +If the injector returned a `*FooBar` instead of a `FooBar`, the generated injector +would look like this: + +```go +func injectFooBar() *FooBar { + foo := ProvideFoo() + fooBar := &FooBar{ + MyFoo: foo, + } + return fooBar +} +``` It is sometimes useful to prevent certain fields from being filled in by the injector, especially when passing `*` to `wire.Struct`. You can tag a field with @@ -412,6 +424,7 @@ func injectedMessage() string { ``` You can add as many field names to a `wire.FieldsOf` function as you like. +For a given field type `T`, `FieldsOf` provides both `T` and `*T`. ### Cleanup functions diff --git a/internal/wire/analyze.go b/internal/wire/analyze.go index 68fe6e56..d5d15ff8 100644 --- a/internal/wire/analyze.go +++ b/internal/wire/analyze.go @@ -84,6 +84,10 @@ type call struct { valueExpr ast.Expr valueTypeInfo *types.Info + + // The following are only set for kind == selectorExpr: + + ptrToField bool } // solve finds the sequence of calls required to produce an output type @@ -226,14 +230,18 @@ dfs: index.Set(curr.t, errAbort) continue dfs } - // Use the args[0] to store the position of the parent struct. + // Use args[0] to store the position of the parent struct. args := []int{v.(int)} + // len(f.Out) is always 2; if curr.t is the 2nd one, then the call must + // provide a pointer to the field. + ptrToField := types.Identical(curr.t, f.Out[1]) calls = append(calls, call{ - kind: selectorExpr, - pkg: f.Pkg, - name: f.Name, - out: curr.t, - args: args, + kind: selectorExpr, + pkg: f.Pkg, + name: f.Name, + out: curr.t, + args: args, + ptrToField: ptrToField, }) default: panic("unknown return value from ProviderSet.For") @@ -382,12 +390,14 @@ func buildProviderMap(fset *token.FileSet, hasher typeutil.Hasher, set *Provider } for _, f := range set.Fields { src := &providerSetSrc{Field: f} - if prevSrc := srcMap.At(f.Out); prevSrc != nil { - ec.add(bindingConflictError(fset, f.Out, set, src, prevSrc.(*providerSetSrc))) - continue + for _, typ := range f.Out { + if prevSrc := srcMap.At(typ); prevSrc != nil { + ec.add(bindingConflictError(fset, typ, set, src, prevSrc.(*providerSetSrc))) + continue + } + providerMap.Set(typ, &ProvidedType{t: typ, f: f}) + srcMap.Set(typ, src) } - providerMap.Set(f.Out, &ProvidedType{t: f.Out, f: f}) - srcMap.Set(f.Out, src) } if len(ec.errors) > 0 { return nil, nil, ec.errors diff --git a/internal/wire/parse.go b/internal/wire/parse.go index fbb82983..39c42b1f 100644 --- a/internal/wire/parse.go +++ b/internal/wire/parse.go @@ -231,8 +231,9 @@ type Field struct { // Pos is the source position of the field declaration. // defining these fields. Pos token.Pos - // Out is the field's type. - Out types.Type + // Out is the field's provided types. The first element provides the + // field type, the second element provides a pointer to it. + Out []types.Type } // Load finds all the provider sets in the packages that match the given @@ -458,7 +459,7 @@ func newObjectCache(pkgs []*packages.Package) *objectCache { } // get converts a Go object into a Wire structure. It may return a *Provider, an -// *IfaceBinding, a *ProviderSet, a *Value, or a *Fields. +// *IfaceBinding, a *ProviderSet, a *Value, or a []*Field. func (oc *objectCache) get(obj types.Object) (val interface{}, errs []error) { ref := objRef{ importPath: obj.Pkg().Path(), @@ -515,7 +516,7 @@ func (oc *objectCache) varDecl(obj *types.Var) *ast.ValueSpec { } // processExpr converts an expression into a Wire structure. It may return a -// *Provider, an *IfaceBinding, a *ProviderSet, a *Value or a *Field. +// *Provider, an *IfaceBinding, a *ProviderSet, a *Value or a []*Field. func (oc *objectCache) processExpr(info *types.Info, pkgPath string, expr ast.Expr, varName string) (interface{}, []error) { exprPos := oc.fset.Position(expr.Pos()) expr = astutil.Unparen(expr) @@ -988,7 +989,7 @@ func processInterfaceValue(fset *token.FileSet, info *types.Info, call *ast.Call }, nil } -// processFieldsOf creates a list of fields from a wire.FieldsOf call. +// processFieldsOf creates a slice of fields from a wire.FieldsOf call. func processFieldsOf(fset *token.FileSet, info *types.Info, call *ast.CallExpr) ([]*Field, error) { // Assumes that call.Fun is wire.FieldsOf. @@ -1034,7 +1035,7 @@ func processFieldsOf(fset *token.FileSet, info *types.Info, call *ast.CallExpr) Name: v.Name(), Pkg: v.Pkg(), Pos: v.Pos(), - Out: v.Type(), + Out: []types.Type{v.Type(), types.NewPointer(v.Type())}, }) } return fields, nil diff --git a/internal/wire/testdata/FieldsOfStruct/foo/foo.go b/internal/wire/testdata/FieldsOfStruct/foo/foo.go index 09e0eb06..9229923f 100644 --- a/internal/wire/testdata/FieldsOfStruct/foo/foo.go +++ b/internal/wire/testdata/FieldsOfStruct/foo/foo.go @@ -26,4 +26,5 @@ func provideS() S { func main() { fmt.Println(injectedMessage()) + fmt.Println("pointer to " + *injectedMessagePtr()) } diff --git a/internal/wire/testdata/FieldsOfStruct/foo/wire.go b/internal/wire/testdata/FieldsOfStruct/foo/wire.go index 94aa3165..7debbfd2 100644 --- a/internal/wire/testdata/FieldsOfStruct/foo/wire.go +++ b/internal/wire/testdata/FieldsOfStruct/foo/wire.go @@ -26,3 +26,10 @@ func injectedMessage() string { wire.FieldsOf(new(S), "Foo")) return "" } + +func injectedMessagePtr() *string { + wire.Build( + provideS, + wire.FieldsOf(new(S), "Foo")) + return nil +} diff --git a/internal/wire/testdata/FieldsOfStruct/want/program_out.txt b/internal/wire/testdata/FieldsOfStruct/want/program_out.txt index 8ab686ea..82baaaf7 100644 --- a/internal/wire/testdata/FieldsOfStruct/want/program_out.txt +++ b/internal/wire/testdata/FieldsOfStruct/want/program_out.txt @@ -1 +1,2 @@ Hello, World! +pointer to Hello, World! diff --git a/internal/wire/testdata/FieldsOfStruct/want/wire_gen.go b/internal/wire/testdata/FieldsOfStruct/want/wire_gen.go index 2092a3db..b01acbeb 100644 --- a/internal/wire/testdata/FieldsOfStruct/want/wire_gen.go +++ b/internal/wire/testdata/FieldsOfStruct/want/wire_gen.go @@ -12,3 +12,9 @@ func injectedMessage() string { string2 := s.Foo return string2 } + +func injectedMessagePtr() *string { + s := provideS() + string2 := &s.Foo + return string2 +} diff --git a/internal/wire/wire.go b/internal/wire/wire.go index 6ec1f3f7..1b64de02 100644 --- a/internal/wire/wire.go +++ b/internal/wire/wire.go @@ -731,10 +731,14 @@ func (ig *injectorGen) valueExpr(lname string, c *call) { func (ig *injectorGen) fieldExpr(lname string, c *call) { a := c.args[0] + ig.p("\t%s := ", lname) + if c.ptrToField { + ig.p("&") + } if a < len(ig.paramNames) { - ig.p("\t%s := %s.%s\n", lname, ig.paramNames[a], c.name) + ig.p("%s.%s\n", ig.paramNames[a], c.name) } else { - ig.p("\t%s := %s.%s\n", lname, ig.localNames[a-len(ig.paramNames)], c.name) + ig.p("%s.%s\n", ig.localNames[a-len(ig.paramNames)], c.name) } } diff --git a/wire.go b/wire.go index 941d6c65..1ef77908 100644 --- a/wire.go +++ b/wire.go @@ -146,7 +146,8 @@ func InterfaceValue(typ interface{}, x interface{}) ProvidedValue { // A StructProvider represents a named struct. type StructProvider struct{} -// Struct specifies that the given struct type will be provided by filling in the fields in the struct that have the names given. +// Struct specifies that the given struct type will be provided by filling in +// the fields in the struct that have the names given. // // The first argument must be a pointer to the struct type. For a struct type // Foo, Wire will use field-filling to provide both Foo and *Foo. The remaining From d95b95ace23015f0992690dcca30a2e25bf3f4e8 Mon Sep 17 00:00:00 2001 From: Robert van Gent Date: Tue, 27 Aug 2019 19:07:32 -0700 Subject: [PATCH 2/2] updates from code review --- docs/guide.md | 9 ++++----- internal/wire/parse.go | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/guide.md b/docs/guide.md index 9f59a155..bbd69e23 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -231,14 +231,13 @@ 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 -### Injected Structs +### Struct Providers Structs can be constructed using provided types. Use the `wire.Struct` function -to inject a struct type and tell the injector which field(s) should be injected. +to construct a struct type and tell the injector which field(s) should be injected. The injector will fill in each field using the provider for the field's type. - -For a given struct type `S`, `wire.Struct` provides both `S` and `*S`. For example, -given the following providers: +For the resulting struct type `S`, `wire.Struct` provides both `S` and `*S`. For +example, given the following providers: ```go type Foo int diff --git a/internal/wire/parse.go b/internal/wire/parse.go index 39c42b1f..be6f50e8 100644 --- a/internal/wire/parse.go +++ b/internal/wire/parse.go @@ -220,7 +220,7 @@ type InjectorArgs struct { Pos token.Pos } -// Field describes a list of fields from a struct. +// Field describes a specific field selected from a struct. type Field struct { // Parent is the struct or pointer to the struct that the field belongs to. Parent types.Type