Merge pull request #84 from eparis/subcommand-invalid-flag

Stop special casing runnable root commands

cobra_test.go

@@ -3,11 +3,13 @@ package cobra
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 )
 
 var _ = fmt.Println
+var _ = os.Stderr
 
 var tp, te, tt, t1 []string
 var flagb1, flagb2, flagb3, flagbr, flagbp bool
@@ -22,7 +24,7 @@ const strtwoChildHelp = "help message for child flag strtwo"
 var cmdPrint = &Command{
 	Use:   "print [string to print]",
 	Short: "Print anything to the screen",
-	Long:  `an utterly useless command for testing.`,
+	Long:  `an absolutely utterly useless command for testing.`,
 	Run: func(cmd *Command, args []string) {
 		tp = args
 	},
@@ -41,7 +43,7 @@ var cmdEcho = &Command{
 var cmdTimes = &Command{
 	Use:   "times [# times] [string to echo]",
 	Short: "Echo anything to the screen more times",
-	Long:  `an slightly useless command for testing.`,
+	Long:  `a slightly useless command for testing.`,
 	Run: func(cmd *Command, args []string) {
 		tt = args
 	},
@@ -417,6 +419,20 @@ func TestTrailingCommandFlags(t *testing.T) {
 	}
 }
 
+func TestInvalidSubCommandFlags(t *testing.T) {
+	cmd := initializeWithRootCmd()
+	cmd.AddCommand(cmdTimes)
+
+	result := simpleTester(cmd, "times --inttwo=2 --badflag=bar")
+
+	checkResultContains(t, result, "unknown flag: --badflag")
+
+	if strings.Contains(result.Output, "unknown flag: --inttwo") {
+		t.Errorf("invalid --badflag flag shouldn't fail on 'unknown' --inttwo flag")
+	}
+
+}
+
 func TestPersistentFlags(t *testing.T) {
 	fullSetupTest("echo -s something -p more here")
 
@@ -473,6 +489,53 @@ func TestRunnableRootCommand(t *testing.T) {
 	}
 }
 
+func TestRunnableRootCommandNilInput(t *testing.T) {
+	empty_arg := make([]string, 0)
+	c := initializeWithRootCmd()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(empty_arg)
+
+	c.Execute()
+
+	if rootcalled != true {
+		t.Errorf("Root Function was not called")
+	}
+}
+
+func TestRunnableRootCommandEmptyInput(t *testing.T) {
+	args := make([]string, 3)
+	args[0] = ""
+	args[1] = "--introot=12"
+	args[2] = ""
+	c := initializeWithRootCmd()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(args)
+
+	c.Execute()
+
+	if rootcalled != true {
+		t.Errorf("Root Function was not called.\n\nOutput was:\n\n%s\n", buf)
+	}
+}
+
+func TestInvalidSubcommandWhenArgsAllowed(t *testing.T) {
+	fullSetupTest("echo invalid-sub")
+
+	if te[0] != "invalid-sub" {
+		t.Errorf("Subcommand didn't work...")
+	}
+}
+
 func TestRootFlags(t *testing.T) {
 	fullSetupTest("-i 17 -b")
 
@@ -534,6 +597,24 @@ func TestFlagAccess(t *testing.T) {
 	}
 }
 
+func TestNoNRunnableRootCommandNilInput(t *testing.T) {
+	args := make([]string, 0)
+	c := initialize()
+
+	buf := new(bytes.Buffer)
+	// Testing flag with invalid input
+	c.SetOutput(buf)
+	cmdEcho.AddCommand(cmdTimes)
+	c.AddCommand(cmdPrint, cmdEcho)
+	c.SetArgs(args)
+
+	c.Execute()
+
+	if !strings.Contains(buf.String(), cmdRootNoRun.Long) {
+		t.Errorf("Expected to get help output, Got: \n %s", buf)
+	}
+}
+
 func TestRootNoCommandHelp(t *testing.T) {
 	x := rootOnlySetupTest("--help")
 

command.go

@@ -305,6 +305,8 @@ func stripFlags(args []string, c *Command) []string {
 				inFlag = true
 			case inFlag:
 				inFlag = false
+			case y == "":
+				// strip empty commands, as the go tests expect this to be ok....
 			case !strings.HasPrefix(y, "-"):
 				commands = append(commands, y)
 				inFlag = false
@@ -375,9 +377,8 @@ func (c *Command) Find(arrs []string) (*Command, []string, error) {
 
 	commandFound, a := innerfind(c, arrs)
 
-	// if commander returned and the first argument (if it exists) doesn't
-	// match the command name, return nil & error
-	if commandFound.Name() == c.Name() && len(arrs[0]) > 0 && commandFound.Name() != arrs[0] {
+	// If we matched on the root, but we asked for a subcommand, return an error
+	if commandFound.Name() == c.Name() && len(stripFlags(arrs, c)) > 0 && commandFound.Name() != arrs[0] {
 		return nil, a, fmt.Errorf("unknown command %q", a[0])
 	}
 
@@ -398,16 +399,6 @@ func (c *Command) Root() *Command {
 	return findRoot(c)
 }
 
-// execute the command determined by args and the command tree
-func (c *Command) findAndExecute(args []string) (err error) {
-
-	cmd, a, e := c.Find(args)
-	if e != nil {
-		return e
-	}
-	return cmd.execute(a)
-}
-
 func (c *Command) execute(a []string) (err error) {
 	if c == nil {
 		return fmt.Errorf("Called Execute() on a nil Command")
@@ -494,45 +485,11 @@ func (c *Command) Execute() (err error) {
 			c.Help()
 		}
 	} else {
-		err = c.findAndExecute(args)
-	}
-
-	// Now handle the case where the root is runnable and only flags are provided
-	if err != nil && c.Runnable() {
-		// This is pretty much a custom version of the *Command.execute method
-		// with a few differences because it's the final command (no fall back)
-		e := c.ParseFlags(args)
+		cmd, flags, e := c.Find(args)
 		if e != nil {
-			// Flags parsing had an error.
-			// If an error happens here, we have to report it to the user
-			c.Println(e.Error())
-			// If an error happens search also for subcommand info about that
-			if c.cmdErrorBuf != nil && c.cmdErrorBuf.Len() > 0 {
-				c.Println(c.cmdErrorBuf.String())
-			} else {
-				c.Usage()
-			}
 			err = e
-			return
 		} else {
-			// If help is called, regardless of other flags, we print that
-			if c.helpFlagVal {
-				c.Help()
-				return nil
-			}
-
-			argWoFlags := c.Flags().Args()
-			if len(argWoFlags) > 0 {
-				// If there are arguments (not flags) one of the earlier
-				// cases should have caught it.. It means invalid usage
-				// print the usage
-				c.Usage()
-			} else {
-				// Only flags left... Call root.Run
-				c.preRun()
-				c.Run(c, argWoFlags)
-				err = nil
-			}
+			err = cmd.execute(flags)
 		}
 	}