diff --git a/README.md b/README.md index 368b098..03996db 100644 --- a/README.md +++ b/README.md @@ -444,7 +444,7 @@ Tag | Description `format:"X"` | Format for parsing input, if supported. `sep:"X"` | Separator for sequences (defaults to ","). May be `none` to disable splitting. `mapsep:"X"` | Separator for maps (defaults to ";"). May be `none` to disable splitting. -`enum:"X,Y,..."` | Set of valid values allowed for this flag. +`enum:"X,Y,..."` | Set of valid values allowed for this flag. An enum field must be `required` or have a valid `default`. `group:"X"` | Logical group for a flag or command. `xor:"X,Y,..."` | Exclusive OR groups for flags. Only one flag in the group can be used which is restricted within the same command. When combined with `required`, at least one of the `xor` group will be required. `prefix:"X"` | Prefix for all sub-flags. diff --git a/build.go b/build.go index d2ca41c..22f9839 100644 --- a/build.go +++ b/build.go @@ -51,7 +51,7 @@ func flattenedFields(v reflect.Value) (out []flattenedField) { for i := 0; i < v.NumField(); i++ { ft := v.Type().Field(i) fv := v.Field(i) - tag := parseTag(fv, ft) + tag := parseTag(v, fv, ft) if tag.Ignored { continue } @@ -156,8 +156,7 @@ func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.S // a positional argument is provided to the child, and move it to the branching argument field. if tag.Arg { if len(child.Positional) == 0 { - fail("positional branch %s.%s must have at least one child positional argument named %q", - v.Type().Name(), ft.Name, name) + failField(v, ft, "positional branch must have at least one child positional argument named %q", name) } value := child.Positional[0] @@ -168,8 +167,7 @@ func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.S child.Name = value.Name if child.Name != name { - fail("first field in positional branch %s.%s must have the same name as the parent field (%s).", - v.Type().Name(), ft.Name, child.Name) + failField(v, ft, "first field in positional branch must have the same name as the parent field (%s).", child.Name) } child.Argument = value @@ -179,14 +177,14 @@ func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.S node.Children = append(node.Children, child) if len(child.Positional) > 0 && len(child.Children) > 0 { - fail("can't mix positional arguments and branching arguments on %s.%s", v.Type().Name(), ft.Name) + failField(v, ft, "can't mix positional arguments and branching arguments") } } func buildField(k *Kong, node *Node, v reflect.Value, ft reflect.StructField, fv reflect.Value, tag *Tag, name string, seenFlags map[string]bool) { mapper := k.registry.ForNamedValue(tag.Type, fv) if mapper == nil { - fail("unsupported field type %s.%s (of type %s), perhaps missing a cmd:\"\" tag?", v.Type(), ft.Name, ft.Type) + failField(v, ft, "unsupported field type %s, perhaps missing a cmd:\"\" tag?", ft.Type) } value := &Value{ @@ -209,13 +207,13 @@ func buildField(k *Kong, node *Node, v reflect.Value, ft reflect.StructField, fv node.Positional = append(node.Positional, value) } else { if seenFlags["--"+value.Name] { - fail("duplicate flag --%s", value.Name) + failField(v, ft, "duplicate flag --%s", value.Name) } else { seenFlags["--"+value.Name] = true } if tag.Short != 0 { if seenFlags["-"+string(tag.Short)] { - fail("duplicate short flag -%c", tag.Short) + failField(v, ft, "duplicate short flag -%c", tag.Short) } else { seenFlags["-"+string(tag.Short)] = true } diff --git a/config_test.go b/config_test.go index 69e279d..a63205b 100644 --- a/config_test.go +++ b/config_test.go @@ -32,7 +32,7 @@ func TestMultipleConfigLoading(t *testing.T) { func TestConfigValidation(t *testing.T) { var cli struct { - Flag string `json:"flag,omitempty" enum:"valid"` + Flag string `json:"flag,omitempty" enum:"valid" required:""` } cli.Flag = "invalid" diff --git a/kong.go b/kong.go index f02b867..3edfc3b 100644 --- a/kong.go +++ b/kong.go @@ -22,6 +22,15 @@ func fail(format string, args ...interface{}) { panic(Error{msg: fmt.Sprintf(format, args...)}) } +func failField(parent reflect.Value, field reflect.StructField, format string, args ...interface{}) { + name := parent.Type().Name() + if name == "" { + name = "" + } + msg := fmt.Sprintf("%s.%s: %s", name, field.Name, fmt.Sprintf(format, args...)) + panic(Error{msg: msg}) +} + // Must creates a new Parser or panics if there is an error. func Must(ast interface{}, options ...Option) *Kong { k, err := New(ast, options...) diff --git a/kong_test.go b/kong_test.go index 1a8af52..37f7529 100644 --- a/kong_test.go +++ b/kong_test.go @@ -643,7 +643,7 @@ func TestRun(t *testing.T) { func TestInterpolationIntoModel(t *testing.T) { var cli struct { Flag string `default:"${default}" help:"Help, I need ${somebody}" enum:"${enum}"` - EnumRef string `enum:"a,b" help:"One of ${enum}"` + EnumRef string `enum:"a,b" required:"" help:"One of ${enum}"` } _, err := kong.New(&cli) require.Error(t, err) @@ -763,7 +763,7 @@ func TestHooksCalledForDefault(t *testing.T) { func TestEnum(t *testing.T) { var cli struct { - Flag string `enum:"a,b,c"` + Flag string `enum:"a,b,c" required:""` } _, err := mustNew(t, &cli).Parse([]string{"--flag", "d"}) require.EqualError(t, err, "--flag must be one of \"a\",\"b\",\"c\" but got \"d\"") @@ -978,7 +978,7 @@ func TestIssue153(t *testing.T) { func TestEnumArg(t *testing.T) { var cli struct { Nested struct { - One string `arg:"" enum:"a,b,c"` + One string `arg:"" enum:"a,b,c" required:""` Two string `arg:""` } `cmd:""` } @@ -1142,7 +1142,7 @@ func TestDuplicateShortflags(t *testing.T) { Flag2 bool `short:"t"` }{} _, err := kong.New(&cli) - require.EqualError(t, err, "duplicate short flag -t") + require.EqualError(t, err, ".Flag2: duplicate short flag -t") } func TestDuplicateNestedShortFlags(t *testing.T) { @@ -1153,5 +1153,5 @@ func TestDuplicateNestedShortFlags(t *testing.T) { } `cmd:""` }{} _, err := kong.New(&cli) - require.EqualError(t, err, "duplicate short flag -t") + require.EqualError(t, err, ".Flag2: duplicate short flag -t") } diff --git a/tag.go b/tag.go index 9ecde85..7c9ae99 100644 --- a/tag.go +++ b/tag.go @@ -129,7 +129,7 @@ func tagSplitFn(r rune) bool { return r == ',' || r == ' ' } -func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { +func parseTag(parent, fv reflect.Value, ft reflect.StructField) *Tag { if ft.Tag.Get("kong") == "-" { t := newEmptyTag() t.Ignored = true @@ -146,7 +146,7 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { required := t.Has("required") optional := t.Has("optional") if required && optional { - fail("can't specify both required and optional") + failField(parent, ft, "can't specify both required and optional") } t.Required = required t.Optional = optional @@ -161,7 +161,7 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { t.Env = t.Get("env") t.Short, err = t.GetRune("short") if err != nil && t.Get("short") != "" { - fail("invalid short flag name %q: %s", t.Get("short"), err) + failField(parent, ft, "invalid short flag name %q: %s", t.Get("short"), err) } t.Hidden = t.Has("hidden") t.Format = t.Get("format") @@ -175,7 +175,7 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { t.Embed = t.Has("embed") negatable := t.Has("negatable") if negatable && ft.Type.Kind() != reflect.Bool { - fail("negatable can only be set on booleans") + failField(parent, ft, "negatable can only be set on booleans") } t.Negatable = negatable aliases := t.Get("aliases") @@ -186,7 +186,7 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { for _, set := range t.GetAll("set") { parts := strings.SplitN(set, "=", 2) if len(parts) == 0 { - fail("set should be in the form key=value but got %q", set) + failField(parent, ft, "set should be in the form key=value but got %q", set) } t.Vars[parts[0]] = parts[1] } @@ -195,9 +195,12 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag { t.PlaceHolder = strings.ToUpper(dashedString(fv.Type().Name())) } t.Enum = t.Get("enum") + if t.Enum != "" && !(t.Required || t.Default != "") { + failField(parent, ft, "enum value is only valid if it is either required or has a valid default value") + } passthrough := t.Has("passthrough") if passthrough && !t.Arg { - fail("passthrough only makes sense for positional arguments") + failField(parent, ft, "passthrough only makes sense for positional arguments") } t.Passthrough = passthrough return t diff --git a/tag_test.go b/tag_test.go index ae8c38d..ca9f213 100644 --- a/tag_test.go +++ b/tag_test.go @@ -198,5 +198,5 @@ func TestInvalidRuneErrors(t *testing.T) { Flag bool `short:"invalid"` }{} _, err := kong.New(&cli) - require.EqualError(t, err, "invalid short flag name \"invalid\": invalid rune") + require.EqualError(t, err, ".Flag: invalid short flag name \"invalid\": invalid rune") }