From e778224db2fd0e9e61d93335c98cd6f1ae369077 Mon Sep 17 00:00:00 2001 From: Alex Flint Date: Fri, 23 May 2025 11:01:34 -0400 Subject: [PATCH] improve handling of negative numbers --- parse.go | 37 ++++++++++++--------- parse_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 99 insertions(+), 27 deletions(-) diff --git a/parse.go b/parse.go index bf6784a..d5eee12 100644 --- a/parse.go +++ b/parse.go @@ -700,7 +700,7 @@ func (p *Parser) process(args []string) error { if spec.cardinality == multiple { var values []string if value == "" { - for i+1 < len(args) && !isFlag(args[i+1]) && args[i+1] != "--" { + for i+1 < len(args) && isValue(args[i+1], spec.field.Type, specs) && args[i+1] != "--" { values = append(values, args[i+1]) i++ if spec.separate { @@ -728,7 +728,7 @@ func (p *Parser) process(args []string) error { if i+1 == len(args) { return fmt.Errorf("missing value for %s", arg) } - if !nextIsNumeric(spec.field.Type, args[i+1]) && isFlag(args[i+1]) { + if !isValue(args[i+1], spec.field.Type, specs) { return fmt.Errorf("missing value for %s", arg) } value = args[i+1] @@ -801,24 +801,31 @@ func (p *Parser) process(args []string) error { return nil } -func nextIsNumeric(t reflect.Type, s string) bool { - switch t.Kind() { - case reflect.Ptr: - return nextIsNumeric(t.Elem(), s) - case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: - v := reflect.New(t) - err := scalar.ParseValue(v, s) - return err == nil - default: - return false - } -} - // isFlag returns true if a token is a flag such as "-v" or "--user" but not "-" or "--" func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } +// isValue returns true if a token should be consumed as a value for a flag of type t. This +// is almost always the inverse of isFlag. The one exception is for negative numbers, in which +// case we check the list of active options and return true if its not present there. +func isValue(s string, t reflect.Type, specs []*spec) bool { + switch t.Kind() { + case reflect.Ptr, reflect.Slice: + return isValue(s, t.Elem(), specs) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Float32, reflect.Float64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + v := reflect.New(t) + err := scalar.ParseValue(v, s) + // if value can be parsed and is not an explicit option declared elsewhere, then use it as a value + if err == nil && (!strings.HasPrefix(s, "-") || findOption(specs, strings.TrimPrefix(s, "-")) == nil) { + return true + } + } + + // default case that is used in all cases other than negative numbers: inverse of isFlag + return !isFlag(s) +} + // val returns a reflect.Value corresponding to the current value for the // given path func (p *Parser) val(dest path) reflect.Value { diff --git a/parse_test.go b/parse_test.go index 249cbf3..4d2df63 100644 --- a/parse_test.go +++ b/parse_test.go @@ -120,17 +120,91 @@ func TestNegativeInt(t *testing.T) { assert.EqualValues(t, args.Foo, -100) } +func TestNegativeFloat(t *testing.T) { + var args struct { + Foo float64 + } + err := parse("-foo -99", &args) + require.NoError(t, err) + assert.EqualValues(t, args.Foo, -99) +} + +func TestNumericFlag(t *testing.T) { + var args struct { + UseIPv6 bool `arg:"-6"` + Foo int + } + err := parse("-6", &args) + require.NoError(t, err) + assert.EqualValues(t, args.UseIPv6, true) +} + +func TestNumericFlagTakesPrecedence(t *testing.T) { + var args struct { + UseIPv6 bool `arg:"-6"` + Foo int + } + err := parse("-foo -6", &args) + require.Error(t, err) +} + +func TestRepeatedNegativeInts(t *testing.T) { + var args struct { + Ints []int `arg:"--numbers"` + } + err := parse("--numbers -1 -2 -6", &args) + require.NoError(t, err) + assert.EqualValues(t, args.Ints, []int{-1, -2, -6}) +} + +func TestRepeatedNegativeFloats(t *testing.T) { + var args struct { + Floats []float32 `arg:"--numbers"` + } + err := parse("--numbers -1 -2 -6", &args) + require.NoError(t, err) + assert.EqualValues(t, args.Floats, []float32{-1, -2, -6}) +} + +func TestRepeatedNegativeFloatsThenNumericFlag(t *testing.T) { + var args struct { + Floats []float32 `arg:"--numbers"` + UseIPv6 bool `arg:"-6"` + } + err := parse("--numbers -1 -2 -6", &args) + require.NoError(t, err) + assert.EqualValues(t, args.Floats, []float32{-1, -2}) + assert.True(t, args.UseIPv6) +} + +func TestRepeatedNegativeFloatsThenNonexistentFlag(t *testing.T) { + var args struct { + Floats []float32 `arg:"--numbers"` + UseIPv6 bool `arg:"-6"` + } + err := parse("--numbers -1 -2 -n", &args) + require.Error(t, err, "unknown argument -n") +} + +func TestRepeatedNegativeIntsThenFloat(t *testing.T) { + var args struct { + Ints []int `arg:"--numbers"` + } + err := parse("--numbers -1 -2 -0.1", &args) + require.Error(t, err, "unknown argument -0.1") +} + func TestNegativeIntAndFloatAndTricks(t *testing.T) { var args struct { Foo int Bar float64 N int `arg:"--100"` } - err := parse("-foo -100 -bar -60.14 -100 -100", &args) + err := parse("-foo -99 -bar -60.14 -100 -101", &args) require.NoError(t, err) - assert.EqualValues(t, args.Foo, -100) + assert.EqualValues(t, args.Foo, -99) assert.EqualValues(t, args.Bar, -60.14) - assert.EqualValues(t, args.N, -100) + assert.EqualValues(t, args.N, -101) } func TestUint(t *testing.T) { @@ -525,15 +599,6 @@ func TestMissingValueInMiddle(t *testing.T) { assert.Error(t, err) } -func TestNegativeValue(t *testing.T) { - var args struct { - Foo int - } - err := parse("--foo -123", &args) - require.NoError(t, err) - assert.Equal(t, -123, args.Foo) -} - func TestInvalidInt(t *testing.T) { var args struct { Foo int