Reject v1-style flag aliases

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",

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},
 			},
 		},
 		{

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:

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

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

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
 

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))

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()}

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

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

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))

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))

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

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

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))

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 {

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")
 	}

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

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

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))

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))