From 70e53f62be77e03f376be845efacbd99c00024ca Mon Sep 17 00:00:00 2001 From: Unai Martinez-Corral <38422348+umarcor@users.noreply.github.com> Date: Sat, 10 Sep 2022 15:33:34 +0200 Subject: [PATCH] Deprecate ExactValidArgs() and test combinations of args validators (#1643) * deprecate ExactValidArgs in favour of MatchAll(OnlyValidArgs, ...) * test combinations of args validators * adjust docs --- args.go | 24 ++--- args_test.go | 250 +++++++++++++++++++++++++++++++++++++++++++++----- user_guide.md | 52 +++++++---- 3 files changed, 272 insertions(+), 54 deletions(-) diff --git a/args.go b/args.go index 20a022b..e2f40d5 100644 --- a/args.go +++ b/args.go @@ -32,7 +32,8 @@ func NoArgs(cmd *Command, args []string) error { return nil } -// OnlyValidArgs returns an error if any args are not in the list of ValidArgs. +// OnlyValidArgs returns an error if there are any positional args that are not in +// the `ValidArgs` field of `Command` func OnlyValidArgs(cmd *Command, args []string) error { if len(cmd.ValidArgs) > 0 { // Remove any description that may be included in ValidArgs. @@ -41,7 +42,6 @@ func OnlyValidArgs(cmd *Command, args []string) error { for _, v := range cmd.ValidArgs { validArgs = append(validArgs, strings.Split(v, "\t")[0]) } - for _, v := range args { if !stringInSlice(v, validArgs) { return fmt.Errorf("invalid argument %q for %q%s", v, cmd.CommandPath(), cmd.findSuggestions(args[0])) @@ -86,18 +86,6 @@ func ExactArgs(n int) PositionalArgs { } } -// ExactValidArgs returns an error if -// there are not exactly N positional args OR -// there are any positional args that are not in the `ValidArgs` field of `Command` -func ExactValidArgs(n int) PositionalArgs { - return func(cmd *Command, args []string) error { - if err := ExactArgs(n)(cmd, args); err != nil { - return err - } - return OnlyValidArgs(cmd, args) - } -} - // RangeArgs returns an error if the number of args is not within the expected range. func RangeArgs(min int, max int) PositionalArgs { return func(cmd *Command, args []string) error { @@ -119,3 +107,11 @@ func MatchAll(pargs ...PositionalArgs) PositionalArgs { return nil } } + +// ExactValidArgs returns an error if there are not exactly N positional args OR +// there are any positional args that are not in the `ValidArgs` field of `Command` +// +// Deprecated: use MatchAll(ExactArgs(n), OnlyValidArgs) instead +func ExactValidArgs(n int) PositionalArgs { + return MatchAll(ExactArgs(n), OnlyValidArgs) +} diff --git a/args_test.go b/args_test.go index 6bd41c8..ae4df2e 100644 --- a/args_test.go +++ b/args_test.go @@ -27,7 +27,7 @@ func expectSuccess(output string, err error, t *testing.T) { } } -func validWithInvalidArgs(err error, t *testing.T) { +func validOnlyWithInvalidArgs(err error, t *testing.T) { if err == nil { t.Fatal("Expected an error") } @@ -38,12 +38,12 @@ func validWithInvalidArgs(err error, t *testing.T) { } } -func noArgsWithArgs(err error, t *testing.T) { +func noArgsWithArgs(err error, t *testing.T, arg string) { if err == nil { t.Fatal("Expected an error") } got := err.Error() - expected := `unknown command "illegal" for "c"` + expected := `unknown command "` + arg + `" for "c"` if got != expected { t.Errorf("Expected: %q, got: %q", expected, got) } @@ -93,102 +93,280 @@ func rangeArgsWithInvalidCount(err error, t *testing.T) { } } +// NoArgs + func TestNoArgs(t *testing.T) { c := getCommand(NoArgs, false) output, err := executeCommand(c) expectSuccess(output, err, t) } -func TestNoArgsWithArgs(t *testing.T) { +func TestNoArgs_WithArgs(t *testing.T) { c := getCommand(NoArgs, false) - _, err := executeCommand(c, "illegal") - noArgsWithArgs(err, t) + _, err := executeCommand(c, "one") + noArgsWithArgs(err, t, "one") } +func TestNoArgs_WithValid_WithArgs(t *testing.T) { + c := getCommand(NoArgs, true) + _, err := executeCommand(c, "one") + noArgsWithArgs(err, t, "one") +} + +func TestNoArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(NoArgs, true) + _, err := executeCommand(c, "a") + noArgsWithArgs(err, t, "a") +} + +func TestNoArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, NoArgs), true) + _, err := executeCommand(c, "a") + validOnlyWithInvalidArgs(err, t) +} + +// OnlyValidArgs + func TestOnlyValidArgs(t *testing.T) { c := getCommand(OnlyValidArgs, true) output, err := executeCommand(c, "one", "two") expectSuccess(output, err, t) } -func TestOnlyValidArgsWithInvalidArgs(t *testing.T) { +func TestOnlyValidArgs_WithInvalidArgs(t *testing.T) { c := getCommand(OnlyValidArgs, true) _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) + validOnlyWithInvalidArgs(err, t) } +// ArbitraryArgs + func TestArbitraryArgs(t *testing.T) { c := getCommand(ArbitraryArgs, false) output, err := executeCommand(c, "a", "b") expectSuccess(output, err, t) } +func TestArbitraryArgs_WithValid(t *testing.T) { + c := getCommand(ArbitraryArgs, true) + output, err := executeCommand(c, "one", "two") + expectSuccess(output, err, t) +} + +func TestArbitraryArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(ArbitraryArgs, true) + output, err := executeCommand(c, "a") + expectSuccess(output, err, t) +} + +func TestArbitraryArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, ArbitraryArgs), true) + _, err := executeCommand(c, "a") + validOnlyWithInvalidArgs(err, t) +} + +// MinimumNArgs + func TestMinimumNArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } -func TestMinimumNArgsWithLessArgs(t *testing.T) { +func TestMinimumNArgs_WithValid(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + output, err := executeCommand(c, "one", "three") + expectSuccess(output, err, t) +} + +func TestMinimumNArgs_WithValid__WithInvalidArgs(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + output, err := executeCommand(c, "a", "b") + expectSuccess(output, err, t) +} + +func TestMinimumNArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, MinimumNArgs(2)), true) + _, err := executeCommand(c, "a", "b") + validOnlyWithInvalidArgs(err, t) +} + +func TestMinimumNArgs_WithLessArgs(t *testing.T) { c := getCommand(MinimumNArgs(2), false) _, err := executeCommand(c, "a") minimumNArgsWithLessArgs(err, t) } +func TestMinimumNArgs_WithLessArgs_WithValid(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + _, err := executeCommand(c, "one") + minimumNArgsWithLessArgs(err, t) +} + +func TestMinimumNArgs_WithLessArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(MinimumNArgs(2), true) + _, err := executeCommand(c, "a") + minimumNArgsWithLessArgs(err, t) +} + +func TestMinimumNArgs_WithLessArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, MinimumNArgs(2)), true) + _, err := executeCommand(c, "a") + validOnlyWithInvalidArgs(err, t) +} + +// MaximumNArgs + func TestMaximumNArgs(t *testing.T) { c := getCommand(MaximumNArgs(3), false) output, err := executeCommand(c, "a", "b") expectSuccess(output, err, t) } -func TestMaximumNArgsWithMoreArgs(t *testing.T) { +func TestMaximumNArgs_WithValid(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + output, err := executeCommand(c, "one", "three") + expectSuccess(output, err, t) +} + +func TestMaximumNArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + output, err := executeCommand(c, "a", "b") + expectSuccess(output, err, t) +} + +func TestMaximumNArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, MaximumNArgs(2)), true) + _, err := executeCommand(c, "a", "b") + validOnlyWithInvalidArgs(err, t) +} + +func TestMaximumNArgs_WithMoreArgs(t *testing.T) { c := getCommand(MaximumNArgs(2), false) _, err := executeCommand(c, "a", "b", "c") maximumNArgsWithMoreArgs(err, t) } +func TestMaximumNArgs_WithMoreArgs_WithValid(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + _, err := executeCommand(c, "one", "three", "two") + maximumNArgsWithMoreArgs(err, t) +} + +func TestMaximumNArgs_WithMoreArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(MaximumNArgs(2), true) + _, err := executeCommand(c, "a", "b", "c") + maximumNArgsWithMoreArgs(err, t) +} + +func TestMaximumNArgs_WithMoreArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, MaximumNArgs(2)), true) + _, err := executeCommand(c, "a", "b", "c") + validOnlyWithInvalidArgs(err, t) +} + +// ExactArgs + func TestExactArgs(t *testing.T) { c := getCommand(ExactArgs(3), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } -func TestExactArgsWithInvalidCount(t *testing.T) { +func TestExactArgs_WithValid(t *testing.T) { + c := getCommand(ExactArgs(3), true) + output, err := executeCommand(c, "three", "one", "two") + expectSuccess(output, err, t) +} + +func TestExactArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(ExactArgs(3), true) + output, err := executeCommand(c, "three", "a", "two") + expectSuccess(output, err, t) +} + +func TestExactArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, ExactArgs(3)), true) + _, err := executeCommand(c, "three", "a", "two") + validOnlyWithInvalidArgs(err, t) +} + +func TestExactArgs_WithInvalidCount(t *testing.T) { c := getCommand(ExactArgs(2), false) _, err := executeCommand(c, "a", "b", "c") exactArgsWithInvalidCount(err, t) } -func TestExactValidArgs(t *testing.T) { - c := getCommand(ExactValidArgs(3), true) - output, err := executeCommand(c, "three", "one", "two") - expectSuccess(output, err, t) -} - -func TestExactValidArgsWithInvalidCount(t *testing.T) { - c := getCommand(ExactValidArgs(2), false) +func TestExactArgs_WithInvalidCount_WithValid(t *testing.T) { + c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "one", "two") exactArgsWithInvalidCount(err, t) } -func TestExactValidArgsWithInvalidArgs(t *testing.T) { - c := getCommand(ExactValidArgs(3), true) +func TestExactArgs_WithInvalidCount_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(ExactArgs(2), true) _, err := executeCommand(c, "three", "a", "two") - validWithInvalidArgs(err, t) + exactArgsWithInvalidCount(err, t) } +func TestExactArgs_WithInvalidCount_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, ExactArgs(2)), true) + _, err := executeCommand(c, "three", "a", "two") + validOnlyWithInvalidArgs(err, t) +} + +// RangeArgs + func TestRangeArgs(t *testing.T) { c := getCommand(RangeArgs(2, 4), false) output, err := executeCommand(c, "a", "b", "c") expectSuccess(output, err, t) } -func TestRangeArgsWithInvalidCount(t *testing.T) { +func TestRangeArgs_WithValid(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + output, err := executeCommand(c, "three", "one", "two") + expectSuccess(output, err, t) +} + +func TestRangeArgs_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + output, err := executeCommand(c, "three", "a", "two") + expectSuccess(output, err, t) +} + +func TestRangeArgs_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, RangeArgs(2, 4)), true) + _, err := executeCommand(c, "three", "a", "two") + validOnlyWithInvalidArgs(err, t) +} + +func TestRangeArgs_WithInvalidCount(t *testing.T) { c := getCommand(RangeArgs(2, 4), false) _, err := executeCommand(c, "a") rangeArgsWithInvalidCount(err, t) } +func TestRangeArgs_WithInvalidCount_WithValid(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + _, err := executeCommand(c, "two") + rangeArgsWithInvalidCount(err, t) +} + +func TestRangeArgs_WithInvalidCount_WithValid_WithInvalidArgs(t *testing.T) { + c := getCommand(RangeArgs(2, 4), true) + _, err := executeCommand(c, "a") + rangeArgsWithInvalidCount(err, t) +} + +func TestRangeArgs_WithInvalidCount_WithValidOnly_WithInvalidArgs(t *testing.T) { + c := getCommand(MatchAll(OnlyValidArgs, RangeArgs(2, 4)), true) + _, err := executeCommand(c, "a") + validOnlyWithInvalidArgs(err, t) +} + +// Takes(No)Args + func TestRootTakesNoArgs(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} childCmd := &Command{Use: "child", Run: emptyRun} @@ -293,6 +471,32 @@ func TestMatchAll(t *testing.T) { } } +// DEPRECATED + +func TestExactValidArgs(t *testing.T) { + c := getCommand(ExactValidArgs(3), true) + output, err := executeCommand(c, "three", "one", "two") + expectSuccess(output, err, t) +} + +func TestExactValidArgs_WithInvalidCount(t *testing.T) { + c := getCommand(ExactValidArgs(2), false) + _, err := executeCommand(c, "three", "one", "two") + exactArgsWithInvalidCount(err, t) +} + +func TestExactValidArgs_WithInvalidCount_WithInvalidArgs(t *testing.T) { + c := getCommand(ExactValidArgs(2), true) + _, err := executeCommand(c, "three", "a", "two") + exactArgsWithInvalidCount(err, t) +} + +func TestExactValidArgs_WithInvalidArgs(t *testing.T) { + c := getCommand(ExactValidArgs(2), true) + _, err := executeCommand(c, "three", "a") + validOnlyWithInvalidArgs(err, t) +} + // This test make sure we keep backwards-compatibility with respect // to the legacyArgs() function. // It makes sure the root command accepts arguments if it does not have diff --git a/user_guide.md b/user_guide.md index 5a7acf8..6128ae4 100644 --- a/user_guide.md +++ b/user_guide.md @@ -302,15 +302,15 @@ rootCmd.MarkPersistentFlagRequired("region") ### Flag Groups -If you have different flags that must be provided together (e.g. if they provide the `--username` flag they MUST provide the `--password` flag as well) then +If you have different flags that must be provided together (e.g. if they provide the `--username` flag they MUST provide the `--password` flag as well) then Cobra can enforce that requirement: ```go rootCmd.Flags().StringVarP(&u, "username", "u", "", "Username (required if password is set)") rootCmd.Flags().StringVarP(&pw, "password", "p", "", "Password (required if username is set)") rootCmd.MarkFlagsRequiredTogether("username", "password") -``` +``` -You can also prevent different flags from being provided together if they represent mutually +You can also prevent different flags from being provided together if they represent mutually exclusive options such as specifying an output format as either `--json` or `--yaml` but never both: ```go rootCmd.Flags().BoolVar(&u, "json", false, "Output in JSON") @@ -327,29 +327,47 @@ In both of these cases: ## Positional and Custom Arguments Validation of positional arguments can be specified using the `Args` field of `Command`. -If `Args` is undefined or `nil`, it defaults to `ArbitraryArgs`. - The following validators are built in: -- `NoArgs` - the command will report an error if there are any positional args. -- `ArbitraryArgs` - the command will accept any args. -- `OnlyValidArgs` - the command will report an error if there are any positional args that are not in the `ValidArgs` field of `Command`. -- `MinimumNArgs(int)` - the command will report an error if there are not at least N positional args. -- `MaximumNArgs(int)` - the command will report an error if there are more than N positional args. -- `ExactArgs(int)` - the command will report an error if there are not exactly N positional args. -- `ExactValidArgs(int)` - the command will report an error if there are not exactly N positional args OR if there are any positional args that are not in the `ValidArgs` field of `Command` -- `RangeArgs(min, max)` - the command will report an error if the number of args is not between the minimum and maximum number of expected args. -- `MatchAll(pargs ...PositionalArgs)` - enables combining existing checks with arbitrary other checks (e.g. you want to check the ExactArgs length along with other qualities). +- Number of arguments: + - `NoArgs` - report an error if there are any positional args. + - `ArbitraryArgs` - accept any number of args. + - `MinimumNArgs(int)` - report an error if less than N positional args are provided. + - `MaximumNArgs(int)` - report an error if more than N positional args are provided. + - `ExactArgs(int)` - report an error if there are not exactly N positional args. + - `RangeArgs(min, max)` - report an error if the number of args is not between `min` and `max`. +- Content of the arguments: + - `OnlyValidArgs` - report an error if there are any positional args not specified in the `ValidArgs` field of `Command`, which can optionally be set to a list of valid values for positional args. -An example of setting the custom validator: +If `Args` is undefined or `nil`, it defaults to `ArbitraryArgs`. + +Moreover, `MatchAll(pargs ...PositionalArgs)` enables combining existing checks with arbitrary other checks. +For instance, if you want to report an error if there are not exactly N positional args OR if there are any positional +args that are not in the `ValidArgs` field of `Command`, you can call `MatchAll` on `ExactArgs` and `OnlyValidArgs`, as +shown below: + +```go +var cmd = &cobra.Command{ + Short: "hello", + Args: MatchAll(ExactArgs(2), OnlyValidArgs), + Run: func(cmd *cobra.Command, args []string) { + fmt.Println("Hello, World!") + }, +} +``` + +It is possible to set any custom validator that satisfies `func(cmd *cobra.Command, args []string) error`. +For example: ```go var cmd = &cobra.Command{ Short: "hello", Args: func(cmd *cobra.Command, args []string) error { - if len(args) < 1 { - return errors.New("requires a color argument") + // Optionally run one of the validators provided by cobra + if err := cobra.MinimumNArgs(1)(cmd, args); err != nil { + return err } + // Run the custom validation logic if myapp.IsValidColor(args[0]) { return nil }