Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(depinject)!: require exported functions & types #12797

Merged
merged 32 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e4b5846
refactor(depinject)!: require exported functions
aaronc Aug 2, 2022
555c6fe
unexport ProviderDescriptor
aaronc Aug 2, 2022
88f0935
WIP on tests
aaronc Aug 2, 2022
1f9f6c7
fix tests and check for bound instance methods
aaronc Aug 3, 2022
4fbf97a
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/depin…
aaronc Aug 3, 2022
b1f0c98
address merge issues
aaronc Aug 3, 2022
6e5ee43
WIP on checking valid types
aaronc Aug 3, 2022
9925907
WIP on checking valid types
aaronc Aug 3, 2022
5f2b439
WIP
aaronc Aug 10, 2022
30ffdb9
tests passing
aaronc Aug 10, 2022
8db2564
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/depin…
aaronc Aug 10, 2022
ec9e610
revert changes outside module
aaronc Aug 10, 2022
6546067
docs
aaronc Aug 10, 2022
75b4456
docs
aaronc Aug 10, 2022
0459519
docs
aaronc Aug 10, 2022
0c0e801
Merge branch 'main' into aaronc/depinject-codegen-require-export
julienrbrt Aug 10, 2022
6861c58
add comment
aaronc Aug 11, 2022
5975b98
Merge remote-tracking branch 'origin/aaronc/depinject-codegen-require…
aaronc Aug 11, 2022
2788ed0
Merge branch 'main' into aaronc/depinject-codegen-require-export
aaronc Aug 11, 2022
227c7cf
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/depin…
aaronc Aug 15, 2022
30c67e9
Merge branch 'aaronc/depinject-codegen-require-export' of github.com:…
aaronc Aug 15, 2022
da5a0a1
revert
aaronc Aug 15, 2022
8950c3f
update depinject go.mod versions
aaronc Aug 17, 2022
c83d045
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/depin…
aaronc Aug 17, 2022
a5c4048
remove go.work
aaronc Aug 17, 2022
811b316
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/depin…
aaronc Aug 30, 2022
d037fa0
add go.work back
aaronc Aug 30, 2022
ed1ef9f
Merge branch 'main' into aaronc/depinject-codegen-require-export
aaronc Aug 30, 2022
b14fe7d
go mod tidy
aaronc Aug 30, 2022
30d3205
Merge branch 'main' into aaronc/depinject-codegen-require-export
aaronc Aug 31, 2022
95a0df1
fix docs
aaronc Aug 31, 2022
0c8e64e
Merge remote-tracking branch 'origin/aaronc/depinject-codegen-require…
aaronc Aug 31, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions core/appconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package appconfig

