From 9e28ee65456c420d6cfe017a767fdda8d0d87ef4 Mon Sep 17 00:00:00 2001 From: Celestino Amoroso Date: Fri, 6 Mar 2026 10:36:07 +0100 Subject: [PATCH] final checks on opt values moved after option parsing is complete --- cli.go | 108 +++++++++++++++++++++++++++----------------- opt-base.go | 6 +-- opt-bool.go | 4 +- opt-int-array.go | 4 +- opt-int.go | 4 +- opt-multi.go | 4 +- opt-string-array.go | 4 +- opt-string-map.go | 4 +- opt-string.go | 4 +- opt_test.go | 107 +++++++++++++++++++++++-------------------- parser.go | 8 +--- 11 files changed, 142 insertions(+), 115 deletions(-) diff --git a/cli.go b/cli.go index b195eab..59caf51 100644 --- a/cli.go +++ b/cli.go @@ -25,7 +25,7 @@ type cliOptionParser interface { isSet() bool isHidden() bool requiresValue() bool - finalCheck(cliValue string) (err error) + finalCheck() (err error) } type OptManager interface { @@ -33,7 +33,7 @@ type OptManager interface { } type SpecialValueFunc func(manager OptManager, cliValue string, currentValue any) (value any, err error) -type FinalCheckFunc func(cliValue string, currentValue any) (err error) +type FinalCheckFunc func(currentValue any) (err error) type OptReference interface { AddSpecialValue(cliValue string, specialFunc SpecialValueFunc) @@ -158,17 +158,12 @@ func (cli *CliParser) addHelpAndVersion() { } -func (cli *CliParser) Parse() (err error) { - var arg string - var i int - var commandArgs []string +func (cli *CliParser) parseOptions() (commandArgs []string, err error) { + // var commandArgs []string var optionsAllowed bool = true - cli.addHelpAndVersion() - - // first parse options and collect argument in the args array skipNext := false - for i, arg = range cli.cliArgs[1:] { + for i, arg := range cli.cliArgs[1:] { if skipNext { skipNext = false } else { @@ -186,49 +181,78 @@ func (cli *CliParser) Parse() (err error) { } } } + return +} - // then parse collected arguments - if err == nil { - var n int - i = 0 - - // acquire arguments as required by the argSpecs - specIndex := -1 - for index, argSpec := range cli.argSpecs { - specIndex = index - if n, err = argSpec.parse(cli, specIndex, commandArgs, i); err != nil { - break - } - i += n - if i >= len(commandArgs) { - break - } +func (cli *CliParser) checkOptionValues() (err error) { + for _, opt := range cli.options { + if err = opt.finalCheck(); err != nil { + break } + } + return +} - // check if there are remaining arg-specs that require a value - if err == nil { - if i < len(commandArgs) { - err = fmt.Errorf("too many arguments: %d allowed", i) - } else { - specIndex++ - if specIndex < len(cli.argSpecs) { - // skip all non required args - for _, spec := range cli.argSpecs[specIndex:] { - if !spec.getBase().required { - specIndex++ - } +func (cli *CliParser) parseCommandArgs(commandArgs []string) (err error) { + var n int + + i := 0 + // acquire arguments as required by the argSpecs + specIndex := -1 + for index, argSpec := range cli.argSpecs { + specIndex = index + if n, err = argSpec.parse(cli, specIndex, commandArgs, i); err != nil { + break + } + i += n + if i >= len(commandArgs) { + break + } + } + + // check if there are remaining arg-specs that require a value + if err == nil { + if i < len(commandArgs) { + err = fmt.Errorf("too many arguments: %d allowed", i) + } else { + specIndex++ + if specIndex < len(cli.argSpecs) { + // skip all non required args + for _, spec := range cli.argSpecs[specIndex:] { + if !spec.getBase().required { + specIndex++ } } - // return error if there are remaining arg-specs that require a value - if specIndex < len(cli.argSpecs) { - err = errTooFewArguments(len(cli.argSpecs)) - } + } + // return error if there are remaining arg-specs that require a value + if specIndex < len(cli.argSpecs) { + err = errTooFewArguments(len(cli.argSpecs)) } } } return } +func (cli *CliParser) Parse() (err error) { + var commandArgs []string + + cli.addHelpAndVersion() + + // first parse options and collect command arguments in the args array + if commandArgs, err = cli.parseOptions(); err != nil { + return + } + + // do final checks + if err = cli.checkOptionValues(); err != nil { + return + } + + // then parse collected arguments + err = cli.parseCommandArgs(commandArgs) + return +} + func (cli *CliParser) checkCompatibility(arg string) (err error) { var opti cliOptionParser if opti = cli.findOptionByArg(arg); opti != nil { diff --git a/opt-base.go b/opt-base.go index 3cf1b0a..ec86555 100644 --- a/opt-base.go +++ b/opt-base.go @@ -153,13 +153,13 @@ func (opt *cliOptionBase) fetchOptionValue(parser cliParser, argIndex int, value return } -func (opt *cliOptionBase) finalCheck(cliValue string) (err error) { +func (opt *cliOptionBase) finalCheck() (err error) { return } -func (opt *cliOptionBase) checkValue(cliValue string, value any) (err error) { +func (opt *cliOptionBase) checkValue(value any) (err error) { if opt.finalCheckFunc != nil { - err = opt.finalCheckFunc(cliValue, value) + err = opt.finalCheckFunc(value) } return } diff --git a/opt-bool.go b/opt-bool.go index 66ba054..cb565ec 100644 --- a/opt-bool.go +++ b/opt-bool.go @@ -67,9 +67,9 @@ func (opt *cliOptionBool) parse(parser cliParser, argIndex int, valuePtr *string return } -func (opt *cliOptionBool) finalCheck(cliValue string) (err error) { +func (opt *cliOptionBool) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddBoolOpt(name, short string, targetVar *bool, description string, aliases ...string) OptReference { diff --git a/opt-int-array.go b/opt-int-array.go index d039aaf..5975627 100644 --- a/opt-int-array.go +++ b/opt-int-array.go @@ -97,9 +97,9 @@ func (opt *cliOptionIntArray) parse(parser cliParser, argIndex int, valuePtr *st return } -func (opt *cliOptionIntArray) finalCheck(cliValue string) (err error) { +func (opt *cliOptionIntArray) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddIntArrayOpt(name, short string, targetVar *[]int, defaultValue []int, description string, aliases ...string) OptReference { diff --git a/opt-int.go b/opt-int.go index 7c28314..20a6e72 100644 --- a/opt-int.go +++ b/opt-int.go @@ -57,9 +57,9 @@ func (opt *cliOptionInt) parse(parser cliParser, argIndex int, valuePtr *string) } return } -func (opt *cliOptionInt) finalCheck(cliValue string) (err error) { +func (opt *cliOptionInt) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddIntOpt(name, short string, targetVar *int, defaultValue int, description string, aliases ...string) OptReference { diff --git a/opt-multi.go b/opt-multi.go index 01e5c54..fbc2fd2 100644 --- a/opt-multi.go +++ b/opt-multi.go @@ -53,9 +53,9 @@ func (opt *cliOptionMulti) parse(parser cliParser, argIndex int, valuePtr *strin value = "true" return } -func (opt *cliOptionMulti) finalCheck(cliValue string) (err error) { +func (opt *cliOptionMulti) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddMultiOpt(name, short string, targetVar *int, defaultValue int, description string, aliases ...string) OptReference { diff --git a/opt-string-array.go b/opt-string-array.go index b914df9..ed52c56 100644 --- a/opt-string-array.go +++ b/opt-string-array.go @@ -85,7 +85,7 @@ func (cli *CliParser) AddStringArrayOpt(name, short string, targetVar *[]string, return opt } -func (opt *cliOptionStringArray) finalCheck(cliValue string) (err error) { +func (opt *cliOptionStringArray) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } diff --git a/opt-string-map.go b/opt-string-map.go index e19c475..b35590e 100644 --- a/opt-string-map.go +++ b/opt-string-map.go @@ -89,9 +89,9 @@ func (opt *cliOptionStringMap) parse(parser cliParser, argIndex int, valuePtr *s return } -func (opt *cliOptionStringMap) finalCheck(cliValue string) (err error) { +func (opt *cliOptionStringMap) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddStringMapOpt(name, short string, targetVar *map[string]string, defaultValue map[string]string, description string, aliases ...string) OptReference { diff --git a/opt-string.go b/opt-string.go index fc723b9..769f416 100644 --- a/opt-string.go +++ b/opt-string.go @@ -55,9 +55,9 @@ func (opt *cliOptionString) parse(parser cliParser, argIndex int, valuePtr *stri } return } -func (opt *cliOptionString) finalCheck(cliValue string) (err error) { +func (opt *cliOptionString) finalCheck() (err error) { currentValue, _ := opt.getTargetVar() - return opt.getBase().checkValue(cliValue, currentValue) + return opt.getBase().checkValue(currentValue) } func (cli *CliParser) AddStringOpt(name, short string, targetVar *string, defaultValue string, description string, aliases ...string) OptReference { diff --git a/opt_test.go b/opt_test.go index c5fba1f..ce7555f 100644 --- a/opt_test.go +++ b/opt_test.go @@ -37,7 +37,7 @@ func TestOneOptWithEqual(t *testing.T) { func addBoxOption(cli *CliParser, boxPtr *[]int) { // Define options optRef := cli.AddIntArrayOpt("box", "b", boxPtr, []int{1, 2, 3, 4}, "Box spec: Left,Width,Top,Height; provide 4 int values") - optRef.OnFinalCheck(func(cliValue string, currentValue any) (err error) { + optRef.OnFinalCheck(func(currentValue any) (err error) { if array, ok := currentValue.([]int); ok { if len(array) != 4 { err = fmt.Errorf("--box option requires exactly 4 items, %d provided", len(array)) @@ -49,8 +49,34 @@ func addBoxOption(cli *CliParser, boxPtr *[]int) { }) } -func TestIntArrayOpt(t *testing.T) { +func testIntArrayOptOk(t *testing.T, n int, args []string) { var box []int + var cli CliParser + t.Logf("Arg set n. %d", n) + addBoxOption(&cli, &box) + cli.Init(args, "1.0.0", "TestIntArrayOpt description") + if err := cli.Parse(); err != nil { + t.Error(err) + } else if len(box) != 4 { + t.Errorf(`Expected 4 items, got %d`, len(box)) + } +} + +func testIntArrayOptKo(t *testing.T, n int, args []string, msg string) { + var box []int + var cli CliParser + t.Logf("Arg set n. %d", n) + addBoxOption(&cli, &box) + cli.Init(args, "1.0.0", "TestIntArrayOpt description") + if err := cli.Parse(); err == nil { + t.Errorf("Expected error, got nil") + } else if err.Error() != msg { + t.Errorf(`Invalid error: %q; expected: %q`, err.Error(), msg) + } +} + +func TestIntArrayOpt(t *testing.T) { + var args []string // Always recover from panic to return error before adding options defer func() { if r := recover(); r != nil { @@ -61,68 +87,49 @@ func TestIntArrayOpt(t *testing.T) { } }() - args_1 := []string{ + n := 0 + + n++ + args = []string{ "TestIntArrayOpt", "--box=100,200,150,450", } + testIntArrayOptOk(t, n, args) - if true { - var cli CliParser - t.Logf("Arg set n. 1") - addBoxOption(&cli, &box) - cli.Init(args_1, "1.0.0", "TestIntArrayOpt description") - if err := cli.Parse(); err != nil { - t.Error(err) - } else if len(box) != 4 { - t.Errorf(`Expected 4 items, got %d`, len(box)) - } - } - - args_2 := []string{ + n++ + args = []string{ "TestIntArrayOpt", "--box=100,200,150", } - if true { - var cli CliParser - t.Logf("Arg set n. 2") - addBoxOption(&cli, &box) - cli.Init(args_2, "1.0.0", "TestIntArrayOpt description") - if err := cli.Parse(); err == nil { - t.Errorf("Expected error, got nil") - } else if err.Error() != "--box option requires exactly 4 items, 3 provided" { - t.Errorf(`Invalid error: %q; expected: "--box option requires exactly 4 items, 3 provided"`, err.Error()) - } - } + testIntArrayOptKo(t, n, args, "--box option requires exactly 4 items, 3 provided") - args_3 := []string{ + n++ + args = []string{ "TestIntArrayOpt", "--box", "100,200,150,450", } - if true { - var cli CliParser - t.Logf("Arg set n. 3") - addBoxOption(&cli, &box) - cli.Init(args_3, "1.0.0", "TestIntArrayOpt description") - if err := cli.Parse(); err != nil { - t.Error(err) - } else if len(box) != 4 { - t.Errorf(`Expected 4 items, got %d`, len(box)) - } - } + testIntArrayOptOk(t, n, args) - args_4 := []string{ + n++ + args = []string{ "TestIntArrayOpt", "--box", } - if true { - var cli CliParser - t.Logf("Arg set n. 4") - addBoxOption(&cli, &box) - cli.Init(args_4, "1.0.0", "TestIntArrayOpt description") - if err := cli.Parse(); err == nil { - t.Errorf("Expected error, got nil") - } else if err.Error() != `option "box" requires a value` { - t.Errorf(`Invalid error - Expected: 'option "box" requires a value'; got '%s'`, err.Error()) - } + testIntArrayOptKo(t, n, args, `option "box" requires a value`) + + n++ + args = []string{ + "TestIntArrayOpt", + "--box", "100,200,150", + "--box", "450", } + testIntArrayOptOk(t, n, args) + + n++ + args = []string{ + "TestIntArrayOpt", + "--box", "100,200,150", + "--box", "450,12", + } + testIntArrayOptKo(t, n, args, "--box option requires exactly 4 items, 5 provided") } diff --git a/parser.go b/parser.go index 51e2cc0..a518010 100644 --- a/parser.go +++ b/parser.go @@ -104,16 +104,12 @@ func (cli *CliParser) parseArg(arg string, index int) (skipNextArg bool, err err } for i, optName := range opts { if opt := cli.findOptionByArg(dashes + optName); opt != nil { - var optValue string if equalPresent && i == len(opts)-1 { - _, optValue, err = opt.parse(cli, index, &value) + _, _, err = opt.parse(cli, index, &value) } else if i < len(opts)-1 && opt.requiresValue() { err = errMissingOptionValue(dashes + optName) } else { - skipNextArg, optValue, err = opt.parse(cli, index, nil) - } - if err == nil && i == len(opts)-1 { - err = opt.finalCheck(optValue) + skipNextArg, _, err = opt.parse(cli, index, nil) } } else { err = errUnknownOption(dashes + optName)