feat: generalize ValidArgs; use it implicitly with any validator

2024-11-24 22:57:12 +00:00 · 2019-03-18 23:32:12 +01:00 · 2019-03-18 23:32:12 +01:00 · 2cd7871821
commit 2cd7871821
parent dbf85f6104
5 changed files with 270 additions and 69 deletions
--- a/args.go
+++ b/args.go
@ -7,6 +7,25 @@ import (

 type PositionalArgs func(cmd *Command, args []string) error

+// validateArgs returns an error if there are any positional args that are not in
+// the `ValidArgs` field of `Command`
+func validateArgs(cmd *Command, args []string) error {
+	if len(cmd.ValidArgs) > 0 {
+		// Remove any description that may be included in ValidArgs.
+		// A description is following a tab character.
+		var validArgs []string
+		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]))
+			}
+		}
+	}
+	return nil
+}
+
 // Legacy arg validation has the following behaviour:
 // - root commands with no subcommands can take arbitrary arguments
 // - root commands with subcommands will do subcommand validity checking
@ -32,25 +51,6 @@ func NoArgs(cmd *Command, args []string) error {
 	return nil
 }

-// OnlyValidArgs returns an error if any args are not in the list of ValidArgs.
-func OnlyValidArgs(cmd *Command, args []string) error {
-	if len(cmd.ValidArgs) > 0 {
-		// Remove any description that may be included in ValidArgs.
-		// A description is following a tab character.
-		var validArgs []string
-		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]))
-			}
-		}
-	}
-	return nil
-}
-
 // ArbitraryArgs never returns an error.
 func ArbitraryArgs(cmd *Command, args []string) error {
 	return nil
@ -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,18 @@ 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: now `ExactArgs` honors `ValidArgs`, when defined and not empty
+func ExactValidArgs(n int) PositionalArgs {
+	return ExactArgs(n)
+}
+
+// OnlyValidArgs returns an error if any args are not in the list of `ValidArgs`.
+//
+// Deprecated: now `ArbitraryArgs` honors `ValidArgs`, when defined and not empty
+func OnlyValidArgs(cmd *Command, args []string) error {
+	return ArbitraryArgs(cmd, args)
+}
--- a/args_test.go
+++ b/args_test.go
@ -31,6 +31,7 @@ func validWithInvalidArgs(err error, t *testing.T) {
 	if err == nil {
 		t.Fatal("Expected an error")
 	}
+
 	got := err.Error()
 	expected := `invalid argument "a" for "c"`
 	if got != expected {
@ -43,7 +44,7 @@ func noArgsWithArgs(err error, t *testing.T) {
 		t.Fatal("Expected an error")
 	}
 	got := err.Error()
-	expected := `unknown command "illegal" for "c"`
+	expected := `unknown command "one" for "c"`
 	if got != expected {
 		t.Errorf("Expected: %q, got: %q", expected, got)
 	}
@ -64,6 +65,7 @@ func maximumNArgsWithMoreArgs(err error, t *testing.T) {
 	if err == nil {
 		t.Fatal("Expected an error")
 	}
+
 	got := err.Error()
 	expected := "accepts at most 2 arg(s), received 3"
 	if got != expected {
@ -93,6 +95,8 @@ func rangeArgsWithInvalidCount(err error, t *testing.T) {
 	}
 }

+// NoArgs
+
 func TestNoArgs(t *testing.T) {
 	c := getCommand(NoArgs, false)
 	output, err := executeCommand(c)
@ -101,21 +105,17 @@ func TestNoArgs(t *testing.T) {

 func TestNoArgsWithArgs(t *testing.T) {
 	c := getCommand(NoArgs, false)
-	_, err := executeCommand(c, "illegal")
+	_, err := executeCommand(c, "one")
 	noArgsWithArgs(err, t)
 }

-func TestOnlyValidArgs(t *testing.T) {
-	c := getCommand(OnlyValidArgs, true)
-	output, err := executeCommand(c, "one", "two")
-	expectSuccess(output, err, t)
+func TestNoArgsWithArgsWithValid(t *testing.T) {
+	c := getCommand(NoArgs, true)
+	_, err := executeCommand(c, "one")
+	noArgsWithArgs(err, t)
 }

-func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
-	c := getCommand(OnlyValidArgs, true)
-	_, err := executeCommand(c, "a")
-	validWithInvalidArgs(err, t)
-}
+// ArbitraryArgs

 func TestArbitraryArgs(t *testing.T) {
 	c := getCommand(ArbitraryArgs, false)
@ -123,72 +123,172 @@ func TestArbitraryArgs(t *testing.T) {
 	expectSuccess(output, err, t)
 }

+func TestArbitraryArgsWithValid(t *testing.T) {
+	c := getCommand(ArbitraryArgs, true)
+	output, err := executeCommand(c, "one", "two")
+	expectSuccess(output, err, t)
+}
+
+func TestArbitraryArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(ArbitraryArgs, true)
+	_, err := executeCommand(c, "a")
+	validWithInvalidArgs(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 TestMinimumNArgsWithValid(t *testing.T) {
+	c := getCommand(MinimumNArgs(2), true)
+	output, err := executeCommand(c, "one", "three")
+	expectSuccess(output, err, t)
+}
+
+func TestMinimumNArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(MinimumNArgs(2), true)
+	_, err := executeCommand(c, "a", "b")
+	validWithInvalidArgs(err, t)
+}
+
 func TestMinimumNArgsWithLessArgs(t *testing.T) {
 	c := getCommand(MinimumNArgs(2), false)
 	_, err := executeCommand(c, "a")
 	minimumNArgsWithLessArgs(err, t)
 }

+func TestMinimumNArgsWithLessArgsWithValid(t *testing.T) {
+	c := getCommand(MinimumNArgs(2), true)
+	_, err := executeCommand(c, "one")
+	minimumNArgsWithLessArgs(err, t)
+}
+
+func TestMinimumNArgsWithLessArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(MinimumNArgs(2), true)
+	_, err := executeCommand(c, "a")
+	validWithInvalidArgs(err, t)
+}
+
+// MaximumNArgs
+
 func TestMaximumNArgs(t *testing.T) {
 	c := getCommand(MaximumNArgs(3), false)
 	output, err := executeCommand(c, "a", "b")
 	expectSuccess(output, err, t)
 }

+func TestMaximumNArgsWithValid(t *testing.T) {
+	c := getCommand(MaximumNArgs(2), true)
+	output, err := executeCommand(c, "one", "three")
+	expectSuccess(output, err, t)
+}
+
+func TestMaximumNArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(MaximumNArgs(2), true)
+	_, err := executeCommand(c, "a", "b")
+	validWithInvalidArgs(err, t)
+}
+
 func TestMaximumNArgsWithMoreArgs(t *testing.T) {
 	c := getCommand(MaximumNArgs(2), false)
 	_, err := executeCommand(c, "a", "b", "c")
 	maximumNArgsWithMoreArgs(err, t)
 }

+func TestMaximumNArgsWithMoreArgsWithValid(t *testing.T) {
+	c := getCommand(MaximumNArgs(2), true)
+	_, err := executeCommand(c, "one", "three", "two")
+	maximumNArgsWithMoreArgs(err, t)
+}
+
+func TestMaximumNArgsWithMoreArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(MaximumNArgs(2), true)
+	_, err := executeCommand(c, "a", "b", "c")
+	validWithInvalidArgs(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 TestExactArgsWithValid(t *testing.T) {
+	c := getCommand(ExactArgs(3), true)
+	output, err := executeCommand(c, "three", "one", "two")
+	expectSuccess(output, err, t)
+}
+
+func TestExactArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(ExactArgs(3), true)
+	_, err := executeCommand(c, "three", "a", "two")
+	validWithInvalidArgs(err, t)
+}
+
 func TestExactArgsWithInvalidCount(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 TestExactArgsWithInvalidCountWithValid(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 TestExactArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(ExactArgs(2), true)
 	_, err := executeCommand(c, "three", "a", "two")
 	validWithInvalidArgs(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 TestRangeArgsWithValid(t *testing.T) {
+	c := getCommand(RangeArgs(2, 4), true)
+	output, err := executeCommand(c, "three", "one", "two")
+	expectSuccess(output, err, t)
+}
+
+func TestRangeArgsWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(RangeArgs(2, 4), true)
+	_, err := executeCommand(c, "three", "a", "two")
+	validWithInvalidArgs(err, t)
+}
+
 func TestRangeArgsWithInvalidCount(t *testing.T) {
 	c := getCommand(RangeArgs(2, 4), false)
 	_, err := executeCommand(c, "a")
 	rangeArgsWithInvalidCount(err, t)
 }

+func TestRangeArgsWithInvalidCountWithValid(t *testing.T) {
+	c := getCommand(RangeArgs(2, 4), true)
+	_, err := executeCommand(c, "two")
+	rangeArgsWithInvalidCount(err, t)
+}
+
+func TestRangeArgsWithInvalidCountWithValidWithInvalidArgs(t *testing.T) {
+	c := getCommand(RangeArgs(2, 4), true)
+	_, err := executeCommand(c, "a")
+	validWithInvalidArgs(err, t)
+}
+
+// Takes(No)Args
+
 func TestRootTakesNoArgs(t *testing.T) {
 	rootCmd := &Command{Use: "root", Run: emptyRun}
 	childCmd := &Command{Use: "child", Run: emptyRun}
@ -293,6 +393,91 @@ func TestMatchAll(t *testing.T) {
 	}
 }

+// DEPRECATED
+
+func TestOnlyValidArgs(t *testing.T) {
+	c := &Command{
+		Use:       "c",
+		Args:      OnlyValidArgs,
+		ValidArgs: []string{"one", "two"},
+		Run:       emptyRun,
+	}
+
+	output, err := executeCommand(c, "one", "two")
+	if output != "" {
+		t.Errorf("Unexpected output: %v", output)
+	}
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestOnlyValidArgsWithInvalidArgs(t *testing.T) {
+	c := &Command{
+		Use:       "c",
+		Args:      OnlyValidArgs,
+		ValidArgs: []string{"one", "two"},
+		Run:       emptyRun,
+	}
+
+	_, err := executeCommand(c, "three")
+	if err == nil {
+		t.Fatal("Expected an error")
+	}
+
+	got := err.Error()
+	expected := `invalid argument "three" for "c"`
+	if got != expected {
+		t.Errorf("Expected: %q, got: %q", expected, got)
+	}
+}
+
+func TestExactValidArgs(t *testing.T) {
+	c := &Command{Use: "c", Args: ExactValidArgs(3), ValidArgs: []string{"a", "b", "c"}, Run: emptyRun}
+	output, err := executeCommand(c, "a", "b", "c")
+	if output != "" {
+		t.Errorf("Unexpected output: %v", output)
+	}
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+}
+
+func TestExactValidArgsWithInvalidCount(t *testing.T) {
+	c := &Command{Use: "c", Args: ExactValidArgs(2), Run: emptyRun}
+	_, err := executeCommand(c, "a", "b", "c")
+
+	if err == nil {
+		t.Fatal("Expected an error")
+	}
+
+	got := err.Error()
+	expected := "accepts 2 arg(s), received 3"
+	if got != expected {
+		t.Fatalf("Expected %q, got %q", expected, got)
+	}
+}
+
+func TestExactValidArgsWithInvalidArgs(t *testing.T) {
+	c := &Command{
+		Use:       "c",
+		Args:      ExactValidArgs(1),
+		ValidArgs: []string{"one", "two"},
+		Run:       emptyRun,
+	}
+
+	_, err := executeCommand(c, "three")
+	if err == nil {
+		t.Fatal("Expected an error")
+	}
+
+	got := err.Error()
+	expected := `invalid argument "three" for "c"`
+	if got != expected {
+		t.Errorf("Expected: %q, got: %q", expected, got)
+	}
+}
+
 // 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
--- a/bash_completions_test.go
+++ b/bash_completions_test.go
@ -139,7 +139,7 @@ func TestBashCompletions(t *testing.T) {
 	timesCmd := &Command{
 		Use:        "times [# times] [string to echo]",
 		SuggestFor: []string{"counts"},
-		Args:       OnlyValidArgs,
+		Args:       ArbitraryArgs,
 		ValidArgs:  []string{"one", "two", "three", "four"},
 		Short:      "Echo anything to the screen more times",
 		Long:       "a slightly useless command for testing.",
--- a/command.go
+++ b/command.go
@ -1011,10 +1011,15 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 	return cmd, err
 }

+// ValidateArgs returns an error if any positional args are not in
+// the `ValidArgs` field of `Command`
 func (c *Command) ValidateArgs(args []string) error {
 	if c.Args == nil {
 		return ArbitraryArgs(c, args)
 	}
+	if err := validateArgs(c, args); err != nil {
+		return err
+	}
 	return c.Args(c, args)
 }

--- 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,37 @@ 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.
+- `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`.
 - `MatchAll(pargs ...PositionalArgs)` - enables combining existing checks with arbitrary other checks (e.g. you want to check the ExactArgs length along with other qualities).

-An example of setting the custom validator:
+If `Args` is undefined or `nil`, it defaults to `ArbitraryArgs`.
+
+Field `ValidArgs` of type `[]string` can be defined in `Command`, in order to report an error if there are any
+positional args that are not in the list.
+This validation is executed implicitly before the validator defined in `Args`.
+
+> NOTE: `OnlyValidArgs` and `ExactValidArgs(int)` are now deprecated.
+> `ArbitraryArgs` and `ExactArgs(int)` provide the same functionality now.
+
+Moreover, 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
    }