Merge 8825c7b3f9 into 2e8ba6f308

args.go

@@ -7,33 +7,9 @@ import (
 
 type PositionalArgs func(cmd *Command, args []string) error
 
-// 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
-// - subcommands will always accept arbitrary arguments
-func legacyArgs(cmd *Command, args []string) error {
-	// no subcommand, always take args
-	if !cmd.HasSubCommands() {
-		return nil
-	}
-
-	// root command with subcommands, do subcommand checking.
-	if !cmd.HasParent() && len(args) > 0 {
-		return fmt.Errorf("unknown command %q for %q%s", args[0], cmd.CommandPath(), cmd.findSuggestions(args[0]))
-	}
-	return nil
-}
-
-// NoArgs returns an error if any args are included.
-func NoArgs(cmd *Command, args []string) error {
-	if len(args) > 0 {
-		return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())
-	}
-	return nil
-}
-
-// OnlyValidArgs returns an error if any args are not in the list of ValidArgs.
-func OnlyValidArgs(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.
@@ -41,7 +17,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]))
@@ -51,6 +26,17 @@ func OnlyValidArgs(cmd *Command, args []string) error {
 	return nil
 }
 
+// NoArgs returns an error if any args are included.
+func NoArgs(cmd *Command, args []string) error {
+	if len(args) > 0 {
+		if cmd.HasAvailableSubCommands() {
+			return fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())
+		}
+		return fmt.Errorf("\"%s\" rejected; %q does not accept args", args[0], cmd.CommandPath())
+	}
+	return nil
+}
+
 // ArbitraryArgs never returns an error.
 func ArbitraryArgs(cmd *Command, args []string) error {
 	return nil
@@ -86,18 +72,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 +93,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)
+}

args_test.go

@@ -1,193 +1,194 @@
 package cobra
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"testing"
 )
 
