Reuse flag name validator for better errors (#52)

internal/flag/flag.go

@@ -71,7 +71,7 @@ type Flag[T Flaggable] struct {
 //	flag.New(&force, "force", 'f', false, "Force deletion without confirmation")
 func New[T Flaggable](p *T, name string, short rune, value T, usage string) (Flag[T], error) {
 	if err := validateFlagName(name); err != nil {
-		return Flag[T]{}, fmt.Errorf("invalid flag name: %w", err)
+		return Flag[T]{}, fmt.Errorf("invalid flag name %q: %w", name, err)
 	}
 	if err := validateFlagShort(short); err != nil {
 		return Flag[T]{}, fmt.Errorf("invalid shorthand for flag %q: %w", name, err)
@@ -392,24 +392,28 @@ func validateFlagName(name string) error {
 	// Hyphen must be in between "words" like "set-default"
 	// we can't have "-default" or "default-"
 	if found && after == "" {
-		return fmt.Errorf("trailing hyphen: %q", name)
+		return errors.New("trailing hyphen")
 	}
 
 	if found && before == "" {
-		return fmt.Errorf("leading hyphen: %q", name)
+		return errors.New("leading hyphen")
 	}
 	for _, char := range name {
+		// No whitespace
+		if unicode.IsSpace(char) {
+			return errors.New("cannot contain whitespace")
+		}
 		// Only ASCII characters allowed
 		if char > unicode.MaxASCII {
-			return fmt.Errorf("non ascii character: %q", string(char))
+			return fmt.Errorf("contains non ascii character: %q", string(char))
 		}
 		// Only non-letter character allowed is a hyphen
 		if !unicode.IsLetter(char) && char != '-' {
-			return fmt.Errorf("not ascii letter: %q", string(char))
+			return fmt.Errorf("contains non ascii letter: %q", string(char))
 		}
 		// Any upper case letters are not allowed
 		if unicode.IsLetter(char) && !unicode.IsLower(char) {
-			return fmt.Errorf("upper case character %q", string(char))
+			return fmt.Errorf("contains upper case character %q", string(char))
 		}
 	}
 

internal/flag/flag_test.go

@@ -545,49 +545,55 @@ func TestFlagValidation(t *testing.T) {
 			name:     "empty name",
 			flagName: "",
 			wantErr:  true,
-			errMsg:   "invalid flag name: must not be empty",
+			errMsg:   `invalid flag name "": must not be empty`,
+		},
+		{
+			name:     "whitespace",
+			flagName: " ",
+			wantErr:  true,
+			errMsg:   `invalid flag name " ": cannot contain whitespace`,
 		},
 		{
 			name:     "mixed case",
 			flagName: "heLlO",
 			wantErr:  true,
-			errMsg:   `invalid flag name: upper case character "L"`,
+			errMsg:   `invalid flag name "heLlO": contains upper case character "L"`,
 		},
 		{
 			name:     "underscore",
 			flagName: "set_default",
 			wantErr:  true,
-			errMsg:   `invalid flag name: not ascii letter: "_"`,
+			errMsg:   `invalid flag name "set_default": contains non ascii letter: "_"`,
 		},
 		{
 			name:     "digits",
 			flagName: "some-06digit",
 			wantErr:  true,
-			errMsg:   `invalid flag name: not ascii letter: "0"`,
+			errMsg:   `invalid flag name "some-06digit": contains non ascii letter: "0"`,
 		},
 		{
 			name:     "just hyphen",
 			flagName: "-",
 			wantErr:  true,
-			errMsg:   `invalid flag name: trailing hyphen: "-"`,
+			errMsg:   `invalid flag name "-": trailing hyphen`,
 		},
 		{
 			name:     "leading hyphen",
 			flagName: "-something",
 			wantErr:  true,
-			errMsg:   `invalid flag name: leading hyphen: "-something"`,
+			errMsg:   `invalid flag name "-something": leading hyphen`,
 		},
 		{
 			name:     "trailing hyphen",
 			flagName: "something-",
 			wantErr:  true,
-			errMsg:   `invalid flag name: trailing hyphen: "something-"`,
+			errMsg:   `invalid flag name "something-": trailing hyphen`,
 		},
 		{
 			name:     "non ascii",
 			flagName: "語ç日ð本",
 			wantErr:  true,
-			errMsg:   `invalid flag name: non ascii character: "語"`,
+			errMsg:   `invalid flag name "語ç日ð本": contains non ascii character: "語"`,
 		},
 		{
 			name:     "short is digit",

internal/flag/set.go

@@ -94,13 +94,11 @@ func (s *Set) parseLongFlag(long string, rest []string) (remaining []string, err
 	// Could either be "flag" or "flag=value"
 	name := strings.TrimPrefix(long, "--")
 
-	// TODO: A proper validation function like validateFlagName
-	if name == "" {
-		return nil, errors.New("invalid flag syntax: empty flag name")
-	}
-
 	// name will either be the entire string or the name before the "="
 	name, equalsValue, containsEquals := strings.Cut(name, "=")
+	if err := validateFlagName(name); err != nil {
+		return nil, fmt.Errorf("invalid flag name %q: %w", name, err)
+	}
 	flag, exists := s.flags[name]
 	if !exists {
 		return nil, fmt.Errorf("unrecognised flag: --%s", name)

internal/flag/set_test.go

@@ -72,7 +72,47 @@ func TestParse(t *testing.T) {
 			},
 			args:    []string{"--"},
 			wantErr: true,
-			errMsg:  "invalid flag syntax: empty flag name",
+			errMsg:  `invalid flag name "": must not be empty`,
+		},
+		{
+			name: "bad syntax extra hyphen",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				return flag.NewSet()
+			},
+			args:    []string{"---"},
+			wantErr: true,
+			errMsg:  `invalid flag name "-": trailing hyphen`,
+		},
+		{
+			name: "bad syntax leading whitespace",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				return flag.NewSet()
+			},
+			args:    []string{"-- delete"},
+			wantErr: true,
+			errMsg:  `invalid flag name " delete": cannot contain whitespace`,
+		},
+		{
+			name: "bad syntax trailing whitespace",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				return flag.NewSet()
+			},
+			args:    []string{"--delete "},
+			wantErr: true,
+			errMsg:  `invalid flag name "delete ": cannot contain whitespace`,
+		},
+		{
+			name: "bad syntax internal whitespace",
+			newSet: func(t *testing.T) *flag.Set {
+				t.Helper()
+				return flag.NewSet()
+			},
+			args:    []string{"--de lete"},
+			wantErr: true,
+			errMsg:  `invalid flag name "de lete": cannot contain whitespace`,
 		},
 		{
 			name: "valid long",