From d064a0545a9fa477885883e66fa670a9d6820883 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 10 May 2024 00:04:40 +0200 Subject: [PATCH 1/3] fix(templates): fail at duplicate fields --- changelog.md | 1 + ignite/pkg/protoanalysis/protoutil/helpers.go | 43 +++++++++++++++++++ ignite/templates/module/create/configs.go | 7 +++ ignite/templates/module/create/params.go | 5 +++ 4 files changed, 56 insertions(+) diff --git a/changelog.md b/changelog.md index f1e570e6bf..61439e0ee2 100644 --- a/changelog.md +++ b/changelog.md @@ -35,6 +35,7 @@ ### Fixes +- []() Check for duplicate proto fields in config - [#3969](https://github.com/ignite/cli/pull/3969) Get first config validator using a getter to avoid index errors - [#4000](https://github.com/ignite/cli/pull/4000) Run all dry runners before the wet run in the `xgenny` pkg - [#4086](https://github.com/ignite/cli/pull/4086) Retry to get the IBC balance if it fails the first time diff --git a/ignite/pkg/protoanalysis/protoutil/helpers.go b/ignite/pkg/protoanalysis/protoutil/helpers.go index f6c5a2857c..f2381b5f59 100644 --- a/ignite/pkg/protoanalysis/protoutil/helpers.go +++ b/ignite/pkg/protoanalysis/protoutil/helpers.go @@ -248,6 +248,39 @@ func GetImportByPath(f *proto.Proto, path string) (node *proto.Import, err error return nil, errors.Errorf("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) (node *proto.NormalField, err error) { + node, err = nil, nil + 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 + } + return nil, errors.Errorf("field %s not found", name) +} + // HasMessage returns true if the given message is found in the given file. // // f, _ := ParseProtoPath("foo.proto") @@ -277,3 +310,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(), From 6aadda388518ad3f1e17a8fe5cbedaa5d7f0ead8 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Fri, 10 May 2024 00:07:23 +0200 Subject: [PATCH 2/3] update pr number --- changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index 61439e0ee2..e67b01a0ee 100644 --- a/changelog.md +++ b/changelog.md @@ -35,12 +35,12 @@ ### Fixes -- []() Check for duplicate proto fields in config - [#3969](https://github.com/ignite/cli/pull/3969) Get first config validator using a getter to avoid index errors - [#4000](https://github.com/ignite/cli/pull/4000) Run all dry runners before the wet run in the `xgenny` pkg - [#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) From c0547fbabf5ae7388709325c4b415e0f348e3f2f Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 13 May 2024 11:25:46 +0200 Subject: [PATCH 3/3] styling --- ignite/pkg/protoanalysis/protoutil/helpers.go | 57 +++++++++++++------ 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/ignite/pkg/protoanalysis/protoutil/helpers.go b/ignite/pkg/protoanalysis/protoutil/helpers.go index f2381b5f59..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,15 @@ 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. @@ -255,8 +270,12 @@ func GetImportByPath(f *proto.Proto, path string) (node *proto.Import, err error // m := GetMessageByName(f, "Foo") // f := GetFieldByName(m, "Bar") // f.Name // "Bar" -func GetFieldByName(f *proto.Message, name string) (node *proto.NormalField, err error) { - node, err = nil, nil +func GetFieldByName(f *proto.Message, name string) (*proto.NormalField, error) { + var ( + node *proto.NormalField + err error + ) + found := false Apply(f, func(c *Cursor) bool { @@ -274,11 +293,13 @@ func GetFieldByName(f *proto.Message, name string) (node *proto.NormalField, err 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("field %s not found", name) + + return nil, errors.Errorf("proto field %s not found", name) } // HasMessage returns true if the given message is found in the given file.