-func getCommand(args PositionalArgs, withValid bool) *Command {
-	c := &Command{
-		Use:  "c",
-		Args: args,
-		Run:  emptyRun,
+func executeUsage(c *Command) (string, error) {
+	buf := new(bytes.Buffer)
+	c.SetOutput(buf)
+	err := c.Usage()
+	return buf.String(), err
+}
+
+func checkOutput(o string, t *testing.T, i string) {
+	str := map[rune]string{
+		'u': "Usage:",
+		'h': "Run 'c --help' for usage",
+		'c': "c [command]",
+		'v': "Valid Args:",
+		'a': "c [flags] [args]",
+		'f': "c [flags]",
 	}
-	if withValid {
+	for _, x := range "uhcva" {
+		b := strings.Contains(i, string(x))
+		if s := str[x]; b != strings.Contains(o, s) {
+			m := "Did not expect"
+			if b {
+				m = "Expected"
+			}
+			t.Errorf("%s to find '%s' in the output", m, s)
+			continue
+		}
+		if (x == 'a') && b {
+			return
+		}
+	}
+}
+
+func expectErrorAndCheckOutput(t *testing.T, err error, err_k, o, i string) {
+	//	expectError(err, t, err_k)
+	//	checkOutput(o, t, i)
+}
+
+type argsTestcase struct {
+	exerr        string         // Expected error key (see map[string][string])
+	args         PositionalArgs // Args validator
+	wValid, wRun bool           // Define `ValidArgs` in the command
+	rargs        []string       // Runtime args
+}
+
+var errStrings = map[string]string{
+	"run":        `command "c" is not runnable`,
+	"runsub":     `command "c" is not runnable; please provide a subcmd`,
+	"no":         `"one" rejected; "c" does not accept args`,
+	"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 newCmd(args PositionalArgs, wValid, wRun bool) *Command {
+	c := &Command{
+		Use:   "c",
+		Short: "A generator",
+		Long:  `Cobra is a CLI ...`,
+		//Run:   emptyRun,
+	}
+	if args != nil {
+		c.Args = args
+	}
+	if wValid {
 		c.ValidArgs = []string{"one", "two", "three"}
 	}
+	if wRun {
+		c.Run = func(cmd *Command, args []string) {
+			//fmt.Println("RUN", args)
+		}
+	}
 	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 (tc *argsTestcase) test(t *testing.T) {
+	o, e := executeCommand(newCmd(tc.args, tc.wValid, tc.wRun), 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 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 testArgs(t *testing.T, tests map[string]argsTestcase) {
+	for name, tc := range tests {
+		t.Run(name, tc.test)
 	}
 }
 
-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)
-	}
+func TestArgs_No(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | ":      {"", NoArgs, false, true, []string{}},
+		"      | Arb":   {"no", NoArgs, false, true, []string{"one"}},
+		"Valid | Valid": {"no", NoArgs, true, true, []string{"one"}},
+	})
+}
+func TestArgs_Nil(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":     {"", nil, false, true, []string{"a", "b"}},
+		"Valid | Valid":   {"", nil, true, true, []string{"one", "two"}},
+		"Valid | Invalid": {"invalid", nil, true, true, []string{"a"}},
+	})
+}
+func TestArgs_Arbitrary(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":     {"", ArbitraryArgs, false, true, []string{"a", "b"}},
+		"Valid | Valid":   {"", ArbitraryArgs, true, true, []string{"one", "two"}},
+		"Valid | Invalid": {"invalid", ArbitraryArgs, true, true, []string{"a"}},
+	})
+}
+func TestArgs_MinimumN(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":         {"", MinimumNArgs(2), false, true, []string{"a", "b", "c"}},
+		"Valid | Valid":       {"", MinimumNArgs(2), true, true, []string{"one", "three"}},
+		"Valid | Invalid":     {"invalid", MinimumNArgs(2), true, true, []string{"a", "b"}},
+		"      | Less":        {"less", MinimumNArgs(2), false, true, []string{"a"}},
+		"Valid | Less":        {"less", MinimumNArgs(2), true, true, []string{"one"}},
+		"Valid | LessInvalid": {"invalid", MinimumNArgs(2), true, true, []string{"a"}},
+	})
+}
+func TestArgs_MaximumN(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":         {"", MaximumNArgs(3), false, true, []string{"a", "b"}},
+		"Valid | Valid":       {"", MaximumNArgs(2), true, true, []string{"one", "three"}},
+		"Valid | Invalid":     {"invalid", MaximumNArgs(2), true, true, []string{"a", "b"}},
+		"      | More":        {"more", MaximumNArgs(2), false, true, []string{"a", "b", "c"}},
+		"Valid | More":        {"more", MaximumNArgs(2), true, true, []string{"one", "three", "two"}},
+		"Valid | MoreInvalid": {"invalid", MaximumNArgs(2), true, true, []string{"a", "b", "c"}},
+	})
+}
+func TestArgs_Exact(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":                 {"", ExactArgs(3), false, true, []string{"a", "b", "c"}},
+		"Valid | Valid":               {"", ExactArgs(3), true, true, []string{"three", "one", "two"}},
+		"Valid | Invalid":             {"invalid", ExactArgs(3), true, true, []string{"three", "a", "two"}},
+		"      | InvalidCount":        {"notexact", ExactArgs(2), false, true, []string{"a", "b", "c"}},
+		"Valid | InvalidCount":        {"notexact", ExactArgs(2), true, true, []string{"three", "one", "two"}},
+		"Valid | InvalidCountInvalid": {"invalid", ExactArgs(2), true, true, []string{"three", "a", "two"}},
+	})
+}
+func TestArgs_Range(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"      | Arb":                 {"", RangeArgs(2, 4), false, true, []string{"a", "b", "c"}},
+		"Valid | Valid":               {"", RangeArgs(2, 4), true, true, []string{"three", "one", "two"}},
+		"Valid | Invalid":             {"invalid", RangeArgs(2, 4), true, true, []string{"three", "a", "two"}},
+		"      | InvalidCount":        {"notinrange", RangeArgs(2, 4), false, true, []string{"a"}},
+		"Valid | InvalidCount":        {"notinrange", RangeArgs(2, 4), true, true, []string{"two"}},
+		"Valid | InvalidCountInvalid": {"invalid", RangeArgs(2, 4), true, true, []string{"a"}},
+	})
+}
+func TestArgs_DEPRECATED(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"OnlyValid  | Valid | Valid":        {"", OnlyValidArgs, true, true, []string{"one", "two"}},
+		"OnlyValid  | Valid | Invalid":      {"invalid", OnlyValidArgs, true, true, []string{"a"}},
+		"ExactValid | Valid | Valid":        {"", ExactValidArgs(3), true, true, []string{"two", "three", "one"}},
+		"ExactValid | Valid | InvalidCount": {"notexact", ExactValidArgs(2), true, true, []string{"two", "three", "one"}},
+		"ExactValid | Valid | Invalid":      {"invalid", ExactValidArgs(2), true, true, []string{"two", "a"}},
+	})
 }
 
