Allow default commands with cmds/args/flags (#188)

* Move default command validation to build Move the validation of default commands - checking if a node has multiple default commands or a default command has children - to the build phase rather than tracing. These errors are with the structure of the CLI ast and are detectable before parsing the command line. Add a couple of tests for some of the default command error cases. * Disallow positional args on a default command Do not allow a default command to have positional arguments. The current check is only for branching args, but the error message implies that positional args are not allowed either. So add a check for positional args too, and add a test case for it. This is breaking change to the API but is unlikely to have ill-effect as positional args on a default command cannot be used without explicitly naming the command (i.e. not using it as a default). * Allow default commands with cmds/args/flags Allow default commands to have sub-commands, args and flags when tagged with `default:"withargs"`. This makes specifying the name of the command on the CLI completely optional as long as the args to that command are not ambiguous with other commands.
2021-07-12 14:19:37 +10:00
parent cfbe844aa4
commit 89315e74ad
6 changed files with 150 additions and 45 deletions
@@ -35,15 +35,13 @@ linters:
    - tagliatelle
    - thelper
    - godox
+    - goconst

 linters-settings:
  govet:
    check-shadowing: true
  dupl:
    threshold: 100
-  goconst:
-    min-len: 5
-    min-occurrences: 3
  gocyclo:
    min-complexity: 20
  exhaustive:
@@ -302,7 +302,7 @@ type CLI struct {
 }
 ```

-If a sub-command is tagged with `default:"1"` it will be selected if there are no further arguments.
+If a sub-command is tagged with `default:"1"` it will be selected if there are no further arguments. If a sub-command is tagged with `default:"withargs"` it will be selected even if there are further arguments or flags and those arguments or flags are valid for the sub-command. This allows the user to omit the sub-command name on the CLI if its arguments/flags are not ambiguous with the sibling commands or flags.

 ## Branching positional arguments

@@ -435,6 +435,7 @@ Tag                    | Description
 `placeholder:"X"`      | Placeholder text.
 `default:"X"`          | Default value.
 `default:"1"`          | On a command, make it the default.
+`default:"withargs"`   | On a command, make it the default and allow args/flags from that command
 `short:"X"`            | Short name, if flag.
 `aliases:"X,Y"`        | One or more aliases (for cmd).
 `required:""`          | If present, flag/arg is required.
@@ -141,6 +141,7 @@ func buildNode(k *Kong, v reflect.Value, typ NodeType, seenFlags map[string]bool

 func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.StructField, fv reflect.Value, tag *Tag, name string, seenFlags map[string]bool) {
 	child := buildNode(k, fv, typ, seenFlags)
+	child.Name = name
 	child.Tag = tag
 	child.Parent = node
 	child.Help = tag.Help
@@ -158,21 +159,23 @@ func buildChild(k *Kong, node *Node, typ NodeType, v reflect.Value, ft reflect.S
 		if len(child.Positional) == 0 {
 			failField(v, ft, "positional branch must have at least one child positional argument named %q", name)
 		}
-
-		value := child.Positional[0]
-		child.Positional = child.Positional[1:]
-		if child.Help == "" {
-			child.Help = value.Help
-		}
-
-		child.Name = value.Name
-		if child.Name != name {
+		if child.Positional[0].Name != name {
 			failField(v, ft, "first field in positional branch must have the same name as the parent field (%s).", child.Name)
 		}

-		child.Argument = value
-	} else {
-		child.Name = name
+		child.Argument = child.Positional[0]
+		child.Positional = child.Positional[1:]
+		if child.Help == "" {
+			child.Help = child.Argument.Help
+		}
+	} else if tag.Default != "" {
+		if node.DefaultCmd != nil {
+			failField(v, ft, "can't have more than one default command under %s", node.Summary())
+		}
+		if tag.Default != "withargs" && (len(child.Children) > 0 || len(child.Positional) > 0) {
+			failField(v, ft, "default command %s must not have subcommands or arguments", child.Summary())
+		}
+		node.DefaultCmd = child
 	}
 	node.Children = append(node.Children, child)

@@ -343,7 +343,14 @@ func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
 	positional := 0

 	flags := []*Flag{}
-	for _, group := range node.AllFlags(false) {
+	flagNode := node
+	if node.DefaultCmd != nil && node.DefaultCmd.Tag.Default == "withargs" {
+		// Add flags of the default command if the current node has one
+		// and that default command allows args / flags without explicitly
+		// naming the command on the CLI.
+		flagNode = node.DefaultCmd
+	}
+	for _, group := range flagNode.AllFlags(false) {
 		flags = append(flags, group...)
 	}

@@ -483,6 +490,17 @@ func (c *Context) trace(node *Node) (err error) { // nolint: gocyclo
 				}
 			}

+			// If there is a default command that allows args and nothing else
+			// matches, take the branch of the default command
+			if node.DefaultCmd != nil && node.DefaultCmd.Tag.Default == "withargs" {
+				c.Path = append(c.Path, &Path{
+					Parent:  node,
+					Command: node.DefaultCmd,
+					Flags:   node.DefaultCmd.Flags,
+				})
+				return c.trace(node.DefaultCmd)
+			}
+
 			return findPotentialCandidates(token.String(), candidates, "unexpected argument %s", token)
 		default:
 			return fmt.Errorf("unexpected token %s", token)
@@ -499,21 +517,12 @@ func (c *Context) maybeSelectDefault(flags []*Flag, node *Node) error {
 			return nil
 		}
 	}
-	var defaultNode *Path
-	for _, child := range node.Children {
-		if child.Type == CommandNode && child.Tag.Default != "" {
-			if defaultNode != nil {
-				return fmt.Errorf("can't have more than one default command under %s", node.Summary())
-			}
-			defaultNode = &Path{
-				Parent:  child,
-				Command: child,
-				Flags:   child.Flags,
-			}
-		}
-	}
-	if defaultNode != nil {
-		c.Path = append(c.Path, defaultNode)
+	if node.DefaultCmd != nil {
+		c.Path = append(c.Path, &Path{
+			Parent:  node.DefaultCmd,
+			Command: node.DefaultCmd,
+			Flags:   node.DefaultCmd.Flags,
+		})
 	}
 	return nil
 }
@@ -794,7 +803,6 @@ func checkMissingChildren(node *Node) error {
 		missing = append(missing, strconv.Quote(strings.Join(missingArgs, " ")))
 	}

-	haveDefault := 0
 	for _, child := range node.Children {
 		if child.Hidden {
 			continue
@@ -805,19 +813,10 @@ func checkMissingChildren(node *Node) error {
 			}
 			missing = append(missing, strconv.Quote(child.Summary()))
 		} else {
-			if child.Tag.Default != "" {
-				if len(child.Children) > 0 {
-					return fmt.Errorf("default command %s must not have subcommands or arguments", child.Summary())
-				}
-				haveDefault++
-			}
 			missing = append(missing, strconv.Quote(child.Name))
 		}
 	}
-	if haveDefault > 1 {
-		return fmt.Errorf("more than one default command found under %s", node.Summary())
-	}
-	if len(missing) == 0 || haveDefault > 0 {
+	if len(missing) == 0 {
 		return nil
 	}

@@ -1020,9 +1020,112 @@ func TestMultipleDefaultCommands(t *testing.T) {
 		One struct{} `cmd:"" default:"1"`
 		Two struct{} `cmd:"" default:"1"`
 	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.Two: can't have more than one default command under  <command>")
+}
+
+func TestDefaultCommandWithSubCommand(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct{} `cmd:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <command> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedSubCommand(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct{} `cmd:""`
+		} `cmd:"" default:"withargs"`
+	}
 	p := mustNew(t, &cli)
