diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 01396e639..f94dd2c5f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3,6 +3,7 @@ name: Test on: push: pull_request: + workflow_dispatch: env: GO111MODULE: on diff --git a/args.go b/args.go index 20a022b30..aad963bd1 100644 --- 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) +} diff --git a/args_test.go b/args_test.go index ded3d0d59..d97cf18cd 100644 --- a/args_test.go +++ b/args_test.go @@ -6,188 +6,136 @@ import ( "testing" ) -func getCommand(args PositionalArgs, withValid bool) *Command { - c := &Command{ - Use: "c", - Args: args, - Run: emptyRun, - } - if withValid { - c.ValidArgs = []string{"one", "two", "three"} - } - return c -} - -func expectSuccess(output string, err error, t *testing.T) { - if output != "" { - t.Errorf("Unexpected output: %v", output) - } - if err != nil { - t.Fatalf("Unexpected error: %v", err) - } -} - -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 { - t.Errorf("Expected: %q, got: %q", expected, got) - } -} - -func noArgsWithArgs(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := `unknown command "illegal" for "c"` - if got != expected { - t.Errorf("Expected: %q, got: %q", expected, got) - } +type argsTestcase struct { + exerr string // Expected error key (see map[string][string]) + args PositionalArgs // Args validator + wValid bool // Define `ValidArgs` in the command + rargs []string // Runtime args } -func minimumNArgsWithLessArgs(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") - } - got := err.Error() - expected := "requires at least 2 arg(s), only received 1" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - -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 { - t.Fatalf("Expected %q, got %q", expected, got) - } -} - -func exactArgsWithInvalidCount(err error, t *testing.T) { - 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) - } +var errStrings = map[string]string{ + "invalid": `invalid argument "a" for "c"`, + "unknown": `unknown command "one" for "c"`, + "less": "requires at least 2 arg(s), only received 1", + "more": "accepts at most 2 arg(s), received 3", + "notexact": "accepts 2 arg(s), received 3", + "notinrange": "accepts between 2 and 4 arg(s), received 1", } -func rangeArgsWithInvalidCount(err error, t *testing.T) { - if err == nil { - t.Fatal("Expected an error") +func (tc *argsTestcase) test(t *testing.T) { + c := &Command{ + Use: "c", + Args: tc.args, + Run: emptyRun, } - got := err.Error() - expected := "accepts between 2 and 4 arg(s), received 1" - if got != expected { - t.Fatalf("Expected %q, got %q", expected, got) + if tc.wValid { + c.ValidArgs = []string{"one", "two", "three"} } -} - -func TestNoArgs(t *testing.T) { - c := getCommand(NoArgs, false) - output, err := executeCommand(c) - expectSuccess(output, err, t) -} - -func TestNoArgsWithArgs(t *testing.T) { - c := getCommand(NoArgs, false) - _, err := executeCommand(c, "illegal") - noArgsWithArgs(err, t) -} - -func TestOnlyValidArgs(t *testing.T) { - c := getCommand(OnlyValidArgs, true) - output, err := executeCommand(c, "one", "two") - expectSuccess(output, err, t) -} - -func TestOnlyValidArgsWithInvalidArgs(t *testing.T) { - c := getCommand(OnlyValidArgs, true) - _, err := executeCommand(c, "a") - validWithInvalidArgs(err, t) -} - -func TestArbitraryArgs(t *testing.T) { - c := getCommand(ArbitraryArgs, false) - output, err := executeCommand(c, "a", "b") - expectSuccess(output, err, t) -} -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) { - c := getCommand(MinimumNArgs(2), false) - _, err := executeCommand(c, "a") - minimumNArgsWithLessArgs(err, t) -} - -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) { - c := getCommand(MaximumNArgs(2), false) - _, err := executeCommand(c, "a", "b", "c") - maximumNArgsWithMoreArgs(err, t) -} - -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) { - 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) - _, err := executeCommand(c, "three", "one", "two") - exactArgsWithInvalidCount(err, t) -} - -func TestExactValidArgsWithInvalidArgs(t *testing.T) { - c := getCommand(ExactValidArgs(3), true) - _, err := executeCommand(c, "three", "a", "two") - validWithInvalidArgs(err, t) -} - -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) { - c := getCommand(RangeArgs(2, 4), false) - _, err := executeCommand(c, "a") - rangeArgsWithInvalidCount(err, t) -} + o, e := executeCommand(c, tc.rargs...) + + if len(tc.exerr) > 0 { + // Expect error + if e == nil { + t.Fatal("Expected an error") + } + expected, ok := errStrings[tc.exerr] + if !ok { + t.Errorf(`key "%s" is not found in map "errStrings"`, tc.exerr) + return + } + if got := e.Error(); got != expected { + t.Errorf("Expected: %q, got: %q", expected, got) + } + } else { + // Expect success + if o != "" { + t.Errorf("Unexpected output: %v", o) + } + if e != nil { + t.Fatalf("Unexpected error: %v", e) + } + } +} + +func testArgs(t *testing.T, tests map[string]argsTestcase) { + for name, tc := range tests { + t.Run(name, tc.test) + } +} + +func TestArgs_No(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | ": {"", NoArgs, false, []string{}}, + " | Arb": {"unknown", NoArgs, false, []string{"one"}}, + "Valid | Valid": {"unknown", NoArgs, true, []string{"one"}}, + }) +} +func TestArgs_Nil(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", nil, false, []string{"a", "b"}}, + "Valid | Valid": {"", nil, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", nil, true, []string{"a"}}, + }) +} +func TestArgs_Arbitrary(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", ArbitraryArgs, false, []string{"a", "b"}}, + "Valid | Valid": {"", ArbitraryArgs, true, []string{"one", "two"}}, + "Valid | Invalid": {"invalid", ArbitraryArgs, true, []string{"a"}}, + }) +} +func TestArgs_MinimumN(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", MinimumNArgs(2), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", MinimumNArgs(2), true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MinimumNArgs(2), true, []string{"a", "b"}}, + " | Less": {"less", MinimumNArgs(2), false, []string{"a"}}, + "Valid | Less": {"less", MinimumNArgs(2), true, []string{"one"}}, + "Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, []string{"a"}}, + }) +} +func TestArgs_MaximumN(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", MaximumNArgs(3), false, []string{"a", "b"}}, + "Valid | Valid": {"", MaximumNArgs(2), true, []string{"one", "three"}}, + "Valid | Invalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b"}}, + " | More": {"more", MaximumNArgs(2), false, []string{"a", "b", "c"}}, + "Valid | More": {"more", MaximumNArgs(2), true, []string{"one", "three", "two"}}, + "Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, []string{"a", "b", "c"}}, + }) +} +func TestArgs_Exact(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", ExactArgs(3), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", ExactArgs(3), true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", ExactArgs(3), true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notexact", ExactArgs(2), false, []string{"a", "b", "c"}}, + "Valid | InvalidCount": {"notexact", ExactArgs(2), true, []string{"three", "one", "two"}}, + "Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, []string{"three", "a", "two"}}, + }) +} +func TestArgs_Range(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + " | Arb": {"", RangeArgs(2, 4), false, []string{"a", "b", "c"}}, + "Valid | Valid": {"", RangeArgs(2, 4), true, []string{"three", "one", "two"}}, + "Valid | Invalid": {"invalid", RangeArgs(2, 4), true, []string{"three", "a", "two"}}, + " | InvalidCount": {"notinrange", RangeArgs(2, 4), false, []string{"a"}}, + "Valid | InvalidCount": {"notinrange", RangeArgs(2, 4), true, []string{"two"}}, + "Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, []string{"a"}}, + }) +} +func TestArgs_DEPRECATED(t *testing.T) { + testArgs(t, map[string]argsTestcase{ + "OnlyValid | Valid | Valid": {"", OnlyValidArgs, true, []string{"one", "two"}}, + "OnlyValid | Valid | Invalid": {"invalid", OnlyValidArgs, true, []string{"a"}}, + "ExactValid | Valid | Valid": {"", ExactValidArgs(3), true, []string{"two", "three", "one"}}, + "ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, []string{"two", "three", "one"}}, + "ExactValid | Valid | Invalid": {"invalid", ExactValidArgs(2), true, []string{"two", "a"}}, + }) +} + +// Takes(No)Args func TestRootTakesNoArgs(t *testing.T) { rootCmd := &Command{Use: "root", Run: emptyRun} diff --git a/bash_completions_test.go b/bash_completions_test.go index 21d1fe0eb..46e8e674c 100644 --- 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.", diff --git a/command.go b/command.go index 2cc18891d..dce77608d 100644 --- a/command.go +++ b/command.go @@ -930,7 +930,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { // initialize help at the last point to allow for user overriding c.InitDefaultHelpCmd() // initialize completion at the last point to allow for user overriding - c.initDefaultCompletionCmd() + c.InitDefaultCompletionCmd() args := c.args @@ -995,7 +995,13 @@ 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`. Then, run the `Args` validator, if +// specified. func (c *Command) ValidateArgs(args []string) error { + if err := validateArgs(c, args); err != nil { + return err + } if c.Args == nil { return nil } diff --git a/completions.go b/completions.go index ac25a9d03..3dc0782ea 100644 --- a/completions.go +++ b/completions.go @@ -580,12 +580,12 @@ func checkIfFlagCompletion(finalCmd *Command, args []string, lastArg string) (*p return flag, trimmedArgs, lastArg, nil } -// initDefaultCompletionCmd adds a default 'completion' command to c. +// InitDefaultCompletionCmd adds a default 'completion' command to c. // This function will do nothing if any of the following is true: // 1- the feature has been explicitly disabled by the program, // 2- c has no subcommands (to avoid creating one), // 3- c already has a 'completion' command provided by the program. -func (c *Command) initDefaultCompletionCmd() { +func (c *Command) InitDefaultCompletionCmd() { if c.CompletionOptions.DisableDefaultCmd || !c.HasSubCommands() { return } diff --git a/doc/man_docs.go b/doc/man_docs.go index 916e36144..0eeaf4cf6 100644 --- a/doc/man_docs.go +++ b/doc/man_docs.go @@ -201,6 +201,7 @@ func manPrintOptions(buf io.StringWriter, command *cobra.Command) { func genMan(cmd *cobra.Command, header *GenManHeader) []byte { cmd.InitDefaultHelpCmd() cmd.InitDefaultHelpFlag() + cmd.InitDefaultCompletionCmd() // something like `rootcmd-subcmd1-subcmd2` dashCommandName := strings.Replace(cmd.CommandPath(), " ", "-", -1) diff --git a/doc/man_docs_test.go b/doc/man_docs_test.go index f3fe3d613..c4258026e 100644 --- a/doc/man_docs_test.go +++ b/doc/man_docs_test.go @@ -131,7 +131,7 @@ func TestGenManSeeAlso(t *testing.T) { if err := assertNextLineEquals(scanner, ".PP"); err != nil { t.Fatalf("First line after SEE ALSO wasn't break-indent: %v", err) } - if err := assertNextLineEquals(scanner, `\fBroot-bbb(1)\fP, \fBroot-ccc(1)\fP`); err != nil { + if err := assertNextLineEquals(scanner, `\fBroot-bbb(1)\fP, \fBroot-ccc(1)\fP, \fBroot-completion(1)\fP`); err != nil { t.Fatalf("Second line after SEE ALSO wasn't correct: %v", err) } } diff --git a/doc/md_docs.go b/doc/md_docs.go index 5181af8dc..6f89fe287 100644 --- a/doc/md_docs.go +++ b/doc/md_docs.go @@ -54,6 +54,7 @@ func GenMarkdown(cmd *cobra.Command, w io.Writer) error { func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) string) error { cmd.InitDefaultHelpCmd() cmd.InitDefaultHelpFlag() + cmd.InitDefaultCompletionCmd() buf := new(bytes.Buffer) name := cmd.CommandPath() diff --git a/doc/rest_docs.go b/doc/rest_docs.go index 051d8dc83..77f49ec7a 100644 --- a/doc/rest_docs.go +++ b/doc/rest_docs.go @@ -61,6 +61,7 @@ func GenReST(cmd *cobra.Command, w io.Writer) error { func GenReSTCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string, string) string) error { cmd.InitDefaultHelpCmd() cmd.InitDefaultHelpFlag() + cmd.InitDefaultCompletionCmd() buf := new(bytes.Buffer) name := cmd.CommandPath() diff --git a/doc/yaml_docs.go b/doc/yaml_docs.go index 96e6ad721..1b668eeda 100644 --- a/doc/yaml_docs.go +++ b/doc/yaml_docs.go @@ -92,6 +92,7 @@ func GenYaml(cmd *cobra.Command, w io.Writer) error { func GenYamlCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) string) error { cmd.InitDefaultHelpCmd() cmd.InitDefaultHelpFlag() + cmd.InitDefaultCompletionCmd() yamlDoc := cmdDoc{} yamlDoc.Name = cmd.CommandPath() diff --git a/user_guide.md b/user_guide.md index e87cdf218..9d28f2c72 100644 --- a/user_guide.md +++ b/user_guide.md @@ -302,30 +302,38 @@ rootCmd.MarkPersistentFlagRequired("region") ## Positional and Custom Arguments -Validation of positional arguments can be specified using the `Args` field -of `Command`. - +Validation of positional arguments can be specified using the `Args` field of `Command`. 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 }