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 1 commit
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
Prev Previous commit
Next Next commit
tests passing
  • Loading branch information
aaronc committed Aug 10, 2022
commit 30ffdb9116ee709a3fd047113c6d9b07cc2fd757
4 changes: 2 additions & 2 deletions depinject/check_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ func isExportedType(typ reflect.Type) error {
pkgPath := typ.PkgPath()
if name != "" && pkgPath != "" {
if unicode.IsLower([]rune(name)[0]) {
return errors.Errorf("type is not exported: %s", typ)
return errors.Errorf("type must be exported: %s", typ)
}

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

return nil
Expand Down
18 changes: 2 additions & 16 deletions depinject/check_type_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"gotest.tools/v3/assert"

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

Expand Down Expand Up @@ -42,19 +41,11 @@ func TestCheckIsExportedType(t *testing.T) {
expectValidType(t, uintptr(0))
expectValidType(t, (*Location)(nil))

expectInvalidType(t, container{}, "not exported")
expectInvalidType(t, &container{}, "not exported")
expectInvalidType(t, container{}, "must be exported")
expectInvalidType(t, &container{}, "must be 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{}) {
Expand All @@ -66,8 +57,3 @@ 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
}
32 changes: 16 additions & 16 deletions depinject/invoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,49 +10,49 @@ import (
)

func TestInvoke(t *testing.T) {
gocuke.NewRunner(t, &invokeSuite{}).
gocuke.NewRunner(t, &InvokeSuite{}).
Path("features/invoke.feature").
Step("an int provider returning 5", (*invokeSuite).AnIntProviderReturning5).
Step(`a string pointer provider pointing to "foo"`, (*invokeSuite).AStringPointerProviderPointingToFoo).
Step("an int provider returning 5", (*InvokeSuite).AnIntProviderReturning5).
Step(`a string pointer provider pointing to "foo"`, (*InvokeSuite).AStringPointerProviderPointingToFoo).
Run()
}

type invokeSuite struct {
type InvokeSuite struct {
gocuke.TestingT
configs []depinject.Config
i int
sp *string
}

func (s *invokeSuite) AnInvokerRequestingAnIntAndStringPointer() {
func (s *InvokeSuite) AnInvokerRequestingAnIntAndStringPointer() {
s.configs = append(s.configs,
depinject.Supply(s),
depinject.Invoke((*invokeSuite).IntStringPointerInvoker),
depinject.Invoke((*InvokeSuite).IntStringPointerInvoker),
)
}

func (s *invokeSuite) IntStringPointerInvoker(i int, sp *string) {
func (s *InvokeSuite) IntStringPointerInvoker(i int, sp *string) {
s.i = i
s.sp = sp
}

func (s *invokeSuite) TheContainerIsBuilt() {
func (s *InvokeSuite) TheContainerIsBuilt() {
assert.NilError(s, depinject.Inject(depinject.Configs(s.configs...)))
}

func (s *invokeSuite) TheInvokerWillGetTheIntParameterSetTo(a int64) {
func (s *InvokeSuite) TheInvokerWillGetTheIntParameterSetTo(a int64) {
assert.Equal(s, int(a), s.i)
}

func (s *invokeSuite) TheInvokerWillGetTheStringPointerParameterSetToNil() {
func (s *InvokeSuite) TheInvokerWillGetTheStringPointerParameterSetToNil() {
if s.sp != nil {
s.Fatalf("expected a nil string pointer, got %s", *s.sp)
}
}

func IntProvider5() int { return 5 }

func (s *invokeSuite) AnIntProviderReturning5() {
func (s *InvokeSuite) AnIntProviderReturning5() {
s.configs = append(s.configs, depinject.Provide(IntProvider5))
}

Expand All @@ -61,28 +61,28 @@ func StringPtrProviderFoo() *string {
return &x
}

func (s *invokeSuite) AStringPointerProviderPointingToFoo() {
func (s *InvokeSuite) AStringPointerProviderPointingToFoo() {
s.configs = append(s.configs, depinject.Provide(StringPtrProviderFoo))
}

func (s *invokeSuite) TheInvokerWillGetTheStringPointerParameterSetTo(a string) {
func (s *InvokeSuite) TheInvokerWillGetTheStringPointerParameterSetTo(a string) {
if s.sp == nil {
s.Fatalf("expected a non-nil string pointer")
}
assert.Equal(s, a, *s.sp)
}

func (s *invokeSuite) AnInvokerRequestingAnIntAndStringPointerRunInModule(a string) {
func (s *InvokeSuite) AnInvokerRequestingAnIntAndStringPointerRunInModule(a string) {
s.configs = append(s.configs,
depinject.Supply(s),
depinject.InvokeInModule(a, (*invokeSuite).IntStringPointerInvoker),
depinject.InvokeInModule(a, (*InvokeSuite).IntStringPointerInvoker),
)
}

func ProvideLenModuleKey(key depinject.ModuleKey) int {
return len(key.Name())
}

func (s *invokeSuite) AModulescopedIntProviderWhichReturnsTheLengthOfTheModuleName() {
func (s *InvokeSuite) AModulescopedIntProviderWhichReturnsTheLengthOfTheModuleName() {
s.configs = append(s.configs, depinject.Provide(ProvideLenModuleKey))
}
29 changes: 2 additions & 27 deletions depinject/provider_desc.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package depinject

import (
"fmt"
"reflect"
"strings"
"unicode"
Expand Down Expand Up @@ -147,42 +146,18 @@ func postProcessProvider(descriptor providerDescriptor) (providerDescriptor, err

func checkInputAndOutputTypes(descriptor providerDescriptor) error {
for _, input := range descriptor.Inputs {
err := checkType(input.Type)
err := isExportedType(input.Type)
if err != nil {
return err
}
}

for _, output := range descriptor.Outputs {
err := checkType(output.Type)
err := isExportedType(output.Type)
if err != nil {
return err
}
}

return nil
}

func checkType(typ reflect.Type) error {

name := typ.Name()
if name == "" {
return nil
}

pkgPath := typ.PkgPath()
if pkgPath == "" {

}

if unicode.IsLower([]rune(name)[0]) {
return fmt.Errorf("type must be exported: %s", typ)
}

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

return nil
}
33 changes: 27 additions & 6 deletions depinject/provider_desc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import (
"testing"

"gotest.tools/v3/assert"

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

type StructIn struct {
Expand All @@ -26,6 +29,10 @@ type StructOut struct {

func privateProvider(int, float64) (string, []byte) { return "", nil }

func PrivateInAndOut(containerConfig) *container { return nil }

func InternalInAndOut(graphviz.Attributes) *codegen.FileGen { return nil }

type SomeStruct struct{}

func (SomeStruct) privateMethod() int { return 0 }
Expand Down Expand Up @@ -77,6 +84,20 @@ func TestExtractProviderDescriptor(t *testing.T) {
nil,
"function must be exported",
},
{
"private in and out",
PrivateInAndOut,
nil,
nil,
"type must be exported",
},
{
"internal in and out",
InternalInAndOut,
nil,
nil,
"internal",
},
{
"struct",
SomeStruct{},
Expand Down Expand Up @@ -134,13 +155,13 @@ func TestExtractProviderDescriptor(t *testing.T) {
assert.ErrorContains(t, err, tt.wantErr)
} else {
assert.NilError(t, err)
}

if !reflect.DeepEqual(got.Inputs, tt.wantIn) {
t.Errorf("extractProviderDescriptor() got = %v, want %v", got.Inputs, tt.wantIn)
}
if !reflect.DeepEqual(got.Outputs, tt.wantOut) {
t.Errorf("extractProviderDescriptor() got = %v, want %v", got.Outputs, tt.wantOut)
if !reflect.DeepEqual(got.Inputs, tt.wantIn) {
t.Errorf("extractProviderDescriptor() got = %v, want %v", got.Inputs, tt.wantIn)
}
if !reflect.DeepEqual(got.Outputs, tt.wantOut) {
t.Errorf("extractProviderDescriptor() got = %v, want %v", got.Outputs, tt.wantOut)
}
}
})
}
Expand Down