From 34987b6bee151f460aa78e61aa39ccade6be2eda Mon Sep 17 00:00:00 2001 From: Ross Light Date: Wed, 28 Mar 2018 17:49:12 -0700 Subject: [PATCH] goose: use function name as implicit provider set Single-element provider sets are frequently useful enough to warrant being a default. Larger groupings within a package are less frequent. Reviewed-by: Herbie Ong Reviewed-by: Tuo Shan --- README.md | 6 +++--- internal/goose/goose.go | 19 +++++++------------ internal/goose/goose_test.go | 34 +++++++++++++++++----------------- 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index 1891938..a7130c5 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ func ProvideFoo() Foo { Providers are always part of a **provider set**: if there is no provider set named on the `//goose:provide` line, then the provider is added to the provider -set with the name `Module`. +set with the same name as the function (`ProvideFoo`, in this case). Providers can specify dependencies with parameters: @@ -71,11 +71,11 @@ func ProvideBaz(ctx context.Context, bar Bar) (Baz, error) { } ``` -Provider sets can import other provider sets. To add `Module` in +Provider sets can import other provider sets. To add the `ProvideFoo` set to `SuperSet`: ```go -// goose:import SuperSet Module +// goose:import SuperSet ProvideFoo ``` You can also import provider sets in another package, provided that you have a diff --git a/internal/goose/goose.go b/internal/goose/goose.go index 615384a..79dac72 100644 --- a/internal/goose/goose.go +++ b/internal/goose/goose.go @@ -256,8 +256,6 @@ type providerSetImport struct { pos token.Pos } -const implicitModuleName = "Module" - // findProviderSets processes a package and extracts the provider sets declared in it. func findProviderSets(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, files []*ast.File) (map[string]*providerSet, error) { sets := make(map[string]*providerSet) @@ -274,14 +272,12 @@ func findProviderSets(fset *token.FileSet, pkg *types.Package, typeInfo *types.I if fileScope == nil { return nil, fmt.Errorf("%s: no scope found for file (likely a bug)", fset.File(f.Pos()).Name()) } - var name, spec string - if strings.HasPrefix(d.line, `"`) { - name, spec = implicitModuleName, d.line - } else if i := strings.IndexByte(d.line, ' '); i != -1 { - name, spec = d.line[:i], d.line[i+1:] - } else { - name, spec = implicitModuleName, d.line + i := strings.IndexByte(d.line, ' ') + // TODO(light): allow multiple imports in one line + if i == -1 { + return nil, fmt.Errorf("%s: invalid import: expected TARGET SETREF", fset.Position(d.pos)) } + name, spec := d.line[:i], d.line[i+1:] ref, err := parseProviderSetRef(spec, fileScope, pkg.Path(), d.pos) if err != nil { return nil, fmt.Errorf("%v: %v", fset.Position(d.pos), err) @@ -337,9 +333,8 @@ func findProviderSets(fset *token.FileSet, pkg *types.Package, typeInfo *types.I if !isFunction { return nil, fmt.Errorf("%v: only functions can be marked as providers", fset.Position(d.pos)) } - if d.line == "" { - providerSetName = implicitModuleName - } else { + providerSetName = fn.Name.Name + if d.line != "" { // TODO(light): validate identifier providerSetName = d.line } diff --git a/internal/goose/goose_test.go b/internal/goose/goose_test.go index 9441547..0a1f331 100644 --- a/internal/goose/goose_test.go +++ b/internal/goose/goose_test.go @@ -44,7 +44,7 @@ func provideMessage() string { return "Hello, World!"; } package main -//goose:use Module +//goose:use provideMessage func injectedMessage() string `, @@ -59,7 +59,7 @@ func injectedMessage() string import "fmt" func main() { fmt.Println(injectedMessage()); } -//goose:provide +//goose:provide Set // provideMessage provides a friendly user greeting. func provideMessage() string { return "Hello, World!"; } @@ -86,17 +86,17 @@ func main() { type Foo int type FooBar int -//goose:provide +//goose:provide Set func provideFoo() Foo { return 41 } -//goose:provide +//goose:provide Set func provideFooBar(foo Foo) FooBar { return FooBar(foo) + 1 } `, "foo/foo_goose.go": `//+build gooseinject package main -//goose:use Module +//goose:use Set func injectFooBar() FooBar `, @@ -117,20 +117,20 @@ type Foo int type Bar int type FooBar int -//goose:provide +//goose:provide Set func provideFoo() Foo { return 40 } -//goose:provide +//goose:provide Set func provideBar() Bar { return 2 } -//goose:provide +//goose:provide Set func provideFooBar(foo Foo, bar Bar) FooBar { return FooBar(foo) + FooBar(bar) } `, "foo/foo_goose.go": `//+build gooseinject package main -//goose:use Module +//goose:use Set func injectFooBar() FooBar `, @@ -151,17 +151,17 @@ type Foo int type Bar int type FooBar int -//goose:provide +//goose:provide Set func provideBar() Bar { return 2 } -//goose:provide +//goose:provide Set func provideFooBar(foo Foo, bar Bar) FooBar { return FooBar(foo) + FooBar(bar) } `, "foo/foo_goose.go": `//+build gooseinject package main -//goose:use Module +//goose:use Set func injectFooBar(foo Foo) FooBar `, @@ -181,17 +181,17 @@ func main() { type Foo int type Bar int -//goose:provide +//goose:provide Set func provideFoo() Foo { return -888 } -//goose:provide +//goose:provide Set func provideBar(foo Foo) Bar { return 2 } `, "foo/foo_goose.go": `//+build gooseinject package main -//goose:use Module +//goose:use Set func injectBar(foo Foo) Bar `, @@ -218,14 +218,14 @@ func main() { type Foo int -//goose:provide +//goose:provide Set func provideFoo() (Foo, error) { return 42, errors.New("there is no Foo") } `, "foo/foo_goose.go": `//+build gooseinject package main -//goose:use Module +//goose:use Set func injectFoo() (Foo, error) `,