Enum fields must be required or have a default.

This is a breaking change, but the previous behaviour was broken so I'm
not concerned.

Also made most programmer errors more useful by giving type.field
context information.

Fixes #179.
This commit is contained in:
Alec Thomas
2021-06-21 20:32:40 +09:30
parent c494f8b8f3
commit 247574041d
7 changed files with 33 additions and 23 deletions
+1 -1
View File
@@ -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.
+7 -9
View File
@@ -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
}
+1 -1
View File
@@ -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"
+9
View File
@@ -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 = "<anonymous struct>"
}
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...)
+5 -5
View File
@@ -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, "<anonymous struct>.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, "<anonymous struct>.Flag2: duplicate short flag -t")
}
+9 -6
View File
@@ -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
+1 -1
View File
@@ -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, "<anonymous struct>.Flag: invalid short flag name \"invalid\": invalid rune")
}