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

fix(protoanalysis,templates): fail at duplicate fields #4128

Merged
merged 6 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
90 changes: 77 additions & 13 deletions ignite/pkg/protoanalysis/protoutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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.
Expand All @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
7 changes: 7 additions & 0 deletions ignite/templates/module/create/configs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package modulecreate

import (
"fmt"
"path/filepath"

"github.com/gobuffalo/genny/v2"
Expand Down Expand Up @@ -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 {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("duplicate field %s in %s", paramField.Name.LowerCamel, params.Name)
}

param := protoutil.NewField(
paramField.Name.LowerCamel,
paramField.DataType(),
Expand Down
5 changes: 5 additions & 0 deletions ignite/templates/module/create/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading