From 8682f3e24422f9cf9978cb9c7ac3c69aa149a75e Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 13 May 2024 12:23:18 +0200 Subject: [PATCH] fix(protoanalysis,templates): fail at duplicate fields (#4128) * fix(templates): fail at duplicate fields * update pr number * styling --- changelog.md | 1 + ignite/pkg/protoanalysis/protoutil/helpers.go | 90 ++++++++++++++++--- ignite/templates/module/create/configs.go | 7 ++ ignite/templates/module/create/params.go | 5 ++ 4 files changed, 90 insertions(+), 13 deletions(-) diff --git a/changelog.md b/changelog.md index 1fa303ff5f..5e8c430bb1 100644 --- a/changelog.md +++ b/changelog.md @@ -40,6 +40,7 @@ - [#4086](https://github.com/ignite/cli/pull/4086) Retry to get the IBC balance if it fails the first time - [#4091](https://github.com/ignite/cli/pull/4091) Fix race conditions in the plugin logic - [#4096](https://github.com/ignite/cli/pull/4096) Add new reserved names module and remove duplicated genesis order +- [#4128](https://github.com/ignite/cli/pull/4128) Check for duplicate proto fields in config ## [`v28.3.0`](https://github.com/ignite/cli/releases/tag/v28.3.0) diff --git a/ignite/pkg/protoanalysis/protoutil/helpers.go b/ignite/pkg/protoanalysis/protoutil/helpers.go index f6c5a2857c..f66a03740c 100644 --- a/ignite/pkg/protoanalysis/protoutil/helpers.go +++ b/ignite/pkg/protoanalysis/protoutil/helpers.go @@ -42,7 +42,7 @@ func AddAfterPackage(f *proto.Proto, v proto.Visitee) error { if inserted { return nil } - return errors.New("could not find package statement") + return errors.New("could not find proto package statement") } // Fallback logic, try and use import after a package and if that fails @@ -118,7 +118,7 @@ func AddImports(f *proto.Proto, fallback bool, imports ...*proto.Import) (err er // recurse with the rest. (might be empty) return AddImports(f, false, imports[1:]...) } - return errors.New("unable to add import, no import statements found") + return errors.New("unable to add proto import, no import statements found") } // NextUniqueID goes through the fields of the given Message and returns @@ -180,10 +180,11 @@ func GetMessageByName(f *proto.Proto, name string) (node *proto.Message, err err }, // return immediately iff found. func(*Cursor) bool { return !found }) + if found { return } - return nil, errors.Errorf("message %s not found", name) + return nil, errors.Errorf("proto message %s not found", name) } // GetServiceByName returns the service with the given name or nil if not found. @@ -192,8 +193,12 @@ func GetMessageByName(f *proto.Proto, name string) (node *proto.Message, err err // f, _ := ParseProtoPath("foo.proto") // s := GetServiceByName(f, "FooSrv") // s.Name // "FooSrv" -func GetServiceByName(f *proto.Proto, name string) (node *proto.Service, err error) { - node, err = nil, nil +func GetServiceByName(f *proto.Proto, name string) (*proto.Service, error) { + var ( + node *proto.Service + err error + ) + found := false Apply(f, func(c *Cursor) bool { @@ -209,12 +214,15 @@ func GetServiceByName(f *proto.Proto, name string) (node *proto.Service, err err _, ok := c.Node().(*proto.Proto) return ok }, + // return immediately iff found. - func(*Cursor) bool { return !found }) + func(*Cursor) bool { return !found }, + ) if found { - return + return node, err } - return nil, errors.Errorf("service %s not found", name) + + return nil, errors.Errorf("proto service %s not found", name) } // GetImportByPath returns the import with the given path or nil if not found. @@ -223,9 +231,13 @@ func GetServiceByName(f *proto.Proto, name string) (node *proto.Service, err err // f, _ := ParseProtoPath("foo.proto") // s := GetImportByPath(f, "other.proto") // s.FileName // "other.proto" -func GetImportByPath(f *proto.Proto, path string) (node *proto.Import, err error) { +func GetImportByPath(f *proto.Proto, path string) (*proto.Import, error) { + var ( + node *proto.Import + err error + ) + found := false - node, err = nil, nil Apply(f, func(c *Cursor) bool { if i, ok := c.Node().(*proto.Import); ok { @@ -240,12 +252,54 @@ func GetImportByPath(f *proto.Proto, path string) (node *proto.Import, err error _, ok := c.Node().(*proto.Proto) return ok }, + // return immediately iff found. - func(*Cursor) bool { return !found }) + func(*Cursor) bool { return !found }, + ) if found { - return + return node, err } - return nil, errors.Errorf("import %s not found", path) + + return nil, errors.Errorf("proto import %s not found", path) +} + +// GetFieldByName returns the field with the given name or nil if not found within a message. +// Only traverses in proto.Message since they are the only nodes that contain fields: +// +// f, _ := ParseProtoPath("foo.proto") +// m := GetMessageByName(f, "Foo") +// f := GetFieldByName(m, "Bar") +// f.Name // "Bar" +func GetFieldByName(f *proto.Message, name string) (*proto.NormalField, error) { + var ( + node *proto.NormalField + err error + ) + + found := false + Apply(f, + func(c *Cursor) bool { + if m, ok := c.Node().(*proto.NormalField); ok { + if m.Name == name { + found = true + node = m + return false + } + // keep looking if we're in a Message + return true + } + // keep looking while we're in a proto.Message. + _, ok := c.Node().(*proto.Message) + return ok + }, + // return immediately iff found. + func(*Cursor) bool { return !found }, + ) + if found { + return node, err + } + + return nil, errors.Errorf("proto field %s not found", name) } // HasMessage returns true if the given message is found in the given file. @@ -277,3 +331,13 @@ func HasImport(f *proto.Proto, path string) bool { _, err := GetImportByPath(f, path) return err == nil } + +func HasField(f *proto.Proto, messageName, field string) bool { + msg, err := GetMessageByName(f, messageName) + if err != nil { + return false + } + + _, err = GetFieldByName(msg, field) + return err == nil +} diff --git a/ignite/templates/module/create/configs.go b/ignite/templates/module/create/configs.go index de74dace00..5d24de6aa5 100644 --- a/ignite/templates/module/create/configs.go +++ b/ignite/templates/module/create/configs.go @@ -1,6 +1,7 @@ package modulecreate import ( + "fmt" "path/filepath" "github.com/gobuffalo/genny/v2" @@ -32,7 +33,13 @@ func configsProtoModify(opts ConfigsOptions) genny.RunFn { if err != nil { return errors.Errorf("couldn't find message 'Module' in %s: %w", path, err) } + for _, paramField := range opts.Configs { + _, err := protoutil.GetFieldByName(params, paramField.Name.LowerCamel) + if err == nil { + return fmt.Errorf("duplicate field %s in %s", paramField.Name.LowerCamel, params.Name) + } + param := protoutil.NewField( paramField.Name.LowerCamel, paramField.DataType(), diff --git a/ignite/templates/module/create/params.go b/ignite/templates/module/create/params.go index 475f5cd4e3..265bf6d487 100644 --- a/ignite/templates/module/create/params.go +++ b/ignite/templates/module/create/params.go @@ -36,6 +36,11 @@ func paramsProtoModify(opts ParamsOptions) genny.RunFn { return errors.Errorf("couldn't find message 'Params' in %s: %w", path, err) } for _, paramField := range opts.Params { + _, err := protoutil.GetFieldByName(params, paramField.Name.LowerCamel) + if err == nil { + return fmt.Errorf("duplicate field %s in %s", paramField.Name.LowerCamel, params.Name) + } + param := protoutil.NewField( paramField.Name.LowerCamel, paramField.DataType(),