diff --git a/tm2/pkg/amino/README.md b/tm2/pkg/amino/README.md index b0e0d8baa30..2921d043e96 100644 --- a/tm2/pkg/amino/README.md +++ b/tm2/pkg/amino/README.md @@ -84,8 +84,8 @@ var Package = amino.RegisterPackage( ) ``` -You can still override global registrations with local \*amino.Codec state. -This is used by genproto.P3Context, which may help development while writing +You can still override global registrations with local `*amino.Codec` state. +This is used by `genproto.P3Context`, which may help development while writing migration scripts. Feedback welcome in the issues section. ## Unsupported types diff --git a/tm2/pkg/amino/amino.go b/tm2/pkg/amino/amino.go index b8942c49029..cbd827b3835 100644 --- a/tm2/pkg/amino/amino.go +++ b/tm2/pkg/amino/amino.go @@ -888,17 +888,25 @@ func (cdc *Codec) MarshalJSONIndent(o interface{}, prefix, indent string) ([]byt // ---------------------------------------- // Other +// Given amino package `pi`, register it with the global codec. // NOTE: do not modify the result. func RegisterPackage(pi *pkg.Package) *Package { gcdc.RegisterPackage(pi) return pi } +// Create an unregistered amino package with args: +// - (gopkg string) The Go package path, e.g. "github.com/gnolang/gno/tm2/pkg/std" +// - (p3pkg string) The (shorter) Proto3 package path (no slashes), e.g. "std" +// - (dirname string) Package directory this is called from. Typical is to use `amino.GetCallersDirname()` func NewPackage(gopkg string, p3pkg string, dirname string) *Package { return pkg.NewPackage(gopkg, p3pkg, dirname) } -// NOTE: duplicated in pkg/pkg.go +// Get caller's package directory. +// Implementation uses `filepath.Dir(runtime.Caller(1))`. +// NOTE: duplicated in pkg/pkg.go; given what it does and how, +// both are probably needed. func GetCallersDirname() string { dirname := "" // derive from caller. _, filename, _, ok := runtime.Caller(1) diff --git a/tm2/pkg/amino/codec_test.go b/tm2/pkg/amino/codec_test.go index 9368d4ef40e..89ced32b064 100644 --- a/tm2/pkg/amino/codec_test.go +++ b/tm2/pkg/amino/codec_test.go @@ -3,6 +3,7 @@ package amino_test import ( "bytes" "encoding/binary" + "reflect" "strings" "testing" "time" @@ -236,4 +237,69 @@ func TestCodecSeal(t *testing.T) { assert.Panics(t, func() { cdc.RegisterPackage(tests.Package) }) } -// XXX Test registering duplicate names or concrete types not in a package. +func TestDupTypesMustPanic(t *testing.T) { + // duplicate types must panic + t.Parallel() + + pkg := amino.NewPackage( + reflect.TypeOf(SimpleStruct{}).PkgPath(), + "amino_test", + amino.GetCallersDirname(), + ) + assert.PanicsWithError( + t, + "type amino_test.SimpleStruct already registered with package", + func() { + pkg.WithTypes( + SimpleStruct{}, + SimpleStruct{}, + ) + }) +} + +func TestTypesOutsidePackageMustPanic(t *testing.T) { + // adding concrete types from within a different package must panic + // (use dependency instead) + t.Parallel() + + makepkg := func() *amino.Package { + return amino.NewPackage( + reflect.TypeOf(tests.EmptyStruct{}).PkgPath(), + "amino_test", + amino.GetCallersDirname(), + ) + } + + makepkg().WithTypes(tests.PrimitivesStruct{}) // from same package ✓ + + assert.Panics(t, func() { + makepkg().WithTypes( + SimpleStruct{}, // from another package ✗ + ) + }) +} + +func TestDupNamesMustPanic(t *testing.T) { + // adding types with the same names must panic + t.Parallel() + + makepkg := func() *amino.Package { + return amino.NewPackage( + reflect.TypeOf(tests.EmptyStruct{}).PkgPath(), + "amino_test", + amino.GetCallersDirname(), + ) + } + makepkg().WithTypes( + tests.EmptyStruct{}, "A", + tests.PrimitivesStruct{}, "B", + tests.ShortArraysStruct{}, "C", + ) + assert.Panics(t, func() { + makepkg().WithTypes( + tests.EmptyStruct{}, "A", + tests.PrimitivesStruct{}, "B", + tests.ShortArraysStruct{}, "A", // Same name! + ) + }) +} diff --git a/tm2/pkg/amino/pkg/pkg.go b/tm2/pkg/amino/pkg/pkg.go index b6ffab9748e..7a31256f184 100644 --- a/tm2/pkg/amino/pkg/pkg.go +++ b/tm2/pkg/amino/pkg/pkg.go @@ -118,11 +118,24 @@ func (pkg *Package) WithGoPkgName(name string) *Package { return pkg } +// Package dependencies need to be declared (for now). +// If a package has no dependency, it is conventional to +// use `.WithDependencies()` with no arguments. func (pkg *Package) WithDependencies(deps ...*Package) *Package { pkg.Dependencies = append(pkg.Dependencies, deps...) return pkg } +// WithType specifies which types are encoded and decoded by the package. +// You must provide a list of instantiated objects in the arguments. +// Each type declaration may be optionally followed by a string which is then +// used as its name. +// +// pkg.WithTypes( +// StructA{}, +// &StructB{}, // If pointer receivers are preferred when decoding to interfaces. +// NoInputsError{}, "NoInputsError", // Named +// ) func (pkg *Package) WithTypes(objs ...interface{}) *Package { var lastType *Type = nil for _, obj := range objs { @@ -183,6 +196,17 @@ func (pkg *Package) WithTypes(objs ...interface{}) *Package { } pkg.Types = append(pkg.Types, lastType) } + seen := make(map[string]struct{}) + for _, tp := range pkg.Types { + // tp.Name is "" for cases like nativePkg, containing go native types. + if tp.Name == "" { + continue + } + if _, ok := seen[tp.Name]; ok { + panic("duplicate type name " + tp.Name) + } + seen[tp.Name] = struct{}{} + } return pkg }