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

@@ -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)
+}

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

user_guide.md

@@ -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
     }