From 6f29897bf0243b47577101db2c09aa8b24ddf4ed Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Mon, 7 Aug 2023 15:53:02 +0200 Subject: [PATCH] refactor(client/v2): refactor of flags (#17306) --- client/v2/autocli/flag/address.go | 6 +- client/v2/autocli/flag/binary.go | 2 +- client/v2/autocli/flag/builder.go | 303 ++++++++++++++++++ client/v2/autocli/flag/coin.go | 2 +- client/v2/autocli/flag/field.go | 180 ----------- .../autocli/flag/{value.go => interface.go} | 12 + .../flag/{message_json.go => json_message.go} | 0 client/v2/autocli/flag/map.go | 70 ++-- .../flag/{map_types.go => maps/generic.go} | 2 +- .../flag/{ => maps}/map_bool_to_value.go | 2 +- .../flag/{ => maps}/map_int32_to_value.go | 2 +- .../flag/{ => maps}/map_int64_to_value.go | 2 +- .../flag/{ => maps}/map_string_to_value.go | 2 +- .../flag/{ => maps}/map_uint32_to_value.go | 2 +- .../flag/{ => maps}/map_uint64_to_value.go | 2 +- .../flag/{message.go => messager_binder.go} | 2 + client/v2/autocli/flag/pagination.go | 21 -- client/v2/autocli/flag/register.go | 128 -------- client/v2/autocli/flag/type.go | 15 - client/v2/autocli/query.go | 4 +- client/v2/autocli/testdata/bank_example.yaml | 28 -- .../testdata/help-deprecated-msg.golden | 4 +- .../autocli/testdata/help-deprecated.golden | 6 +- .../v2/autocli/testdata/help-echo-msg.golden | 4 +- client/v2/autocli/testdata/help-echo.golden | 6 +- docs/docs/building-modules/10-autocli.md | 2 +- 26 files changed, 377 insertions(+), 432 deletions(-) delete mode 100644 client/v2/autocli/flag/field.go rename client/v2/autocli/flag/{value.go => interface.go} (68%) rename client/v2/autocli/flag/{message_json.go => json_message.go} (100%) rename client/v2/autocli/flag/{map_types.go => maps/generic.go} (98%) rename client/v2/autocli/flag/{ => maps}/map_bool_to_value.go (99%) rename client/v2/autocli/flag/{ => maps}/map_int32_to_value.go (99%) rename client/v2/autocli/flag/{ => maps}/map_int64_to_value.go (99%) rename client/v2/autocli/flag/{ => maps}/map_string_to_value.go (98%) rename client/v2/autocli/flag/{ => maps}/map_uint32_to_value.go (99%) rename client/v2/autocli/flag/{ => maps}/map_uint64_to_value.go (99%) rename client/v2/autocli/flag/{message.go => messager_binder.go} (99%) delete mode 100644 client/v2/autocli/flag/pagination.go delete mode 100644 client/v2/autocli/flag/register.go delete mode 100644 client/v2/autocli/flag/type.go delete mode 100644 client/v2/autocli/testdata/bank_example.yaml diff --git a/client/v2/autocli/flag/address.go b/client/v2/autocli/flag/address.go index cf89c51799d9..7e2985ac3c37 100644 --- a/client/v2/autocli/flag/address.go +++ b/client/v2/autocli/flag/address.go @@ -17,7 +17,7 @@ import ( type addressStringType struct{} -func (a addressStringType) NewValue(ctx context.Context, b *Builder) Value { +func (a addressStringType) NewValue(_ context.Context, b *Builder) Value { return &addressValue{addressCodec: b.AddressCodec} } @@ -27,7 +27,7 @@ func (a addressStringType) DefaultValue() string { type validatorAddressStringType struct{} -func (a validatorAddressStringType) NewValue(ctx context.Context, b *Builder) Value { +func (a validatorAddressStringType) NewValue(_ context.Context, b *Builder) Value { return &addressValue{addressCodec: b.ValidatorAddressCodec} } @@ -61,7 +61,7 @@ func (a *addressValue) Set(s string) error { } func (a addressValue) Type() string { - return "bech32 account address key name" + return "bech32 account address" } type consensusAddressStringType struct{} diff --git a/client/v2/autocli/flag/binary.go b/client/v2/autocli/flag/binary.go index 901b400eb92d..a1a93179f290 100644 --- a/client/v2/autocli/flag/binary.go +++ b/client/v2/autocli/flag/binary.go @@ -14,7 +14,7 @@ type binaryType struct{} var _ Value = (*fileBinaryValue)(nil) -func (f binaryType) NewValue(_ context.Context, _ *Builder) Value { +func (f binaryType) NewValue(context.Context, *Builder) Value { return &fileBinaryValue{} } diff --git a/client/v2/autocli/flag/builder.go b/client/v2/autocli/flag/builder.go index 6b9e77982c53..6720bb3faa18 100644 --- a/client/v2/autocli/flag/builder.go +++ b/client/v2/autocli/flag/builder.go @@ -1,10 +1,20 @@ package flag import ( + "context" + "fmt" + "strconv" + + cosmos_proto "github.com/cosmos/cosmos-proto" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protodesc" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/reflect/protoregistry" + autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" + "cosmossdk.io/client/v2/internal/util" "cosmossdk.io/core/address" ) @@ -55,3 +65,296 @@ func (b *Builder) DefineScalarFlagType(scalarName string, flagType Type) { b.init() b.scalarFlagTypes[scalarName] = flagType } + +func (b *Builder) AddMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions) (*MessageBinder, error) { + return b.addMessageFlags(ctx, flagSet, messageType, commandOptions, namingOptions{}) +} + +// AddMessageFlags adds flags for each field in the message to the flag set. +func (b *Builder) addMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions, options namingOptions) (*MessageBinder, error) { + fields := messageType.Descriptor().Fields() + numFields := fields.Len() + handler := &MessageBinder{ + messageType: messageType, + } + + isPositional := map[string]bool{} + hasVarargs := false + hasOptional := false + n := len(commandOptions.PositionalArgs) + // positional args are also parsed using a FlagSet so that we can reuse all the same parsers + handler.positionalFlagSet = pflag.NewFlagSet("positional", pflag.ContinueOnError) + for i, arg := range commandOptions.PositionalArgs { + isPositional[arg.ProtoField] = true + + field := fields.ByName(protoreflect.Name(arg.ProtoField)) + if field == nil { + return nil, fmt.Errorf("can't find field %s on %s", arg.ProtoField, messageType.Descriptor().FullName()) + } + + if arg.Optional && arg.Varargs { + return nil, fmt.Errorf("positional argument %s can't be both optional and varargs", arg.ProtoField) + } + + if arg.Varargs { + if i != n-1 { + return nil, fmt.Errorf("varargs positional argument %s must be the last argument", arg.ProtoField) + } + + hasVarargs = true + } + + if arg.Optional { + if i != n-1 { + return nil, fmt.Errorf("optional positional argument %s must be the last argument", arg.ProtoField) + } + + hasOptional = true + } + + _, hasValue, err := b.addFieldFlag( + ctx, + handler.positionalFlagSet, + field, + &autocliv1.FlagOptions{Name: fmt.Sprintf("%d", i)}, + namingOptions{}, + ) + if err != nil { + return nil, err + } + + handler.positionalArgs = append(handler.positionalArgs, fieldBinding{ + field: field, + hasValue: hasValue, + }) + } + + if hasVarargs { + handler.CobraArgs = cobra.MinimumNArgs(n - 1) + handler.hasVarargs = true + } else if hasOptional { + handler.CobraArgs = cobra.RangeArgs(n-1, n) + handler.hasOptional = true + } else { + handler.CobraArgs = cobra.ExactArgs(n) + } + + // validate flag options + for name := range commandOptions.FlagOptions { + if fields.ByName(protoreflect.Name(name)) == nil { + return nil, fmt.Errorf("can't find field %s on %s specified as a flag", name, messageType.Descriptor().FullName()) + } + } + + flagOptsByFlagName := map[string]*autocliv1.FlagOptions{} + for i := 0; i < numFields; i++ { + field := fields.Get(i) + if isPositional[string(field.Name())] { + continue + } + + flagOpts := commandOptions.FlagOptions[string(field.Name())] + name, hasValue, err := b.addFieldFlag(ctx, flagSet, field, flagOpts, options) + flagOptsByFlagName[name] = flagOpts + if err != nil { + return nil, err + } + + handler.flagBindings = append(handler.flagBindings, fieldBinding{ + hasValue: hasValue, + field: field, + }) + } + + flagSet.VisitAll(func(flag *pflag.Flag) { + opts := flagOptsByFlagName[flag.Name] + if opts != nil { + // This is a bit of hacking around the pflag API, but + // we need to set these options here using Flag.VisitAll because the flag + // constructors that pflag gives us (StringP, Int32P, etc.) do not + // actually return the *Flag instance + flag.Deprecated = opts.Deprecated + flag.ShorthandDeprecated = opts.ShorthandDeprecated + flag.Hidden = opts.Hidden + } + }) + + return handler, nil +} + +// bindPageRequest create a flag for pagination +func (b *Builder) bindPageRequest(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { + return b.addMessageFlags( + ctx, + flagSet, + util.ResolveMessageType(b.TypeResolver, field.Message()), + &autocliv1.RpcCommandOptions{}, + namingOptions{Prefix: "page-"}, + ) +} + +// namingOptions specifies internal naming options for flags. +type namingOptions struct { + // Prefix is a prefix to prepend to all flags. + Prefix string +} + +// addFieldFlag adds a flag for the provided field to the flag set. +func (b *Builder) addFieldFlag(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { + if opts == nil { + opts = &autocliv1.FlagOptions{} + } + + if field.Kind() == protoreflect.MessageKind && field.Message().FullName() == "cosmos.base.query.v1beta1.PageRequest" { + hasValue, err := b.bindPageRequest(ctx, flagSet, field) + return "", hasValue, err + } + + name = opts.Name + if name == "" { + name = options.Prefix + util.DescriptorKebabName(field) + } + + usage := opts.Usage + if usage == "" { + usage = util.DescriptorDocs(field) + } + + shorthand := opts.Shorthand + defaultValue := opts.DefaultValue + + if typ := b.resolveFlagType(field); typ != nil { + if defaultValue == "" { + defaultValue = typ.DefaultValue() + } + + val := typ.NewValue(ctx, b) + flagSet.AddFlag(&pflag.Flag{ + Name: name, + Shorthand: shorthand, + Usage: usage, + DefValue: defaultValue, + Value: val, + }) + return name, val, nil + } + + // use the built-in pflag StringP, Int32P, etc. functions + var val HasValue + + if field.IsList() { + val = bindSimpleListFlag(flagSet, field.Kind(), name, shorthand, usage) + } else if field.IsMap() { + keyKind := field.MapKey().Kind() + valKind := field.MapValue().Kind() + val = bindSimpleMapFlag(flagSet, keyKind, valKind, name, shorthand, usage) + } else { + val = bindSimpleFlag(flagSet, field.Kind(), name, shorthand, usage) + } + + // This is a bit of hacking around the pflag API, but the + // defaultValue is set in this way because this is much easier than trying + // to parse the string into the types that StringSliceP, Int32P, etc. expect + if defaultValue != "" { + err = flagSet.Set(name, defaultValue) + } + return name, val, err +} + +func (b *Builder) resolveFlagType(field protoreflect.FieldDescriptor) Type { + typ := b.resolveFlagTypeBasic(field) + if field.IsList() { + if typ != nil { + return compositeListType{simpleType: typ} + } + return nil + } + if field.IsMap() { + keyKind := field.MapKey().Kind() + valType := b.resolveFlagType(field.MapValue()) + if valType != nil { + switch keyKind { + case protoreflect.StringKind: + ct := new(compositeMapType[string]) + ct.keyValueResolver = func(s string) (string, error) { return s, nil } + ct.valueType = valType + ct.keyType = "string" + return ct + case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: + ct := new(compositeMapType[int32]) + ct.keyValueResolver = func(s string) (int32, error) { + i, err := strconv.ParseInt(s, 10, 32) + return int32(i), err + } + ct.valueType = valType + ct.keyType = "int32" + return ct + case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: + ct := new(compositeMapType[int64]) + ct.keyValueResolver = func(s string) (int64, error) { + i, err := strconv.ParseInt(s, 10, 64) + return i, err + } + ct.valueType = valType + ct.keyType = "int64" + return ct + case protoreflect.Uint32Kind, protoreflect.Fixed32Kind: + ct := new(compositeMapType[uint32]) + ct.keyValueResolver = func(s string) (uint32, error) { + i, err := strconv.ParseUint(s, 10, 32) + return uint32(i), err + } + ct.valueType = valType + ct.keyType = "uint32" + return ct + case protoreflect.Uint64Kind, protoreflect.Fixed64Kind: + ct := new(compositeMapType[uint64]) + ct.keyValueResolver = func(s string) (uint64, error) { + i, err := strconv.ParseUint(s, 10, 64) + return i, err + } + ct.valueType = valType + ct.keyType = "uint64" + return ct + case protoreflect.BoolKind: + ct := new(compositeMapType[bool]) + ct.keyValueResolver = strconv.ParseBool + ct.valueType = valType + ct.keyType = "bool" + return ct + } + return nil + + } + return nil + } + + return typ +} + +func (b *Builder) resolveFlagTypeBasic(field protoreflect.FieldDescriptor) Type { + scalar := proto.GetExtension(field.Options(), cosmos_proto.E_Scalar) + if scalar != nil { + b.init() + if typ, ok := b.scalarFlagTypes[scalar.(string)]; ok { + return typ + } + } + + switch field.Kind() { + case protoreflect.BytesKind: + return binaryType{} + case protoreflect.EnumKind: + return enumType{enum: field.Enum()} + case protoreflect.MessageKind: + b.init() + if flagType, ok := b.messageFlagTypes[field.Message().FullName()]; ok { + return flagType + } + return jsonMessageFlagType{ + messageDesc: field.Message(), + } + default: + return nil + } +} diff --git a/client/v2/autocli/flag/coin.go b/client/v2/autocli/flag/coin.go index d2b7a139571d..1a1ab182c748 100644 --- a/client/v2/autocli/flag/coin.go +++ b/client/v2/autocli/flag/coin.go @@ -15,7 +15,7 @@ type coinValue struct { value *basev1beta1.Coin } -func (c coinType) NewValue(_ context.Context, _ *Builder) Value { +func (c coinType) NewValue(context.Context, *Builder) Value { return &coinValue{} } diff --git a/client/v2/autocli/flag/field.go b/client/v2/autocli/flag/field.go deleted file mode 100644 index 7591768246c9..000000000000 --- a/client/v2/autocli/flag/field.go +++ /dev/null @@ -1,180 +0,0 @@ -package flag - -import ( - "context" - "strconv" - - cosmos_proto "github.com/cosmos/cosmos-proto" - "github.com/spf13/pflag" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/reflect/protoreflect" - - autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" - "cosmossdk.io/client/v2/internal/util" -) - -// namingOptions specifies internal naming options for flags. -type namingOptions struct { - // Prefix is a prefix to prepend to all flags. - Prefix string -} - -// addFieldFlag adds a flag for the provided field to the flag set. -func (b *Builder) addFieldFlag(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor, opts *autocliv1.FlagOptions, options namingOptions) (name string, hasValue HasValue, err error) { - if opts == nil { - opts = &autocliv1.FlagOptions{} - } - - if field.Kind() == protoreflect.MessageKind && field.Message().FullName() == "cosmos.base.query.v1beta1.PageRequest" { - hasValue, err := b.bindPageRequest(ctx, flagSet, field) - return "", hasValue, err - } - - name = opts.Name - if name == "" { - name = options.Prefix + util.DescriptorKebabName(field) - } - - usage := opts.Usage - if usage == "" { - usage = util.DescriptorDocs(field) - } - - shorthand := opts.Shorthand - defaultValue := opts.DefaultValue - - if typ := b.resolveFlagType(field); typ != nil { - if defaultValue == "" { - defaultValue = typ.DefaultValue() - } - - val := typ.NewValue(ctx, b) - flagSet.AddFlag(&pflag.Flag{ - Name: name, - Shorthand: shorthand, - Usage: usage, - DefValue: defaultValue, - Value: val, - }) - return name, val, nil - } - - // use the built-in pflag StringP, Int32P, etc. functions - var val HasValue - - if field.IsList() { - val = bindSimpleListFlag(flagSet, field.Kind(), name, shorthand, usage) - } else if field.IsMap() { - keyKind := field.MapKey().Kind() - valKind := field.MapValue().Kind() - val = bindSimpleMapFlag(flagSet, keyKind, valKind, name, shorthand, usage) - } else { - val = bindSimpleFlag(flagSet, field.Kind(), name, shorthand, usage) - } - - // This is a bit of hacking around the pflag API, but the - // defaultValue is set in this way because this is much easier than trying - // to parse the string into the types that StringSliceP, Int32P, etc. expect - if defaultValue != "" { - err = flagSet.Set(name, defaultValue) - } - return name, val, err -} - -func (b *Builder) resolveFlagType(field protoreflect.FieldDescriptor) Type { - typ := b.resolveFlagTypeBasic(field) - if field.IsList() { - if typ != nil { - return compositeListType{simpleType: typ} - } - return nil - } - if field.IsMap() { - keyKind := field.MapKey().Kind() - valType := b.resolveFlagType(field.MapValue()) - if valType != nil { - switch keyKind { - case protoreflect.StringKind: - ct := new(compositeMapType[string]) - ct.keyValueResolver = func(s string) (string, error) { return s, nil } - ct.valueType = valType - ct.keyType = "string" - return ct - case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - ct := new(compositeMapType[int32]) - ct.keyValueResolver = func(s string) (int32, error) { - i, err := strconv.ParseInt(s, 10, 32) - return int32(i), err - } - ct.valueType = valType - ct.keyType = "int32" - return ct - case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - ct := new(compositeMapType[int64]) - ct.keyValueResolver = func(s string) (int64, error) { - i, err := strconv.ParseInt(s, 10, 64) - return i, err - } - ct.valueType = valType - ct.keyType = "int64" - return ct - case protoreflect.Uint32Kind, protoreflect.Fixed32Kind: - ct := new(compositeMapType[uint32]) - ct.keyValueResolver = func(s string) (uint32, error) { - i, err := strconv.ParseUint(s, 10, 32) - return uint32(i), err - } - ct.valueType = valType - ct.keyType = "uint32" - return ct - case protoreflect.Uint64Kind, protoreflect.Fixed64Kind: - ct := new(compositeMapType[uint64]) - ct.keyValueResolver = func(s string) (uint64, error) { - i, err := strconv.ParseUint(s, 10, 64) - return i, err - } - ct.valueType = valType - ct.keyType = "uint64" - return ct - case protoreflect.BoolKind: - ct := new(compositeMapType[bool]) - ct.keyValueResolver = strconv.ParseBool - ct.valueType = valType - ct.keyType = "bool" - return ct - } - return nil - - } - return nil - } - - return typ -} - -func (b *Builder) resolveFlagTypeBasic(field protoreflect.FieldDescriptor) Type { - scalar := proto.GetExtension(field.Options(), cosmos_proto.E_Scalar) - if scalar != nil { - b.init() - if typ, ok := b.scalarFlagTypes[scalar.(string)]; ok { - return typ - } - } - - switch field.Kind() { - case protoreflect.BytesKind: - return binaryType{} - case protoreflect.EnumKind: - return enumType{enum: field.Enum()} - case protoreflect.MessageKind: - b.init() - if flagType, ok := b.messageFlagTypes[field.Message().FullName()]; ok { - return flagType - } - return jsonMessageFlagType{ - messageDesc: field.Message(), - } - default: - return nil - } -} diff --git a/client/v2/autocli/flag/value.go b/client/v2/autocli/flag/interface.go similarity index 68% rename from client/v2/autocli/flag/value.go rename to client/v2/autocli/flag/interface.go index e42c524d7859..c7bb61b9d148 100644 --- a/client/v2/autocli/flag/value.go +++ b/client/v2/autocli/flag/interface.go @@ -1,10 +1,22 @@ package flag import ( + "context" + "github.com/spf13/pflag" "google.golang.org/protobuf/reflect/protoreflect" ) +// Type specifies a custom flag type. +type Type interface { + // NewValue returns a new pflag.Value which must also implement either + // SimpleValue or ListValue. + NewValue(context.Context, *Builder) Value + + // DefaultValue is the default value for this type. + DefaultValue() string +} + // Value represents a single pflag.Value value. type Value interface { pflag.Value diff --git a/client/v2/autocli/flag/message_json.go b/client/v2/autocli/flag/json_message.go similarity index 100% rename from client/v2/autocli/flag/message_json.go rename to client/v2/autocli/flag/json_message.go diff --git a/client/v2/autocli/flag/map.go b/client/v2/autocli/flag/map.go index 66b30e0a7800..c2f512503dfa 100644 --- a/client/v2/autocli/flag/map.go +++ b/client/v2/autocli/flag/map.go @@ -8,6 +8,8 @@ import ( "github.com/cockroachdb/errors" "github.com/spf13/pflag" "google.golang.org/protobuf/reflect/protoreflect" + + "cosmossdk.io/client/v2/autocli/flag/maps" ) func bindSimpleMapFlag(flagSet *pflag.FlagSet, keyKind, valueKind protoreflect.Kind, name, shorthand, usage string) HasValue { @@ -18,126 +20,126 @@ func bindSimpleMapFlag(flagSet *pflag.FlagSet, keyKind, valueKind protoreflect.K val := flagSet.StringToStringP(name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := StringToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.StringToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: val := flagSet.StringToInt64P(name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := StringToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.StringToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := StringToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.StringToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := StringToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.StringToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: switch valueKind { case protoreflect.StringKind: - val := Int32ToStringP(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToStringP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := Int32ToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - val := Int32ToInt64P(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToInt64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := Int32ToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := Int32ToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := Int32ToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.Int32ToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: switch valueKind { case protoreflect.StringKind: - val := Int64ToStringP(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToStringP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := Int64ToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - val := Int64ToInt64P(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToInt64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := Int64ToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := Int64ToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := Int64ToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.Int64ToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } case protoreflect.Uint32Kind: switch valueKind { case protoreflect.StringKind: - val := Uint32ToStringP(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToStringP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := Uint32ToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - val := Uint32ToInt64P(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToInt64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := Uint32ToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := Uint32ToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := Uint32ToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.Uint32ToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } case protoreflect.Uint64Kind: switch valueKind { case protoreflect.StringKind: - val := Uint64ToStringP(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToStringP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := Uint64ToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - val := Uint64ToInt64P(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToInt64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := Uint64ToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := Uint64ToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := Uint64ToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.Uint64ToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } case protoreflect.BoolKind: switch valueKind { case protoreflect.StringKind: - val := BoolToStringP(flagSet, name, shorthand, nil, usage) + val := maps.BoolToStringP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfString) case protoreflect.Int32Kind, protoreflect.Sint32Kind, protoreflect.Sfixed32Kind: - val := BoolToInt32P(flagSet, name, shorthand, nil, usage) + val := maps.BoolToInt32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt32) case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: - val := BoolToInt64P(flagSet, name, shorthand, nil, usage) + val := maps.BoolToInt64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfInt64) case protoreflect.Uint32Kind: - val := BoolToUint32P(flagSet, name, shorthand, nil, usage) + val := maps.BoolToUint32P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint32) case protoreflect.Uint64Kind: - val := BoolToUint64P(flagSet, name, shorthand, nil, usage) + val := maps.BoolToUint64P(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfUint64) case protoreflect.BoolKind: - val := BoolToBoolP(flagSet, name, shorthand, nil, usage) + val := maps.BoolToBoolP(flagSet, name, shorthand, nil, usage) return newMapValue(val, protoreflect.ValueOfBool) } diff --git a/client/v2/autocli/flag/map_types.go b/client/v2/autocli/flag/maps/generic.go similarity index 98% rename from client/v2/autocli/flag/map_types.go rename to client/v2/autocli/flag/maps/generic.go index 8fc0397cbc6f..3d610bf9a062 100644 --- a/client/v2/autocli/flag/map_types.go +++ b/client/v2/autocli/flag/maps/generic.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strings" diff --git a/client/v2/autocli/flag/map_bool_to_value.go b/client/v2/autocli/flag/maps/map_bool_to_value.go similarity index 99% rename from client/v2/autocli/flag/map_bool_to_value.go rename to client/v2/autocli/flag/maps/map_bool_to_value.go index 5f9406db34f1..aae8a7d6f9b0 100644 --- a/client/v2/autocli/flag/map_bool_to_value.go +++ b/client/v2/autocli/flag/maps/map_bool_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/map_int32_to_value.go b/client/v2/autocli/flag/maps/map_int32_to_value.go similarity index 99% rename from client/v2/autocli/flag/map_int32_to_value.go rename to client/v2/autocli/flag/maps/map_int32_to_value.go index a2af9d380797..c10adf739963 100644 --- a/client/v2/autocli/flag/map_int32_to_value.go +++ b/client/v2/autocli/flag/maps/map_int32_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/map_int64_to_value.go b/client/v2/autocli/flag/maps/map_int64_to_value.go similarity index 99% rename from client/v2/autocli/flag/map_int64_to_value.go rename to client/v2/autocli/flag/maps/map_int64_to_value.go index 06bd46770ed5..9d85ef964d4c 100644 --- a/client/v2/autocli/flag/map_int64_to_value.go +++ b/client/v2/autocli/flag/maps/map_int64_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/map_string_to_value.go b/client/v2/autocli/flag/maps/map_string_to_value.go similarity index 98% rename from client/v2/autocli/flag/map_string_to_value.go rename to client/v2/autocli/flag/maps/map_string_to_value.go index 914835bb8670..aff2fd94ba34 100644 --- a/client/v2/autocli/flag/map_string_to_value.go +++ b/client/v2/autocli/flag/maps/map_string_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/map_uint32_to_value.go b/client/v2/autocli/flag/maps/map_uint32_to_value.go similarity index 99% rename from client/v2/autocli/flag/map_uint32_to_value.go rename to client/v2/autocli/flag/maps/map_uint32_to_value.go index ae5edc5ab461..c346cd335102 100644 --- a/client/v2/autocli/flag/map_uint32_to_value.go +++ b/client/v2/autocli/flag/maps/map_uint32_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/map_uint64_to_value.go b/client/v2/autocli/flag/maps/map_uint64_to_value.go similarity index 99% rename from client/v2/autocli/flag/map_uint64_to_value.go rename to client/v2/autocli/flag/maps/map_uint64_to_value.go index a53be1864ca3..e5e4318b5bf1 100644 --- a/client/v2/autocli/flag/map_uint64_to_value.go +++ b/client/v2/autocli/flag/maps/map_uint64_to_value.go @@ -1,4 +1,4 @@ -package flag +package maps import ( "strconv" diff --git a/client/v2/autocli/flag/message.go b/client/v2/autocli/flag/messager_binder.go similarity index 99% rename from client/v2/autocli/flag/message.go rename to client/v2/autocli/flag/messager_binder.go index 5ea1943cd4de..b67cb625da2a 100644 --- a/client/v2/autocli/flag/message.go +++ b/client/v2/autocli/flag/messager_binder.go @@ -89,6 +89,7 @@ func (f fieldBinding) bind(msg protoreflect.Message) error { if err != nil { return err } + kind := f.field.Kind() if !(field.IsList() || field.IsMap() || @@ -96,5 +97,6 @@ func (f fieldBinding) bind(msg protoreflect.Message) error { kind == protoreflect.GroupKind) { msg.Set(f.field, val) } + return nil } diff --git a/client/v2/autocli/flag/pagination.go b/client/v2/autocli/flag/pagination.go deleted file mode 100644 index b7e326d0cfa6..000000000000 --- a/client/v2/autocli/flag/pagination.go +++ /dev/null @@ -1,21 +0,0 @@ -package flag - -import ( - "context" - - "github.com/spf13/pflag" - "google.golang.org/protobuf/reflect/protoreflect" - - autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" - "cosmossdk.io/client/v2/internal/util" -) - -func (b *Builder) bindPageRequest(ctx context.Context, flagSet *pflag.FlagSet, field protoreflect.FieldDescriptor) (HasValue, error) { - return b.addMessageFlags( - ctx, - flagSet, - util.ResolveMessageType(b.TypeResolver, field.Message()), - &autocliv1.RpcCommandOptions{}, - namingOptions{Prefix: "page-"}, - ) -} diff --git a/client/v2/autocli/flag/register.go b/client/v2/autocli/flag/register.go deleted file mode 100644 index df0c6932f400..000000000000 --- a/client/v2/autocli/flag/register.go +++ /dev/null @@ -1,128 +0,0 @@ -package flag - -import ( - "context" - "fmt" - - "github.com/spf13/cobra" - "github.com/spf13/pflag" - "google.golang.org/protobuf/reflect/protoreflect" - - autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" -) - -func (b *Builder) AddMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions) (*MessageBinder, error) { - return b.addMessageFlags(ctx, flagSet, messageType, commandOptions, namingOptions{}) -} - -// AddMessageFlags adds flags for each field in the message to the flag set. -func (b *Builder) addMessageFlags(ctx context.Context, flagSet *pflag.FlagSet, messageType protoreflect.MessageType, commandOptions *autocliv1.RpcCommandOptions, options namingOptions) (*MessageBinder, error) { - fields := messageType.Descriptor().Fields() - numFields := fields.Len() - handler := &MessageBinder{ - messageType: messageType, - } - - isPositional := map[string]bool{} - hasVarargs := false - hasOptional := false - n := len(commandOptions.PositionalArgs) - // positional args are also parsed using a FlagSet so that we can reuse all the same parsers - handler.positionalFlagSet = pflag.NewFlagSet("positional", pflag.ContinueOnError) - for i, arg := range commandOptions.PositionalArgs { - isPositional[arg.ProtoField] = true - - field := fields.ByName(protoreflect.Name(arg.ProtoField)) - if field == nil { - return nil, fmt.Errorf("can't find field %s on %s", arg.ProtoField, messageType.Descriptor().FullName()) - } - - if arg.Optional && arg.Varargs { - return nil, fmt.Errorf("positional argument %s can't be both optional and varargs", arg.ProtoField) - } - - if arg.Varargs { - if i != n-1 { - return nil, fmt.Errorf("varargs positional argument %s must be the last argument", arg.ProtoField) - } - - hasVarargs = true - } - - if arg.Optional { - if i != n-1 { - return nil, fmt.Errorf("optional positional argument %s must be the last argument", arg.ProtoField) - } - - hasOptional = true - } - - _, hasValue, err := b.addFieldFlag( - ctx, - handler.positionalFlagSet, - field, - &autocliv1.FlagOptions{Name: fmt.Sprintf("%d", i)}, - namingOptions{}, - ) - if err != nil { - return nil, err - } - - handler.positionalArgs = append(handler.positionalArgs, fieldBinding{ - field: field, - hasValue: hasValue, - }) - } - - if hasVarargs { - handler.CobraArgs = cobra.MinimumNArgs(n - 1) - handler.hasVarargs = true - } else if hasOptional { - handler.CobraArgs = cobra.RangeArgs(n-1, n) - handler.hasOptional = true - } else { - handler.CobraArgs = cobra.ExactArgs(n) - } - - // validate flag options - for name := range commandOptions.FlagOptions { - if fields.ByName(protoreflect.Name(name)) == nil { - return nil, fmt.Errorf("can't find field %s on %s specified as a flag", name, messageType.Descriptor().FullName()) - } - } - - flagOptsByFlagName := map[string]*autocliv1.FlagOptions{} - for i := 0; i < numFields; i++ { - field := fields.Get(i) - if isPositional[string(field.Name())] { - continue - } - - flagOpts := commandOptions.FlagOptions[string(field.Name())] - name, hasValue, err := b.addFieldFlag(ctx, flagSet, field, flagOpts, options) - flagOptsByFlagName[name] = flagOpts - if err != nil { - return nil, err - } - - handler.flagBindings = append(handler.flagBindings, fieldBinding{ - hasValue: hasValue, - field: field, - }) - } - - flagSet.VisitAll(func(flag *pflag.Flag) { - opts := flagOptsByFlagName[flag.Name] - if opts != nil { - // This is a bit of hacking around the pflag API, but - // we need to set these options here using Flag.VisitAll because the flag - // constructors that pflag gives us (StringP, Int32P, etc.) do not - // actually return the *Flag instance - flag.Deprecated = opts.Deprecated - flag.ShorthandDeprecated = opts.ShorthandDeprecated - flag.Hidden = opts.Hidden - } - }) - - return handler, nil -} diff --git a/client/v2/autocli/flag/type.go b/client/v2/autocli/flag/type.go deleted file mode 100644 index a0e79b553d90..000000000000 --- a/client/v2/autocli/flag/type.go +++ /dev/null @@ -1,15 +0,0 @@ -package flag - -import ( - "context" -) - -// Type specifies a custom flag type. -type Type interface { - // NewValue returns a new pflag.Value which must also implement either - // SimpleValue or ListValue. - NewValue(context.Context, *Builder) Value - - // DefaultValue is the default value for this type. - DefaultValue() string -} diff --git a/client/v2/autocli/query.go b/client/v2/autocli/query.go index 6b1c1789eb87..d5d92e364a00 100644 --- a/client/v2/autocli/query.go +++ b/client/v2/autocli/query.go @@ -114,9 +114,7 @@ func (b *Builder) BuildQueryMethodCommand(descriptor protoreflect.MethodDescript } output := outputType.New() - ctx := cmd.Context() - err = clientConn.Invoke(ctx, methodName, input.Interface(), output.Interface()) - if err != nil { + if err := clientConn.Invoke(cmd.Context(), methodName, input.Interface(), output.Interface()); err != nil { return err } diff --git a/client/v2/autocli/testdata/bank_example.yaml b/client/v2/autocli/testdata/bank_example.yaml deleted file mode 100644 index 6fb937482d28..000000000000 --- a/client/v2/autocli/testdata/bank_example.yaml +++ /dev/null @@ -1,28 +0,0 @@ -tx: - service: cosmos.bank.v1beta1.Msg - rpc_command_options: - - rpc_method: Send - use: send [from_key_or_address] [to_address] [amount...] - positional_args: - - proto_field: from_address - - proto_field: to_address - - proto_field: amount - varargs: true -query: - service: cosmos.bank.v1beta1.Query - rpc_command_options: - - rpc_method: Balance - use: balance [address] [denom] - positional_args: - - proto_field: address - - proto_field: denom - - rpc_method: SupplyOf - # this is a contrived example of how to customize flag options - # we would likely prefer positional args here, but this demonstrates usage - use: supply-of --denom [denom] - flag_options: - # flag_options is a map of proto field names to customization options - denom: - shorthand: d - usage: the denom to query - diff --git a/client/v2/autocli/testdata/help-deprecated-msg.golden b/client/v2/autocli/testdata/help-deprecated-msg.golden index 197beaef9e35..22249f1a69fe 100644 --- a/client/v2/autocli/testdata/help-deprecated-msg.golden +++ b/client/v2/autocli/testdata/help-deprecated-msg.golden @@ -8,9 +8,9 @@ Flags: --a-bool --a-coin cosmos.base.v1beta1.Coin --a-message testpb.AMessage (json) - --a-validator-address bech32 account address key name + --a-validator-address bech32 account address -a, --account-number uint The account number of the signing account (offline mode only) - --an-address bech32 account address key name + --an-address bech32 account address --an-enum Enum (unspecified | one | two | five | neg-three) (default unspecified) --aux Generate aux signer data instead of sending a tx --bools bools (default []) diff --git a/client/v2/autocli/testdata/help-deprecated.golden b/client/v2/autocli/testdata/help-deprecated.golden index 5f83095ad615..e7893d184fd3 100644 --- a/client/v2/autocli/testdata/help-deprecated.golden +++ b/client/v2/autocli/testdata/help-deprecated.golden @@ -5,10 +5,10 @@ Usage: Flags: --a-bool --a-coin cosmos.base.v1beta1.Coin - --a-consensus-address bech32 account address key name + --a-consensus-address bech32 account address --a-message testpb.AMessage (json) - --a-validator-address bech32 account address key name - --an-address bech32 account address key name + --a-validator-address bech32 account address + --an-address bech32 account address --an-enum Enum (unspecified | one | two | five | neg-three) (default unspecified) --bools bools (default []) --bz binary diff --git a/client/v2/autocli/testdata/help-echo-msg.golden b/client/v2/autocli/testdata/help-echo-msg.golden index 8699e608e89b..1e6cb73d4948 100644 --- a/client/v2/autocli/testdata/help-echo-msg.golden +++ b/client/v2/autocli/testdata/help-echo-msg.golden @@ -13,9 +13,9 @@ Flags: --a-bool --a-coin cosmos.base.v1beta1.Coin --a-message testpb.AMessage (json) - --a-validator-address bech32 account address key name + --a-validator-address bech32 account address -a, --account-number uint The account number of the signing account (offline mode only) - --an-address bech32 account address key name + --an-address bech32 account address --an-enum Enum (unspecified | one | two | five | neg-three) (default unspecified) --aux Generate aux signer data instead of sending a tx --bools bools (default []) diff --git a/client/v2/autocli/testdata/help-echo.golden b/client/v2/autocli/testdata/help-echo.golden index 7aeef57c4773..1f2baf4ccf3a 100644 --- a/client/v2/autocli/testdata/help-echo.golden +++ b/client/v2/autocli/testdata/help-echo.golden @@ -12,10 +12,10 @@ echo 1 abc {} Flags: --a-bool --a-coin cosmos.base.v1beta1.Coin some random coin - --a-consensus-address bech32 account address key name + --a-consensus-address bech32 account address --a-message testpb.AMessage (json) - --a-validator-address bech32 account address key name - --an-address bech32 account address key name + --a-validator-address bech32 account address + --an-address bech32 account address --an-enum Enum (unspecified | one | two | five | neg-three) (default unspecified) --bools bools (default []) --bz binary some bytes diff --git a/docs/docs/building-modules/10-autocli.md b/docs/docs/building-modules/10-autocli.md index a321dafedaac..5242ecd863d9 100644 --- a/docs/docs/building-modules/10-autocli.md +++ b/docs/docs/building-modules/10-autocli.md @@ -143,7 +143,7 @@ In order to enable this behavior, set in `AutoCLIOptions()` the `EnhanceCustomCo ```go reference -https://github.com/cosmos/cosmos-sdk/blob/julien/custom-commands/x/gov/autocli.go#L98 +https://github.com/cosmos/cosmos-sdk/blob/fa4d87ef7e6d87aaccc94c337ffd2fe90fcb7a9d/x/gov/autocli.go#L98 ``` If not set to true, `AutoCLI` will not generate commands for the module if there are already commands registered for the module (when `GetTxCmd()` or `GetTxCmd()` are defined).