From 4abf80403232aeb4f65efa525a72644e258237a1 Mon Sep 17 00:00:00 2001 From: Ross Light Date: Wed, 28 Mar 2018 17:31:32 -0700 Subject: [PATCH] goose: rename module to provider set It is more descriptive of what it is, as well as avoids confusion with Go modules in the versioning sense. Reviewed-by: Herbie Ong --- README.md | 107 ++++++++++++++++------------- internal/goose/goose.go | 146 ++++++++++++++++++++-------------------- 2 files changed, 134 insertions(+), 119 deletions(-) diff --git a/README.md b/README.md index 8400a00..1891938 100644 --- a/README.md +++ b/README.md @@ -14,10 +14,10 @@ might have hand-written. The primary mechanism in goose is the **provider**: a function that can produce a value, annotated with the special `goose:provide` directive. These -functions are ordinary Go code and live in packages. +functions are otherwise ordinary Go code. ```go -package module +package foobarbaz type Foo int @@ -29,18 +29,19 @@ func ProvideFoo() Foo { } ``` -Providers are always part of a **module**: if there is no module name specified -on the `//goose:provide` line, then `Module` is used. +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`. Providers can specify dependencies with parameters: ```go -package module - -// goose:provide SuperModule +package foobarbaz type Bar int +// goose:provide SuperSet + // ProvideBar returns a Bar: a negative Foo. func ProvideBar(foo Foo) Bar { return Bar(-foo) @@ -50,7 +51,7 @@ func ProvideBar(foo Foo) Bar { Providers can also return errors: ```go -package module +package foobarbaz import ( "context" @@ -59,7 +60,7 @@ import ( type Baz int -// goose:provide SuperModule +// goose:provide SuperSet // ProvideBaz returns a value if Bar is not zero. func ProvideBaz(ctx context.Context, bar Bar) (Baz, error) { @@ -70,20 +71,33 @@ func ProvideBaz(ctx context.Context, bar Bar) (Baz, error) { } ``` -Modules can import other modules. To import `Module` in `SuperModule`: +Provider sets can import other provider sets. To add `Module` in +`SuperSet`: ```go -// goose:import SuperModule Module +// goose:import SuperSet Module ``` +You can also import provider sets in another package, provided that you have a +Go import for the package: + +```go +// goose:import SuperSet "example.com/some/other/pkg".OtherSet +``` + +A provider set reference is an optional import qualifier (either a package name +or a quoted import path, as seen above) ending with a dot, followed by the +provider set name. + ### Injectors -An application can use these providers by declaring an **injector**: a -generated function that calls providers in dependency order. +An application wires up these providers with an **injector**: a function that +calls providers in dependency order. With goose, you write the injector's +signature, then goose generates the function's body. An injector is declared by writing a function declaration without a body in a file guarded by a `gooseinject` build tag. Let's say that the above providers -were defined in a package called `example.com/module`. The following would +were defined in a package called `example.com/foobarbaz`. The following would declare an injector to obtain a `Baz`: ```go @@ -94,22 +108,21 @@ package main import ( "context" - "example.com/module" + "example.com/foobarbaz" ) -// goose:use module.SuperModule +// goose:use foobarbaz.SuperSet -func initializeApp(ctx context.Context) (module.Baz, error) +func initializeApp(ctx context.Context) (foobarbaz.Baz, error) ``` Like providers, injectors can be parameterized on inputs (which then get sent to -providers) and can return errors. The `goose:use` directive specifies the -modules to use in the injection. Both `goose:use` and `goose:import` use the -same syntax for referencing modules: an optional import qualifier (either a -package name or a quoted import path) with a dot, followed by the module name. -For example: `SamePackageModule`, `foo.Bar`, or `"example.com/foo".Bar`. +providers) and can return errors. Each `goose:use` directive specifies a +provider set to use in the injection. An injector can have one or more +`goose:use` directives. `goose:use` directives use the same syntax as +`goose:import` to reference provider sets. -You can generate the injector using goose: +You can generate the injector by invoking goose in the package directory: ``` goose @@ -125,8 +138,8 @@ go generate (Adding the line to the injection declaration file will be silently ignored by `go generate`.) -goose will produce an implementation of the injector that looks something like -this: +goose will produce an implementation of the injector in a file called +`goose_gen.go` that looks something like this: ```go // Code generated by goose. DO NOT EDIT. @@ -136,13 +149,13 @@ this: package main import ( - "example.com/module" + "example.com/foobarbaz" ) -func initializeApp(ctx context.Context) (module.Baz, error) { - foo := module.ProvideFoo() - bar := module.ProvideBar(foo) - baz, err := module.ProvideBaz(ctx, bar) +func initializeApp(ctx context.Context) (foobarbaz.Baz, error) { + foo := foobarbaz.ProvideFoo() + bar := foobarbaz.ProvideBar(foo) + baz, err := foobarbaz.ProvideBaz(ctx, bar) if err != nil { return 0, err } @@ -159,8 +172,9 @@ written code is just normal Go code, and can be used without goose. ## Best Practices goose is still not mature yet, but guidance that applies to Dagger generally -applies to goose as well. In particular, when thinking about how to group a -package of providers, follow the same [guidance](https://google.github.io/dagger/testing.html#organize-modules-for-testability) as Dagger: +applies to goose as well. In particular, when thinking about how to group +providers into sets, follow the same [guidance](https://google.github.io/dagger/testing.html#organize-modules-for-testability) +as Dagger (provider sets are called modules in Dagger/Guice): > Some [...] bindings will have reasonable alternatives, especially for > testing, and others will not. For example, there are likely to be @@ -174,30 +188,31 @@ package of providers, follow the same [guidance](https://google.github.io/dagger > > Once you’ve classified your bindings into [...] bindings with reasonable > alternatives [and] bindings without reasonable alternatives, consider -> arranging them into packages like this: +> arranging them into provider sets like this: > -> - One [package] for each [...] binding with a reasonable alternative. (If -> you are also writing the alternatives, each one gets its own [package].) That -> [package] contains exactly one provider. -> - All [...] bindings with no reasonable alternatives go into [packages] +> - One [provider set] for each [...] binding with a reasonable alternative. +> (If you are also writing the alternatives, each one gets its own [provider +> set].) That [provider set] contains exactly one provider. +> - All [...] bindings with no reasonable alternatives go into [provider sets] > organized along functional lines. -> - The [packages] should each include the no-reasonable-alternative [packages] that -> require the [...] bindings each provides. +> - The [provider sets] should each include the no-reasonable-alternative +> [provider sets] that require the [...] bindings each provides. One goose-specific practice though: create one-off types where in Java you -would use a binding annotation. +would use a binding annotation. For example, if you need to pass a string +through the dependency graph, you would create a wrapping type: + +```go +type MySQLConnectionString string +``` ## Future Work - The names of imports and provider results in the generated code are not actually as nice as shown above. I'd like to make them nicer in the common cases while ensuring uniqueness. -- I'd like to support optional and multiple bindings. -- At the moment, the entire transitive closure of all dependencies are read - for providers. It might be better to have provider imports be opt-in, but - that seems like too many levels of magic. +- Support for map bindings. +- Support for multiple provider outputs. - Currently, all dependency satisfaction is done using identity. I'd like to use a limited form of assignability for interface types, but I'm unsure how well this implicit satisfaction will work in practice. -- Errors emitted by goose are not very good, but it has all the information - it needs to emit better ones. diff --git a/internal/goose/goose.go b/internal/goose/goose.go index d50a457..615384a 100644 --- a/internal/goose/goose.go +++ b/internal/goose/goose.go @@ -43,7 +43,7 @@ func Generate(bctx *build.Context, wd string, pkg string) ([]byte, error) { } pkgInfo := prog.InitialPackages()[0] g := newGen(pkgInfo.Pkg.Path()) - mc := newModuleCache(prog) + mc := newProviderSetCache(prog) var directives []directive for _, f := range pkgInfo.Files { if !isInjectFile(f) { @@ -60,19 +60,19 @@ func Generate(bctx *build.Context, wd string, pkg string) ([]byte, error) { for _, c := range cmap[fn] { directives = extractDirectives(directives, c) } - modules := make([]moduleRef, 0, len(directives)) + sets := make([]providerSetRef, 0, len(directives)) for _, d := range directives { if d.kind != "use" { return nil, fmt.Errorf("%v: cannot use %s directive on inject function", prog.Fset.Position(d.pos), d.kind) } - ref, err := parseModuleRef(d.line, fileScope, g.currPackage, d.pos) + ref, err := parseProviderSetRef(d.line, fileScope, g.currPackage, d.pos) if err != nil { return nil, fmt.Errorf("%v: %v", prog.Fset.Position(d.pos), err) } - modules = append(modules, ref) + sets = append(sets, ref) } sig := pkgInfo.ObjectOf(fn.Name).Type().(*types.Signature) - if err := g.inject(mc, fn.Name.Name, sig, modules); err != nil { + if err := g.inject(mc, fn.Name.Name, sig, sets); err != nil { return nil, fmt.Errorf("%v: %v", prog.Fset.Position(fn.Pos()), err) } } @@ -128,7 +128,7 @@ func (g *gen) frame(pkgName string) []byte { } // inject emits the code for an injector. -func (g *gen) inject(mc *moduleCache, name string, sig *types.Signature, modules []moduleRef) error { +func (g *gen) inject(mc *providerSetCache, name string, sig *types.Signature, sets []providerSetRef) error { results := sig.Results() returnsErr := false switch results.Len() { @@ -150,7 +150,7 @@ func (g *gen) inject(mc *moduleCache, name string, sig *types.Signature, modules for i := 0; i < params.Len(); i++ { given[i] = params.At(i).Type() } - calls, err := solve(mc, outType, given, modules) + calls, err := solve(mc, outType, given, sets) if err != nil { return err } @@ -244,23 +244,23 @@ func (g *gen) p(format string, args ...interface{}) { fmt.Fprintf(&g.buf, format, args...) } -// A module describes a set of providers. The zero value is an empty -// module. -type module struct { +// A providerSet describes a set of providers. The zero value is an empty +// providerSet. +type providerSet struct { providers []*providerInfo - imports []moduleImport + imports []providerSetImport } -type moduleImport struct { - moduleRef +type providerSetImport struct { + providerSetRef pos token.Pos } const implicitModuleName = "Module" -// findModules processes a package and extracts the modules declared in it. -func findModules(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, files []*ast.File) (map[string]*module, error) { - modules := make(map[string]*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) var directives []directive for _, f := range files { fileScope := typeInfo.Scopes[f] @@ -282,7 +282,7 @@ func findModules(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, } else { name, spec = implicitModuleName, d.line } - ref, err := parseModuleRef(spec, fileScope, pkg.Path(), d.pos) + ref, err := parseProviderSetRef(spec, fileScope, pkg.Path(), d.pos) if err != nil { return nil, fmt.Errorf("%v: %v", fset.Position(d.pos), err) } @@ -295,23 +295,23 @@ func findModules(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, } } if !imported { - return nil, fmt.Errorf("%v: module %s imports %q which is not in the package's imports", fset.Position(d.pos), name, ref.importPath) + return nil, fmt.Errorf("%v: provider set %s imports %q which is not in the package's imports", fset.Position(d.pos), name, ref.importPath) } } - if mod := modules[name]; mod != nil { + if mod := sets[name]; mod != nil { found := false for _, other := range mod.imports { - if ref == other.moduleRef { + if ref == other.providerSetRef { found = true break } } if !found { - mod.imports = append(mod.imports, moduleImport{moduleRef: ref, pos: d.pos}) + mod.imports = append(mod.imports, providerSetImport{providerSetRef: ref, pos: d.pos}) } } else { - modules[name] = &module{ - imports: []moduleImport{{moduleRef: ref, pos: d.pos}}, + sets[name] = &providerSet{ + imports: []providerSetImport{{providerSetRef: ref, pos: d.pos}}, } } default: @@ -326,25 +326,25 @@ func findModules(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, directives = extractDirectives(directives, cg) } fn, isFunction := decl.(*ast.FuncDecl) - var providerModule string + var providerSetName string for _, d := range directives { if d.kind != "provide" { continue } - if providerModule != "" { + if providerSetName != "" { return nil, fmt.Errorf("%v: multiple provide directives for %s", fset.Position(d.pos), fn.Name.Name) } if !isFunction { return nil, fmt.Errorf("%v: only functions can be marked as providers", fset.Position(d.pos)) } if d.line == "" { - providerModule = implicitModuleName + providerSetName = implicitModuleName } else { // TODO(light): validate identifier - providerModule = d.line + providerSetName = d.line } } - if providerModule == "" { + if providerSetName == "" { continue } fpos := fn.Pos() @@ -380,67 +380,67 @@ func findModules(fset *token.FileSet, pkg *types.Package, typeInfo *types.Info, } } } - if mod := modules[providerModule]; mod != nil { + if mod := sets[providerSetName]; mod != nil { for _, other := range mod.providers { if types.Identical(other.out, provider.out) { - return nil, fmt.Errorf("%v: module %s has multiple providers for %s (previous declaration at %v)", fset.Position(fpos), providerModule, types.TypeString(provider.out, nil), fset.Position(other.pos)) + return nil, fmt.Errorf("%v: provider set %s has multiple providers for %s (previous declaration at %v)", fset.Position(fpos), providerSetName, types.TypeString(provider.out, nil), fset.Position(other.pos)) } } mod.providers = append(mod.providers, provider) } else { - modules[providerModule] = &module{ + sets[providerSetName] = &providerSet{ providers: []*providerInfo{provider}, } } } } - return modules, nil + return sets, nil } -// moduleCache is a lazily evaluated index of modules. -type moduleCache struct { - modules map[string]map[string]*module - fset *token.FileSet - prog *loader.Program +// providerSetCache is a lazily evaluated index of provider sets. +type providerSetCache struct { + sets map[string]map[string]*providerSet + fset *token.FileSet + prog *loader.Program } -func newModuleCache(prog *loader.Program) *moduleCache { - return &moduleCache{ +func newProviderSetCache(prog *loader.Program) *providerSetCache { + return &providerSetCache{ fset: prog.Fset, prog: prog, } } -func (mc *moduleCache) get(ref moduleRef) (*module, error) { - if mods, cached := mc.modules[ref.importPath]; cached { - mod := mods[ref.moduleName] +func (mc *providerSetCache) get(ref providerSetRef) (*providerSet, error) { + if mods, cached := mc.sets[ref.importPath]; cached { + mod := mods[ref.name] if mod == nil { - return nil, fmt.Errorf("no such module %s in package %q", ref.moduleName, ref.importPath) + return nil, fmt.Errorf("no such provider set %s in package %q", ref.name, ref.importPath) } return mod, nil } - if mc.modules == nil { - mc.modules = make(map[string]map[string]*module) + if mc.sets == nil { + mc.sets = make(map[string]map[string]*providerSet) } pkg, info, files, err := mc.getpkg(ref.importPath) if err != nil { - mc.modules[ref.importPath] = nil + mc.sets[ref.importPath] = nil return nil, fmt.Errorf("analyze package: %v", err) } - mods, err := findModules(mc.fset, pkg, info, files) + mods, err := findProviderSets(mc.fset, pkg, info, files) if err != nil { - mc.modules[ref.importPath] = nil + mc.sets[ref.importPath] = nil return nil, err } - mc.modules[ref.importPath] = mods - mod := mods[ref.moduleName] + mc.sets[ref.importPath] = mods + mod := mods[ref.name] if mod == nil { - return nil, fmt.Errorf("no such module %s in package %q", ref.moduleName, ref.importPath) + return nil, fmt.Errorf("no such provider set %s in package %q", ref.name, ref.importPath) } return mod, nil } -func (mc *moduleCache) getpkg(path string) (*types.Package, *types.Info, []*ast.File, error) { +func (mc *providerSetCache) getpkg(path string) (*types.Package, *types.Info, []*ast.File, error) { // TODO(light): allow other implementations for testing pkg := mc.prog.Package(path) @@ -452,7 +452,7 @@ func (mc *moduleCache) getpkg(path string) (*types.Package, *types.Info, []*ast. // solve finds the sequence of calls required to produce an output type // with an optional set of provided inputs. -func solve(mc *moduleCache, out types.Type, given []types.Type, modules []moduleRef) ([]call, error) { +func solve(mc *providerSetCache, out types.Type, given []types.Type, sets []providerSetRef) ([]call, error) { for i, g := range given { for _, h := range given[:i] { if types.Identical(g, h) { @@ -460,7 +460,7 @@ func solve(mc *moduleCache, out types.Type, given []types.Type, modules []module } } } - providers, err := buildProviderMap(mc, modules) + providers, err := buildProviderMap(mc, sets) if err != nil { return nil, err } @@ -527,18 +527,18 @@ func solve(mc *moduleCache, out types.Type, given []types.Type, modules []module return calls, nil } -func buildProviderMap(mc *moduleCache, modules []moduleRef) (*typeutil.Map, error) { +func buildProviderMap(mc *providerSetCache, sets []providerSetRef) (*typeutil.Map, error) { type nextEnt struct { - to moduleRef + to providerSetRef - from moduleRef + from providerSetRef pos token.Pos } pm := new(typeutil.Map) // to *providerInfo - visited := make(map[moduleRef]struct{}) + visited := make(map[providerSetRef]struct{}) var next []nextEnt - for _, ref := range modules { + for _, ref := range sets { next = append(next, nextEnt{to: ref}) } for len(next) > 0 { @@ -569,7 +569,7 @@ func buildProviderMap(mc *moduleCache, modules []moduleRef) (*typeutil.Map, erro pm.Set(p.out, p) } for _, imp := range mod.imports { - next = append(next, nextEnt{to: imp.moduleRef, from: curr.to, pos: imp.pos}) + next = append(next, nextEnt{to: imp.providerSetRef, from: curr.to, pos: imp.pos}) } } return pm, nil @@ -603,40 +603,40 @@ type providerInfo struct { hasErr bool } -// A moduleRef is a parsed reference to a collection of providers. -type moduleRef struct { +// A providerSetRef is a parsed reference to a collection of providers. +type providerSetRef struct { importPath string - moduleName string + name string } -func parseModuleRef(ref string, s *types.Scope, pkg string, pos token.Pos) (moduleRef, error) { - // TODO(light): verify that module name is an identifier before returning +func parseProviderSetRef(ref string, s *types.Scope, pkg string, pos token.Pos) (providerSetRef, error) { + // TODO(light): verify that provider set name is an identifier before returning i := strings.LastIndexByte(ref, '.') if i == -1 { - return moduleRef{importPath: pkg, moduleName: ref}, nil + return providerSetRef{importPath: pkg, name: ref}, nil } imp, name := ref[:i], ref[i+1:] if strings.HasPrefix(imp, `"`) { path, err := strconv.Unquote(imp) if err != nil { - return moduleRef{}, fmt.Errorf("parse module reference %q: bad import path", ref) + return providerSetRef{}, fmt.Errorf("parse provider set reference %q: bad import path", ref) } - return moduleRef{importPath: path, moduleName: name}, nil + return providerSetRef{importPath: path, name: name}, nil } _, obj := s.LookupParent(imp, pos) if obj == nil { - return moduleRef{}, fmt.Errorf("parse module reference %q: unknown identifier %s", ref, imp) + return providerSetRef{}, fmt.Errorf("parse provider set reference %q: unknown identifier %s", ref, imp) } pn, ok := obj.(*types.PkgName) if !ok { - return moduleRef{}, fmt.Errorf("parse module reference %q: %s does not name a package", ref, imp) + return providerSetRef{}, fmt.Errorf("parse provider set reference %q: %s does not name a package", ref, imp) } - return moduleRef{importPath: pn.Imported().Path(), moduleName: name}, nil + return providerSetRef{importPath: pn.Imported().Path(), name: name}, nil } -func (ref moduleRef) String() string { - return strconv.Quote(ref.importPath) + "." + ref.moduleName +func (ref providerSetRef) String() string { + return strconv.Quote(ref.importPath) + "." + ref.name } type directive struct {