-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)
-	}
-}
-
-func rangeArgsWithInvalidCount(err error, t *testing.T) {
-	if err == nil {
-		t.Fatal("Expected an error")
-	}
-	got := err.Error()
-	expected := "accepts between 2 and 4 arg(s), received 1"
-	if got != expected {
-		t.Fatalf("Expected %q, got %q", expected, got)
-	}
-}
-
-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)
-}
+// Takes(No)Args
 
 func TestRootTakesNoArgs(t *testing.T) {
 	rootCmd := &Command{Use: "root", Run: emptyRun}
@@ -228,7 +229,7 @@ func TestChildTakesNoArgs(t *testing.T) {
 	}
 
 	got := err.Error()
-	expected := `unknown command "illegal" for "root child"`
+	expected := `"illegal" rejected; "root child" does not accept args`
 	if !strings.Contains(got, expected) {
 		t.Errorf("expected %q, got %q", expected, got)
 	}
@@ -245,6 +246,280 @@ func TestChildTakesArgs(t *testing.T) {
 	}
 }
 
+// NOTE 'c [command]' is not shown because this command does not have any subcommand
+// NOTE 'Valid Args:' is not shown because this command is not runnable
+// NOTE 'c [flags]' is not shown because this command is not runnable
+func noRunChecks(t *testing.T, err error, err_k, o string) {
+	expectErrorAndCheckOutput(t, err, err_k, o, "u")
+}
+
+// NoRun (no children)
+
+func TestArgs_NoRun(t *testing.T) {
+	tc := argsTestcase{"run", nil, false, false, []string{}}
+	t.Run("|", tc.test)
+	//	noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_ArbValid(t *testing.T) {
+	tc := argsTestcase{"run", nil, false, false, []string{"one", "three"}}
+	t.Run("|", tc.test)
+	//	noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_Invalid(t *testing.T) {
+	tc := argsTestcase{"run", nil, false, false, []string{"two", "a"}}
+	t.Run("|", tc.test)
+	//noRunChecks(t, e, "run", o)
+}
+
+// NoRun (with children)
+// NOTE 'Valid Args:' is not shown because this command is not runnable
+// NOTE 'c [flags]' is not shown because this command is not runnable
+
+func TestArgs_NoRun_wChild(t *testing.T) {
+	c := newCmd(nil, false, false)
+	d := newCmd(nil, false, true)
+	c.AddCommand(d)
+	o, e := executeCommand(c)
+	expectErrorAndCheckOutput(t, e, "runsub", o, "uc")
+}
+
+func TestArgs_NoRun_wChild_ArbValid(t *testing.T) {
+	c := newCmd(nil, false, false)
+	d := newCmd(nil, false, true)
+	c.AddCommand(d)
+	o, e := executeCommand(c, "one", "three")
+	expectErrorAndCheckOutput(t, e, "runsub", o, "h")
+}
+
+func TestArgs_NoRun_wChild_Invalid(t *testing.T) {
+	c := newCmd(nil, false, false)
+	d := newCmd(nil, false, true)
+	c.AddCommand(d)
+	o, e := executeCommand(c, "one", "a")
+	expectErrorAndCheckOutput(t, e, "runsub", o, "h")
+}
+
+// NoRun Args
+
+func TestArgs_NoRun_wArgs(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, false, false, []string{}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wArgs_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, false, false, []string{"one", "three"}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wArgs_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, false, false, []string{"two", "a"}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+// NoRun ValidArgs
+
+func TestArgs_NoRun_wValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", nil, true, false, []string{}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wValid_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", nil, true, false, []string{"one", "three"}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wValid_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", nil, true, false, []string{"two", "a"}},
+	})
+	//noRunChecks(t, e, "run", o)
+}
+
+// NoRun Args ValidArgs
+
+func TestArgs_NoRun_wArgswValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, true, false, []string{}},
+	})
+	//	noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wArgswValid_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, true, false, []string{"one", "three"}},
+	})
+	//	noRunChecks(t, e, "run", o)
+}
+
+func TestArgs_NoRun_wArgswValid_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"run", ArbitraryArgs, true, false, []string{"two", "a"}},
+	})
+	//	noRunChecks(t, e, "run", o)
+}
+
+// Run (no children)
+// NOTE 'c [command]' is not shown because this command does not have any subcommand
+// NOTE 'Valid Args:' is not shown because ValidArgs is not defined
+
+func TestArgs_Run(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", nil, false, true, []string{}},
+	})
+	//o, e = executeUsage(c)
+	//checkOutput(o, t, "ua")
+}
+
+func TestArgs_Run_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", nil, false, true, []string{"one", "three"}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "ua")
+}
+
+func TestArgs_Run_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", nil, false, true, []string{"two", "a"}},
+	})
+	//o, e = executeUsage(c)
+	//checkOutput(o, t, "ua")
+}
+
+// Run (with children)
+// NOTE 'Valid Args:' is not shown because ValidArgs is not defined
+
+func TestArgs_Run_wChild(t *testing.T) {
+	c := newCmd(nil, false, true)
+	d := newCmd(nil, false, true)
+	c.AddCommand(d)
+	//	o, e := executeCommand(c)
+	//	expectSuccess(o, e, t)
+	o, _ := executeUsage(c)
+	checkOutput(o, t, "ucf")
+}
+
+func TestArgs_Run_wChild_ArbValid(t *testing.T) {
+	c := newCmd(nil, false, true)
+	d := newCmd(nil, false, false)
+	c.AddCommand(d)
+	o, _ := executeCommand(c, "one", "three")
+	//	expectError(e, t, "no")
+	// NOTE 'c [command]' is not shown because this command does not have any available subcommand
+	checkOutput(o, t, "uf")
+}
+
+func TestArgs_Run_wChild_Invalid(t *testing.T) {
+	c := newCmd(nil, false, true)
+	d := newCmd(nil, false, false)
+	c.AddCommand(d)
+	o, _ := executeCommand(c, "one", "a")
+	//	expectError(e, t, "no")
+	// NOTE 'c [command]' is not shown because this command does not have any available subcommand
+	checkOutput(o, t, "uf")
+}
+
+// Run Args
+// NOTE 'c [command]' is not shown because this command does not have any subcommand
+
+func TestArgs_Run_wArgs(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", ArbitraryArgs, false, true, []string{}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "ua")
+}
+
+func TestArgs_Run_wArgs_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", ArbitraryArgs, false, true, []string{"one", "three"}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "ua")
+}
+
+func TestArgs_Run_wArgs_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", ArbitraryArgs, false, true, []string{"two", "a"}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "ua")
+}
+
+// Run ValidArgs
+// NOTE 'c [command]' is not shown because this command does not have any subcommand
+
+func TestArgs_Run_wValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", nil, true, true, []string{}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "uva")
+}
+
+func TestArgs_Run_wValid_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", nil, true, true, []string{"one", "three"}},
+	})
+	//	o, e = executeUsage(c)
+	//	checkOutput(o, t, "uva")
+}
+
+func TestArgs_Run_wValid_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"invalid", nil, true, true, []string{"two", "a"}},
+	})
+	//	checkOutput(o, t, "uva")
+}
+
+// Run Args ValidArgs
+// NOTE 'c [command]' is not shown because this command does not have any subcommand
+
+func TestArgs_Run_wArgswValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", ArbitraryArgs, true, true, []string{}},
+	})
+	//o, e = executeUsage(c)
+	//checkOutput(o, t, "uva")
+}
+
+func TestArgs_Run_wArgswValid_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", ArbitraryArgs, true, true, []string{"one", "three"}},
+	})
+	//o, e = executeUsage(c)
+	//checkOutput(o, t, "uva")
+}
+
+func TestArgs_Run_wArgswValid_Invalid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"invalid", ArbitraryArgs, true, true, []string{"two", "a"}},
+	})
+	//checkOutput(o, t, "uva")
+}
+
+//
+
+func TestArgs_Run_wMinimumNArgs_ArbValid(t *testing.T) {
+	testArgs(t, map[string]argsTestcase{
+		"|": {"", MinimumNArgs(2), false, true, []string{"one", "three"}},
+	})
+	//o, e = executeUsage(c)
+	//checkOutput(o, t, "ua")
+}
+
 func TestMatchAll(t *testing.T) {
 	// Somewhat contrived example check that ensures there are exactly 3
 	// arguments, and each argument is exactly 2 bytes long.

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.",

command.go

@@ -29,6 +29,13 @@ import (
 	flag "github.com/spf13/pflag"
 )
 
+func ErrSubCommandRequired(s string) error {
+	return fmt.Errorf("command %s is not runnable; please provide a subcmd", s)
+}
+func ErrCommandNotRunnable(s string) error {
+	return fmt.Errorf(`command "%s" is not runnable`, s)
+}
+
 // FParseErrWhitelist configures Flag parse errors to be ignored
 type FParseErrWhitelist flag.ParseErrorsWhitelist
 
@@ -505,7 +512,10 @@ func (c *Command) UsageTemplate() string {
   {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}}
 
 Aliases:
-  {{.NameAndAliases}}{{end}}{{if .HasExample}}
+  {{.NameAndAliases}}{{end}}{{if (and .HasValidArgs .Runnable)}}
+
+Valid Args:
+  {{range .ValidArgs}}{{.}} {{end}}{{end}}{{if .HasExample}}
 
 Examples:
 {{.Example}}{{end}}{{if .HasAvailableSubCommands}}
@@ -632,7 +642,7 @@ func isFlagArg(arg string) bool {
 
 // Find the target command given the args and command tree
 // Meant to be run on the highest node. Only searches down.
-func (c *Command) Find(args []string) (*Command, []string, error) {
+func (c *Command) Find(args []string) (*Command, []string) {
 	var innerfind func(*Command, []string) (*Command, []string)
 
 	innerfind = func(c *Command, innerArgs []string) (*Command, []string) {
@@ -649,11 +659,13 @@ func (c *Command) Find(args []string) (*Command, []string, error) {
 		return c, innerArgs
 	}
 
-	commandFound, a := innerfind(c, args)
-	if commandFound.Args == nil {
-		return commandFound, a, legacyArgs(commandFound, stripFlags(a, commandFound))
+	cF, a := innerfind(c, args)
+	// if Args is undefined and this is a root command with subcommands,
+	// do not accept arguments, unless ValidArgs is set
+	if cF.Args == nil && cF.HasSubCommands() && !cF.HasParent() && (len(cF.ValidArgs) == 0) {
+		cF.Args = NoArgs
 	}
-	return commandFound, a, nil
+	return cF, a
 }
 
 func (c *Command) findSuggestions(arg string) string {
@@ -694,7 +706,7 @@ func (c *Command) findNext(next string) *Command {
 
 // Traverse the command tree to find the command, and parse args for
 // each parent.
-func (c *Command) Traverse(args []string) (*Command, []string, error) {
+func (c *Command) Traverse(args []string) (*Command, []string) {
 	flags := []string{}
 	inFlag := false
 
@@ -724,15 +736,15 @@ func (c *Command) Traverse(args []string) (*Command, []string, error) {
 
 		cmd := c.findNext(arg)
 		if cmd == nil {
-			return c, args, nil
+			return c, args
 		}
 
 		if err := c.ParseFlags(flags); err != nil {
-			return nil, args, err
+			return nil, append([]string{err.Error()}, args...)
 		}
 		return cmd.Traverse(args[i+1:])
 	}
-	return c, args, nil
+	return c, args
 }
 
 // SuggestionsFor provides suggestions for the typedName.
@@ -828,7 +840,10 @@ func (c *Command) execute(a []string) (err error) {
 	}
 
 	if !c.Runnable() {
-		return flag.ErrHelp
+		if c.HasAvailableSubCommands() {
+			return ErrSubCommandRequired(c.Name())
+		}
+		return ErrCommandNotRunnable(c.Name())
 	}
 
 	c.preRun()
@@ -949,7 +964,6 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 	c.initDefaultCompletionCmd()
 
 	args := c.args
-
 	// Workaround FAIL with "go test -v" or "cobra.test -test.v", see #155
 	if c.args == nil && filepath.Base(os.Args[0]) != "cobra.test" {
 		args = os.Args[1:]
@@ -959,35 +973,29 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 	c.initCompleteCmd(args)
 
 	var flags []string
+	f := c.Find
 	if c.TraverseChildren {
-		cmd, flags, err = c.Traverse(args)
-	} else {
-		cmd, flags, err = c.Find(args)
+		f = c.Traverse
 	}
-	if err != nil {
-		// If found parse to a subcommand and then failed, talk about the subcommand
-		if cmd != nil {
-			c = cmd
+	if cmd, flags = f(args); cmd != nil {
+		cmd.commandCalledAs.called = true
+		if cmd.commandCalledAs.name == "" {
+			cmd.commandCalledAs.name = cmd.Name()
 		}
 		if !c.SilenceErrors {
 			c.PrintErrln("Error:", err.Error())
 			c.PrintErrf("Run '%v --help' for usage.\n", c.CommandPath())
 		}
-		return c, err
+		// We have to pass global context to children command
+		// if context is present on the parent command.
+		if cmd.ctx == nil {
+			cmd.ctx = c.ctx
+		}
+		err = cmd.execute(flags)
+	} else {
+		err = fmt.Errorf(flags[0])
 	}
 
-	cmd.commandCalledAs.called = true
-	if cmd.commandCalledAs.name == "" {
-		cmd.commandCalledAs.name = cmd.Name()
-	}
-
-	// We have to pass global context to children command
-	// if context is present on the parent command.
-	if cmd.ctx == nil {
-		cmd.ctx = c.ctx
-	}
-
-	err = cmd.execute(flags)
 	if err != nil {
 		// Always show help if requested, even if SilenceErrors is in
 		// effect
@@ -996,6 +1004,11 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 			return cmd, nil
 		}
 
+		// Check if a shorter help hint should be shown instead of the full Usage()
+		if err := helpHint(cmd, flags, err.Error()); err != nil {
+			return cmd, err
+		}
+
 		// If root command has SilenceErrors flagged,
 		// all subcommands should respect it
 		if !cmd.SilenceErrors && !c.SilenceErrors {
@@ -1011,7 +1024,32 @@ func (c *Command) ExecuteC() (cmd *Command, err error) {
 	return cmd, err
 }
 
+func helpHint(c *Command, fs []string, e string) error {
+	if len(fs) > 0 {
+		f := fs[0]
+		for _, s := range []string{"please provide a subcmd", "unknown command"} {
+			if strings.Contains(e, s) {
+				if s := c.findSuggestions(f); len(s) != 0 {
+					e += s
+				}
+				if !c.SilenceErrors {
+					c.Printf("Error: %s\n", e)
+					c.Printf("Run '%v --help' for usage.\n", c.CommandPath())
+				}
+				return fmt.Errorf("%s", e)
+			}
+		}
+	}
+	return nil
+}
+
+// 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 ArbitraryArgs(c, args)
 	}
@@ -1271,9 +1309,35 @@ func (c *Command) UseLine() string {
 	if c.HasAvailableFlags() && !strings.Contains(useline, "[flags]") {
 		useline += " [flags]"
 	}
+
+	useline += useLineArgs(c)
+
 	return useline
 }
 
+// useLineArgs puts out '[args]' if a given command accepts positional args
+func useLineArgs(c *Command) (s string) {
+	s = " [args]"
+	if c.Args == nil {
+		if !c.HasAvailableSubCommands() || c.HasParent() {
+			return
+		}
+		// if Args is undefined and this is a root command with subcommands,
+		// do not accept arguments, unless ValidArgs is set
+		if !c.HasParent() && c.HasAvailableSubCommands() && (len(c.ValidArgs) > 0) {
+			return
+		}
+		return ""
+	}
+	// Check if the Args validator is other than 'NoArgs'
+	err := c.Args(c, []string{"someUnexpectedIllegalArg"})
+	nerr := NoArgs(c, []string{"someUnexpectedIllegalArg"})
+	if err == nil || ((nerr != nil) && (err.Error() != nerr.Error())) {
+		return
+	}
+	return ""
+}
+
 // DebugFlags used to determine which flags have been assigned to which commands
 // and which persist.
 func (c *Command) DebugFlags() {
@@ -1365,6 +1429,10 @@ func (c *Command) NameAndAliases() string {
 	return strings.Join(append([]string{c.Name()}, c.Aliases...), ", ")
 }
 
+func (c *Command) HasValidArgs() bool {
+	return len(c.ValidArgs) > 0
+}
+
 // HasExample determines if the command has example.
 func (c *Command) HasExample() bool {
 	return len(c.Example) > 0
@@ -1438,16 +1506,14 @@ func (c *Command) HasHelpSubCommands() bool {
 // HasAvailableSubCommands determines if a command has available sub commands that
 // need to be shown in the usage/help default template under 'available commands'.
 func (c *Command) HasAvailableSubCommands() bool {
-	// return true on the first found available (non deprecated/help/hidden)
-	// sub command
+	// return true on the first found available (non deprecated/help/hidden) subcmd
 	for _, sub := range c.commands {
 		if sub.IsAvailableCommand() {
 			return true
 		}
 	}
-
-	// the command either has no sub commands, or no available (non deprecated/help/hidden)
-	// sub commands
+	// the command either has no sub commands,
+	// or no available (non deprecated/help/hidden) subcmds
 	return false
 }
 

command_test.go

@@ -134,9 +134,7 @@ func TestRootExecuteUnknownCommand(t *testing.T) {
 	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
 
 	output, _ := executeCommand(rootCmd, "unknown")
-
 	expected := "Error: unknown command \"unknown\" for \"root\"\nRun 'root --help' for usage.\n"
-
 	if output != expected {
 		t.Errorf("Expected:\n %q\nGot:\n %q\n", expected, output)
 	}
@@ -997,11 +995,13 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) {
 	rootCmd.AddCommand(childCmd)
 
 	output, err := executeCommand(rootCmd, "child")
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+
+	expected := `command "child" is not runnable`
+	if err.Error() != expected {
+		t.Errorf("Expected %q, got %q", expected, err.Error())
 	}
 
-	checkStringContains(t, output, childCmd.Long)
+	checkStringContains(t, output, "Usage:")
 }
 
 func TestVersionFlagExecuted(t *testing.T) {
@@ -1849,9 +1849,9 @@ func TestTraverseWithParentFlags(t *testing.T) {
 
 	rootCmd.AddCommand(childCmd)
 
-	c, args, err := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"})
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+	c, args := rootCmd.Traverse([]string{"-b", "--str", "ok", "child", "--int"})
+	if c == nil {
+		t.Errorf("Unexpected error: %s", args[0])
 	}
 	if len(args) != 1 && args[0] != "--add" {
 		t.Errorf("Wrong args: %v", args)
@@ -1869,9 +1869,9 @@ func TestTraverseNoParentFlags(t *testing.T) {
 	childCmd.Flags().String("str", "", "")
 	rootCmd.AddCommand(childCmd)
 
-	c, args, err := rootCmd.Traverse([]string{"child"})
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+	c, args := rootCmd.Traverse([]string{"child"})
+	if c == nil {
+		t.Errorf("Unexpected error: %v", args[0])
 	}
 	if len(args) != 0 {
 		t.Errorf("Wrong args %v", args)
@@ -1890,13 +1890,13 @@ func TestTraverseWithBadParentFlags(t *testing.T) {
 
 	expected := "unknown flag: --str"
 
-	c, _, err := rootCmd.Traverse([]string{"--str", "ok", "child"})
-	if err == nil || !strings.Contains(err.Error(), expected) {
-		t.Errorf("Expected error, %q, got %q", expected, err)
-	}
+	c, args := rootCmd.Traverse([]string{"--str", "ok", "child"})
 	if c != nil {
 		t.Errorf("Expected nil command")
 	}
+	if !strings.Contains(args[0], expected) {
+		t.Errorf("Expected error, %q, got %q", expected, args[0])
+	}
 }
 
 func TestTraverseWithBadChildFlag(t *testing.T) {
@@ -1908,9 +1908,9 @@ func TestTraverseWithBadChildFlag(t *testing.T) {
 
 	// Expect no error because the last commands args shouldn't be parsed in
 	// Traverse.
-	c, args, err := rootCmd.Traverse([]string{"child", "--str"})
-	if err != nil {
-		t.Errorf("Unexpected error: %v", err)
+	c, args := rootCmd.Traverse([]string{"child", "--str"})
+	if c == nil {
+		t.Errorf("Unexpected error: %s", args[0])
 	}
 	if len(args) != 1 && args[0] != "--str" {
 		t.Errorf("Wrong args: %v", args)
@@ -1931,9 +1931,9 @@ func TestTraverseWithTwoSubcommands(t *testing.T) {
 	}
 	subCmd.AddCommand(subsubCmd)
 
-	c, _, err := rootCmd.Traverse([]string{"sub", "subsub"})
-	if err != nil {
-		t.Fatalf("Unexpected error: %v", err)
+	c, args := rootCmd.Traverse([]string{"sub", "subsub"})
+	if c == nil {
+		t.Fatalf("Unexpected error: %v", args[0])
 	}
 	if c.Name() != subsubCmd.Name() {
 		t.Fatalf("Expected command: %q, got %q", subsubCmd.Name(), c.Name())

user_guide.md

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