Reject v1-style flag aliases

Author: Bobby

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

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"reflect"
 	"regexp"
 	"runtime"
 	"strings"
@@ -171,6 +172,9 @@ func flagSet(name string, flags []Flag, spec separatorSpec) (*flag.FlagSet, erro
 	set := flag.NewFlagSet(name, flag.ContinueOnError)
 
 	for _, f := range flags {
+		if err := validateFlagName(f); err != nil {
+			return nil, err
+		}
 		if c, ok := f.(customizedSeparator); ok {
 			c.WithSeparatorSpec(spec)
 		}
@@ -182,6 +186,35 @@ func flagSet(name string, flags []Flag, spec separatorSpec) (*flag.FlagSet, erro
 	return set, nil
 }
 
+func validateFlagName(f Flag) error {
+	name, ok := rawFlagName(f)
+	if !ok || !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 rawFlagName(f Flag) (string, bool) {
+	v := reflect.ValueOf(f)
+	for v.IsValid() && v.Kind() == reflect.Pointer {
+		if v.IsNil() {
+			return "", false
+		}
+		v = v.Elem()
+	}
+	if !v.IsValid() || v.Kind() != reflect.Struct {
+		return "", false
+	}
+
+	field := v.FieldByName("Name")
+	if !field.IsValid() || field.Kind() != reflect.String {
+		return "", false
+	}
+
+	return field.String(), true
+}
+
 func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) {
 	switch ff.Value.(type) {
 	case Serializer:

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 {