diff --git a/app_test.go b/app_test.go index f6f9158b05..43809d3827 100644 --- a/app_test.go +++ b/app_test.go @@ -283,7 +283,8 @@ func ExampleApp_Run_bashComplete_withLongFlag() { Aliases: []string{"x"}, }, &StringFlag{ - Name: "some-flag,s", + Name: "some-flag", + Aliases: []string{"s"}, }, &StringFlag{ Name: "similar-flag", diff --git a/context_test.go b/context_test.go index 5dc4fae6d2..6e8fada394 100644 --- a/context_test.go +++ b/context_test.go @@ -626,7 +626,7 @@ func TestCheckRequiredFlags(t *testing.T) { expectedAnError: true, expectedErrorContents: []string{"Required flag \"names\" not set"}, flags: []Flag{ - &StringSliceFlag{Name: "names, n", Required: true}, + &StringSliceFlag{Name: "names", Aliases: []string{"n"}, Required: true}, }, }, { diff --git a/flag.go b/flag.go index 4d04de3da8..48f9efb7d8 100644 --- a/flag.go +++ b/flag.go @@ -182,6 +182,14 @@ func flagSet(name string, flags []Flag, spec separatorSpec) (*flag.FlagSet, erro return set, nil } +func validateFlagName(name string) error { + if !commaWhitespace.MatchString(name) { + return nil + } + + return fmt.Errorf("invalid flag name %q: use Aliases instead of v1-style comma or space separated names", name) +} + func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) { switch ff.Value.(type) { case Serializer: diff --git a/flag_bool.go b/flag_bool.go index 01862ea764..595e467996 100644 --- a/flag_bool.go +++ b/flag_bool.go @@ -106,6 +106,10 @@ func (f *BoolFlag) RunAction(c *Context) error { // Apply populates the flag given the flag set and environment func (f *BoolFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_duration.go b/flag_duration.go index e600cc30ad..5aee319f8d 100644 --- a/flag_duration.go +++ b/flag_duration.go @@ -45,6 +45,10 @@ func (f *DurationFlag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *DurationFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_float64.go b/flag_float64.go index 6a4de5c88b..fa57a59ff1 100644 --- a/flag_float64.go +++ b/flag_float64.go @@ -45,6 +45,10 @@ func (f *Float64Flag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *Float64Flag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_float64_slice.go b/flag_float64_slice.go index 0bc4612c82..397f02cc88 100644 --- a/flag_float64_slice.go +++ b/flag_float64_slice.go @@ -138,6 +138,10 @@ func (f *Float64SliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *Float64SliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]float64, len(f.Value.slice)) diff --git a/flag_generic.go b/flag_generic.go index 7528c934cd..f54acbeccd 100644 --- a/flag_generic.go +++ b/flag_generic.go @@ -73,6 +73,10 @@ func (f *GenericFlag) GetEnvVars() []string { // Apply takes the flagset and calls Set on the generic flag with the value // provided by the user for parsing by the flag func (f *GenericFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it if f.Value != nil { f.defaultValue = &stringGeneric{value: f.Value.String()} diff --git a/flag_int.go b/flag_int.go index 750e7ebfc8..271df8a2dc 100644 --- a/flag_int.go +++ b/flag_int.go @@ -45,6 +45,10 @@ func (f *IntFlag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *IntFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_int64.go b/flag_int64.go index 688c267162..2672bcf175 100644 --- a/flag_int64.go +++ b/flag_int64.go @@ -45,6 +45,10 @@ func (f *Int64Flag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *Int64Flag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_int64_slice.go b/flag_int64_slice.go index d45c2dd440..e5e02f974e 100644 --- a/flag_int64_slice.go +++ b/flag_int64_slice.go @@ -139,6 +139,10 @@ func (f *Int64SliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *Int64SliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]int64, len(f.Value.slice)) diff --git a/flag_int_slice.go b/flag_int_slice.go index da9c09bc73..888e90bfd4 100644 --- a/flag_int_slice.go +++ b/flag_int_slice.go @@ -150,6 +150,10 @@ func (f *IntSliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *IntSliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]int, len(f.Value.slice)) diff --git a/flag_path.go b/flag_path.go index 76cb35248c..9626d4e8c5 100644 --- a/flag_path.go +++ b/flag_path.go @@ -50,6 +50,10 @@ func (f *PathFlag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *PathFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_string.go b/flag_string.go index 0f73e06215..ad19d0bf66 100644 --- a/flag_string.go +++ b/flag_string.go @@ -49,6 +49,10 @@ func (f *StringFlag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *StringFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_string_slice.go b/flag_string_slice.go index 66bdf1afcd..6a02cc32e8 100644 --- a/flag_string_slice.go +++ b/flag_string_slice.go @@ -135,6 +135,10 @@ func (f *StringSliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *StringSliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]string, len(f.Value.slice)) diff --git a/flag_test.go b/flag_test.go index 2e130a5856..1a5d84e71a 100644 --- a/flag_test.go +++ b/flag_test.go @@ -88,6 +88,56 @@ func TestBoolFlagApply_SetsCount(t *testing.T) { expect(t, count, 3) } +func TestAppRunRejectsV1StyleFlagNames(t *testing.T) { + tests := []string{ + "config, cfg", + "config cfg", + } + + for _, name := range tests { + t.Run(name, func(t *testing.T) { + app := newTestApp() + app.Flags = []Flag{ + &StringFlag{Name: name}, + } + + err := app.Run([]string{"app"}) + if err == nil { + t.Fatal("expected invalid flag name error") + } + if !strings.Contains(err.Error(), "invalid flag name") { + t.Fatalf("expected invalid flag name error, got %q", err) + } + if !strings.Contains(err.Error(), name) { + t.Fatalf("expected error to mention %q, got %q", name, err) + } + if !strings.Contains(err.Error(), "Aliases") { + t.Fatalf("expected error to point to Aliases, got %q", err) + } + }) + } +} + +func TestCommandRunRejectsV1StyleFlagNames(t *testing.T) { + app := newTestApp() + app.Commands = []*Command{ + { + Name: "serve", + Flags: []Flag{ + &BoolFlag{Name: "verbose, v"}, + }, + }, + } + + err := app.Run([]string{"app", "serve"}) + if err == nil { + t.Fatal("expected invalid flag name error") + } + if !strings.Contains(err.Error(), "verbose, v") { + t.Fatalf("expected error to mention invalid flag name, got %q", err) + } +} + func TestBoolFlagCountFromContext(t *testing.T) { boolCountTests := []struct { diff --git a/flag_timestamp.go b/flag_timestamp.go index b90123087c..60d1d79bed 100644 --- a/flag_timestamp.go +++ b/flag_timestamp.go @@ -139,6 +139,10 @@ func (f *TimestampFlag) GetEnvVars() []string { // Apply populates the flag given the flag set and environment func (f *TimestampFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + if f.Layout == "" { return fmt.Errorf("timestamp Layout is required") } diff --git a/flag_uint.go b/flag_uint.go index 8d5b85458c..e591ce9d4b 100644 --- a/flag_uint.go +++ b/flag_uint.go @@ -23,6 +23,10 @@ func (f *UintFlag) GetCategory() string { // Apply populates the flag given the flag set and environment func (f *UintFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_uint64.go b/flag_uint64.go index c356e533ba..1f89a786d4 100644 --- a/flag_uint64.go +++ b/flag_uint64.go @@ -23,6 +23,10 @@ func (f *Uint64Flag) GetCategory() string { // Apply populates the flag given the flag set and environment func (f *Uint64Flag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // set default value so that environment wont be able to overwrite it f.defaultValue = f.Value f.defaultValueSet = true diff --git a/flag_uint64_slice.go b/flag_uint64_slice.go index d342018686..c8306a79a0 100644 --- a/flag_uint64_slice.go +++ b/flag_uint64_slice.go @@ -143,6 +143,10 @@ func (f *Uint64SliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *Uint64SliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]uint64, len(f.Value.slice)) diff --git a/flag_uint_slice.go b/flag_uint_slice.go index 4dc13e126a..9b062ad921 100644 --- a/flag_uint_slice.go +++ b/flag_uint_slice.go @@ -154,6 +154,10 @@ func (f *UintSliceFlag) IsSliceFlag() bool { // Apply populates the flag given the flag set and environment func (f *UintSliceFlag) Apply(set *flag.FlagSet) error { + if err := validateFlagName(f.Name); err != nil { + return err + } + // apply any default if f.Destination != nil && f.Value != nil { f.Destination.slice = make([]uint, len(f.Value.slice))