-	_, err := p.Parse([]string{})
-	require.EqualError(t, err, "can't have more than one default command under  <command>")
+	ctx, err := p.Parse([]string{"two"})
+	require.NoError(t, err)
+	require.Equal(t, "one two", ctx.Command())
+}
+
+func TestDefaultCommandWithArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Arg string `arg:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <arg> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Arg  string `arg:""`
+			Flag string
+		} `cmd:"" default:"withargs"`
+	}
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"arg", "--flag=value"})
+	require.NoError(t, err)
+	require.Equal(t, "arg", cli.One.Arg)
+	require.Equal(t, "value", cli.One.Flag)
+}
+
+func TestDefaultCommandWithBranchingArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct {
+				Two string `arg:""`
+			} `arg:""`
+		} `cmd:"" default:"1"`
+	}
+	_, err := kong.New(&cli)
+	require.EqualError(t, err, "<anonymous struct>.One: default command one <command> must not have subcommands or arguments")
+}
+
+func TestDefaultCommandWithAllowedBranchingArgument(t *testing.T) {
+	var cli struct {
+		One struct {
+			Two struct {
+				Two  string `arg:""`
+				Flag string
+			} `arg:""`
+		} `cmd:"" default:"withargs"`
+	}
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"arg", "--flag=value"})
+	require.NoError(t, err)
+	require.Equal(t, "arg", cli.One.Two.Two)
+	require.Equal(t, "value", cli.One.Two.Flag)
+}
+
+func TestDefaultCommandPrecedence(t *testing.T) {
+	var cli struct {
+		Two struct {
+			Arg  string `arg:""`
+			Flag bool
+		} `cmd:"" default:"withargs"`
+		One struct{} `cmd:""`
+	}
+	p := mustNew(t, &cli)
+
+	// A named command should take precedence over a default command with arg
+	ctx, err := p.Parse([]string{"one"})
+	require.NoError(t, err)
+	require.Equal(t, "one", ctx.Command())
+
+	// An explicitly named command with arg should parse, even if labeled default:"witharg"
+	ctx, err = p.Parse([]string{"two", "arg"})
+	require.NoError(t, err)
+	require.Equal(t, "two <arg>", ctx.Command())
+
+	// An arg to a default command that does not match another command should select the default
+	ctx, err = p.Parse([]string{"arg"})
+	require.NoError(t, err)
+	require.Equal(t, "two <arg>", ctx.Command())
+
+	// A flag on a default command should not be valid on a sibling command
+	_, err = p.Parse([]string{"one", "--flag"})
+	require.EqualError(t, err, "unknown flag --flag")
 }

 func TestLoneHpyhen(t *testing.T) {
@@ -51,6 +51,7 @@ type Node struct {
 	Flags      []*Flag
 	Positional []*Positional
 	Children   []*Node
+	DefaultCmd *Node
 	Target     reflect.Value // Pointer to the value in the grammar that this Node is associated with.
 	Tag        *Tag
 	Aliases    []string