import (
"fmt"
"reflect"
"strings"

"google.golang.org/protobuf/encoding/protojson"
Expand Down Expand Up @@ -94,14 +93,8 @@ func Compose(appConfig *appv1alpha1.Config) depinject.Config {
return depinject.Error(err)
}

opts = append(opts, depinject.Provide(depinject.ProviderDescriptor{
Inputs: nil,
Outputs: []depinject.ProviderOutput{{Type: init.ConfigGoType}},
Fn: func(values []reflect.Value) ([]reflect.Value, error) {
return []reflect.Value{reflect.ValueOf(config)}, nil
},
Location: depinject.LocationFromCaller(0),
}))
// supply the module config itself
opts = append(opts, depinject.Supply(config))

for _, provider := range init.Providers {
opts = append(opts, depinject.ProvideInModule(module.Name, provider))
Expand Down
19 changes: 2 additions & 17 deletions core/appmodule/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package appmodule

import (
"cosmossdk.io/core/internal"
"cosmossdk.io/depinject"
)

// Option is a functional option for implementing modules.
Expand All @@ -21,14 +20,7 @@ func (f funcOption) apply(initializer *internal.ModuleInitializer) error {
// documentation on the dependency injection system.
func Provide(providers ...interface{}) Option {
return funcOption(func(initializer *internal.ModuleInitializer) error {
for _, provider := range providers {
desc, err := depinject.ExtractProviderDescriptor(provider)
if err != nil {
return err
}

initializer.Providers = append(initializer.Providers, desc)
}
initializer.Providers = append(initializer.Providers, providers...)
return nil
})
}
Expand All @@ -39,14 +31,7 @@ func Provide(providers ...interface{}) Option {
// invokers impose no additional constraints on the dependency graph. Invoker functions should nil-check all inputs.
func Invoke(invokers ...interface{}) Option {
return funcOption(func(initializer *internal.ModuleInitializer) error {
for _, invoker := range invokers {
desc, err := depinject.ExtractInvokerDescriptor(invoker)
if err != nil {
return err
}

initializer.Invokers = append(initializer.Invokers, desc)
}
initializer.Providers = append(initializer.Providers, invokers...)
return nil
})
}
6 changes: 2 additions & 4 deletions core/internal/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import (
"google.golang.org/protobuf/reflect/protoreflect"

appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1"

"cosmossdk.io/depinject"
)

// ModuleRegistry is the registry of module initializers indexed by their golang
Expand All @@ -21,8 +19,8 @@ type ModuleInitializer struct {
ConfigGoType reflect.Type
ConfigProtoMessage proto.Message
Error error
Providers []depinject.ProviderDescriptor
Invokers []depinject.ProviderDescriptor
Providers []interface{}
Invokers []interface{}
}

// ModulesByProtoMessageName should be used to retrieve modules by their protobuf name.
Expand Down
30 changes: 20 additions & 10 deletions depinject/binding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,18 @@ func (s bindingSuite) TwoImplementationsMallardAndCanvasback() {
// we don't need to do anything because this is defined at the type level
}

func ProvideMallard() Mallard { return Mallard{} }
func ProvideCanvasback() Canvasback { return Canvasback{} }
func ProvideMarbled() Marbled { return Marbled{} }

func (s *bindingSuite) IsProvided(a string) {
switch a {
case "Mallard":
s.addConfig(depinject.Provide(func() Mallard { return Mallard{} }))
s.addConfig(depinject.Provide(ProvideMallard))
case "Canvasback":
s.addConfig(depinject.Provide(func() Canvasback { return Canvasback{} }))
s.addConfig(depinject.Provide(ProvideCanvasback))
case "Marbled":
s.addConfig(depinject.Provide(func() Marbled { return Marbled{} }))
s.addConfig(depinject.Provide(ProvideMarbled))
default:
s.Fatalf("unexpected duck type %s", a)
}
Expand All @@ -79,18 +83,22 @@ func (s *bindingSuite) addConfig(config depinject.Config) {
s.configs = append(s.configs, config)
}

func ProvideDuckWrapper(duck Duck) DuckWrapper {
return DuckWrapper{Module: "", Duck: duck}
}

func (s *bindingSuite) WeTryToResolveADuckInGlobalScope() {
s.addConfig(depinject.Provide(func(duck Duck) DuckWrapper {
return DuckWrapper{Module: "", Duck: duck}
}))
s.addConfig(depinject.Provide(ProvideDuckWrapper))
}

func ResolvePond(ducks []DuckWrapper) Pond { return Pond{Ducks: ducks} }

func (s *bindingSuite) resolvePond() *Pond {
if s.pond != nil {
return s.pond
}

s.addConfig(depinject.Provide(func(ducks []DuckWrapper) Pond { return Pond{Ducks: ducks} }))
s.addConfig(depinject.Provide(ResolvePond))
var pond Pond
s.err = depinject.Inject(depinject.Configs(s.configs...), &pond)
s.pond = &pond
Expand Down Expand Up @@ -131,10 +139,12 @@ func (s *bindingSuite) ThereIsABindingForAInModule(preferredType string, interfa
s.addConfig(depinject.BindInterfaceInModule(moduleName, fullTypeName(interfaceType), fullTypeName(preferredType)))
}

func ProvideModuleDuck(duck Duck, key depinject.OwnModuleKey) DuckWrapper {
return DuckWrapper{Module: depinject.ModuleKey(key).Name(), Duck: duck}
}

func (s *bindingSuite) ModuleWantsADuck(module string) {
s.addConfig(depinject.ProvideInModule(module, func(duck Duck) DuckWrapper {
return DuckWrapper{Module: module, Duck: duck}
}))
s.addConfig(depinject.ProvideInModule(module, ProvideModuleDuck))
}

func (s *bindingSuite) ModuleResolvesA(module string, duckType string) {
Expand Down
63 changes: 63 additions & 0 deletions depinject/check_type.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package depinject

import (
"reflect"
Fixed Show fixed Hide fixed
"strings"
"unicode"

"github.com/pkg/errors"
"golang.org/x/exp/slices"
)

// isExportedType checks if the type is exported and not in an internal
// package.
func isExportedType(typ reflect.Type) error {
name := typ.Name()
pkgPath := typ.PkgPath()
if name != "" && pkgPath != "" {
if unicode.IsLower([]rune(name)[0]) {
return errors.Errorf("type is not exported: %s", typ)
}

pkgParts := strings.Split(pkgPath, "/")
if slices.Contains(pkgParts, "internal") {
return errors.Errorf("type is in an internal package: %s", typ)
}

return nil
}

switch typ.Kind() {
case reflect.Array, reflect.Slice, reflect.Chan, reflect.Pointer:
return isExportedType(typ.Elem())

case reflect.Func:
numIn := typ.NumIn()
for i := 0; i < numIn; i++ {
err := isExportedType(typ.In(i))
if err != nil {
return err
}
}

numOut := typ.NumOut()
for i := 0; i < numOut; i++ {
err := isExportedType(typ.Out(i))
if err != nil {
return err
}
}

return nil

case reflect.Map:
err := isExportedType(typ.Key())
if err != nil {
return err
}
return isExportedType(typ.Elem())

default:
return nil
aaronc marked this conversation as resolved.
Show resolved Hide resolved
}
}
73 changes: 73 additions & 0 deletions depinject/check_type_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package depinject

import (
"os"
"reflect"
"testing"

"gotest.tools/v3/assert"

"cosmossdk.io/depinject/internal/codegen"
"cosmossdk.io/depinject/internal/graphviz"
)

func TestCheckIsExportedType(t *testing.T) {
expectValidType(t, false)
expectValidType(t, uint(0))
expectValidType(t, uint8(0))
expectValidType(t, uint16(0))
expectValidType(t, uint32(0))
expectValidType(t, uint64(0))
expectValidType(t, int(0))
expectValidType(t, int8(0))
expectValidType(t, int16(0))
expectValidType(t, int32(0))
expectValidType(t, int64(0))
expectValidType(t, float32(0))
expectValidType(t, float64(0))
expectValidType(t, complex64(0))
expectValidType(t, complex128(0))
expectValidType(t, os.FileMode(0))
expectValidType(t, [1]int{0})
expectValidType(t, []int{})
expectValidType(t, "")
expectValidType(t, make(chan int))
expectValidType(t, make(<-chan int))
expectValidType(t, make(chan<- int))
expectValidType(t, func(int, string) (bool, error) { return false, nil })
expectValidType(t, func(int, ...string) (bool, error) { return false, nil })
expectValidType(t, In{})
expectValidType(t, map[string]In{})
expectValidType(t, &In{})
expectValidType(t, uintptr(0))
expectValidType(t, (*Location)(nil))

expectInvalidType(t, container{}, "not exported")
expectInvalidType(t, &container{}, "not exported")
expectInvalidType(t, graphviz.Attributes{}, "internal")
expectInvalidType(t, map[string]graphviz.Attributes{}, "internal")
expectInvalidType(t, []graphviz.Attributes{}, "internal")

// generics
expectValidType(t, AGenericStruct[int, In]{})
expectValidType(t, AGenericStruct[*In, *Out]{})
expectInvalidType(t, AGenericStruct[container, containerConfig]{}, "not exported")
expectInvalidType(t, AGenericStruct[*container, *containerConfig]{}, "not exported")
expectInvalidType(t, AGenericStruct[graphviz.Attributes, codegen.FileGen]{}, "internal")
expectInvalidType(t, AGenericStruct[*graphviz.Attributes, *codegen.FileGen]{}, "internal")
}

func expectValidType(t *testing.T, v interface{}) {
t.Helper()
assert.NilError(t, isExportedType(reflect.TypeOf(v)))
}

func expectInvalidType(t *testing.T, v interface{}, errContains string) {
t.Helper()
assert.ErrorContains(t, isExportedType(reflect.TypeOf(v)), errContains)
}

type AGenericStruct[A, B any] struct {
A A
B B
}
4 changes: 2 additions & 2 deletions depinject/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func ProvideInModule(moduleName string, providers ...interface{}) Config {

func provide(ctr *container, key *moduleKey, providers []interface{}) error {
for _, c := range providers {
rc, err := ExtractProviderDescriptor(c)
rc, err := extractProviderDescriptor(c)
if err != nil {
return errors.WithStack(err)
}
Expand Down Expand Up @@ -75,7 +75,7 @@ func InvokeInModule(moduleName string, invokers ...interface{}) Config {

func invoke(ctr *container, key *moduleKey, invokers []interface{}) error {
for _, c := range invokers {
rc, err := ExtractInvokerDescriptor(c)
rc, err := extractInvokerDescriptor(c)
if err != nil {
return errors.WithStack(err)
}
Expand Down
23 changes: 14 additions & 9 deletions depinject/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type container struct {
}

type invoker struct {
fn *ProviderDescriptor
fn *providerDescriptor
modKey *moduleKey
}

Expand Down Expand Up @@ -55,7 +55,7 @@ func newContainer(cfg *debugConfig) *container {
}
}

func (c *container) call(provider *ProviderDescriptor, moduleKey *moduleKey) ([]reflect.Value, error) {
func (c *container) call(provider *providerDescriptor, moduleKey *moduleKey) ([]reflect.Value, error) {
loc := provider.Location
graphNode := c.locationGraphNode(loc, moduleKey)

Expand Down Expand Up @@ -205,7 +205,7 @@ func (c *container) getExplicitResolver(typ reflect.Type, key *moduleKey) (resol

var stringType = reflect.TypeOf("")

func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (interface{}, error) {
func (c *container) addNode(provider *providerDescriptor, key *moduleKey) (interface{}, error) {
providerGraphNode := c.locationGraphNode(provider.Location, key)
hasModuleKeyParam := false
hasOwnModuleKeyParam := false
Expand Down Expand Up @@ -359,7 +359,7 @@ func (c *container) supply(value reflect.Value, location Location) error {
return nil
}

func (c *container) addInvoker(provider *ProviderDescriptor, key *moduleKey) error {
func (c *container) addInvoker(provider *providerDescriptor, key *moduleKey) error {
// make sure there are no outputs
if len(provider.Outputs) > 0 {
return fmt.Errorf("invoker function %s should not return any outputs", provider.Location)
Expand All @@ -373,7 +373,7 @@ func (c *container) addInvoker(provider *ProviderDescriptor, key *moduleKey) err
return nil
}

func (c *container) resolve(in ProviderInput, moduleKey *moduleKey, caller Location) (reflect.Value, error) {
func (c *container) resolve(in providerInput, moduleKey *moduleKey, caller Location) (reflect.Value, error) {
c.resolveStack = append(c.resolveStack, resolveFrame{loc: caller, typ: in.Type})

typeGraphNode := c.typeGraphNode(in.Type)
Expand Down Expand Up @@ -426,17 +426,17 @@ func (c *container) resolve(in ProviderInput, moduleKey *moduleKey, caller Locat
}

func (c *container) build(loc Location, outputs ...interface{}) error {
var providerIn []ProviderInput
var providerIn []providerInput
for _, output := range outputs {
typ := reflect.TypeOf(output)
if typ.Kind() != reflect.Pointer {
return fmt.Errorf("output type must be a pointer, %s is invalid", typ)
}

providerIn = append(providerIn, ProviderInput{Type: typ.Elem()})
providerIn = append(providerIn, providerInput{Type: typ.Elem()})
}

desc := ProviderDescriptor{
desc := providerDescriptor{
Inputs: providerIn,
Outputs: nil,
Fn: func(values []reflect.Value) ([]reflect.Value, error) {
Expand Down Expand Up @@ -524,7 +524,12 @@ func fullyQualifiedTypeName(typ reflect.Type) string {
if typ.Kind() == reflect.Pointer || typ.Kind() == reflect.Slice || typ.Kind() == reflect.Map || typ.Kind() == reflect.Array {
pkgType = typ.Elem()
}
return fmt.Sprintf("%s/%v", pkgType.PkgPath(), typ)
pkgPath := pkgType.PkgPath()
if pkgPath == "" {
return fmt.Sprintf("%v", typ)
}

return fmt.Sprintf("%s/%v", pkgPath, typ)
}

func bindingKeyFromTypeName(typeName string, key *moduleKey) string {
Expand Down
Loading