hotfix: apply CLI flags to entire subtree (#563)

pkgs/commands/command.go

@@ -73,11 +73,40 @@ func NewCommand(
 func (c *Command) AddSubCommands(cmds ...*Command) {
 	for _, cmd := range cmds {
 		if c.cfg != nil {
-			// Register the parent flagset
+			// Register the parent flagset with the child.
+			// The syntax is not intuitive, but the flagset being
+			// modified is the subcommand's, using the flags defined
+			// in the parent command
 			c.cfg.RegisterFlags(cmd.FlagSet)
+
+			// Register the parent flagset with all the
+			// subcommands of the child as well
+			// (ex. grandparent flags are available in child commands)
+			registerFlagsWithSubcommands(c.cfg, &cmd.Command)
 		}
 
 		// Append the subcommand to the parent
 		c.Subcommands = append(c.Subcommands, &cmd.Command)
 	}
 }
+
+// registerFlagsWithSubcommands recursively registers the passed in
+// configuration's flagset with the subcommand tree. At the point of calling
+// this method, the child subcommand tree should already be present, due to the
+// way subcommands are built (LIFO)
+func registerFlagsWithSubcommands(cfg Config, root *ffcli.Command) {
+	subcommands := []*ffcli.Command{root}
+
+	// Traverse the direct subcommand tree,
+	// and register the top-level flagset with each
+	// direct line subcommand
+	for len(subcommands) > 0 {
+		current := subcommands[0]
+		subcommands = subcommands[1:]
+
+		for _, subcommand := range current.Subcommands {
+			cfg.RegisterFlags(subcommand.FlagSet)
+			subcommands = append(subcommands, subcommand)
+		}
+	}
+}

pkgs/commands/commands_test.go

@@ -0,0 +1,155 @@
+package commands
+
+import (
+	"flag"
+	"testing"
+
+	"github.com/peterbourgon/ff/v3/ffcli"
+	"github.com/stretchr/testify/assert"
+)
+
+type configDelegate func(*flag.FlagSet)
+
+type mockConfig struct {
+	configFn configDelegate
+}
+
+func (c *mockConfig) RegisterFlags(fs *flag.FlagSet) {
+	if c.configFn != nil {
+		c.configFn(fs)
+	}
+}
+
+func TestCommand_AddSubCommands(t *testing.T) {
+	t.Parallel()
+
+	// Test setup //
+
+	type testCmd struct {
+		cmd     *Command
+		subCmds []*testCmd
+	}
+
+	getSubcommands := func(t *testCmd) []*Command {
+		res := make([]*Command, len(t.subCmds))
+
+		for i, subCmd := range t.subCmds {
+			res[i] = subCmd.cmd
+		}
+
+		return res
+	}
+
+	generateTestCmd := func(name string) *Command {
+		return NewCommand(
+			Metadata{
+				Name: name,
+			},
+			&mockConfig{
+				func(fs *flag.FlagSet) {
+					fs.String(
+						name,
+						"",
+						"",
+					)
+				},
+			},
+			HelpExec,
+		)
+	}
+
+	var postorderCommands func(root *testCmd) []*testCmd
+
+	postorderCommands = func(root *testCmd) []*testCmd {
+		if root == nil {
+			return nil
+		}
+
+		res := make([]*testCmd, 0)
+
+		for _, child := range root.subCmds {
+			res = append(res, postorderCommands(child)...)
+		}
+
+		return append(res, root)
+	}
+
+	// Cases //
+
+	testTable := []struct {
+		name   string
+		topCmd *testCmd
+	}{
+		{
+			name: "no subcommands",
+			topCmd: &testCmd{
+				cmd:     generateTestCmd("level0"),
+				subCmds: nil,
+			},
+		},
+		{
+			name: "single subcommand level",
+			topCmd: &testCmd{
+				cmd: generateTestCmd("level0"),
+				subCmds: []*testCmd{
+					{
+						cmd:     generateTestCmd("level1"),
+						subCmds: nil,
+					},
+				},
+			},
+		},
+		{
+			name: "multiple subcommand levels",
+			topCmd: &testCmd{
+				cmd: generateTestCmd("level0"),
+				subCmds: []*testCmd{
+					{
+						cmd: generateTestCmd("level1"),
+						subCmds: []*testCmd{
+							{
+								cmd:     generateTestCmd("level2"),
+								subCmds: nil,
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, testCase := range testTable {
+		testCase := testCase
+
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			var validateSubcommandTree func(flag string, root *ffcli.Command)
+
+			validateSubcommandTree = func(flag string, root *ffcli.Command) {
+				assert.NotNil(t, root.FlagSet.Lookup(flag))
+
+				for _, subcommand := range root.Subcommands {
+					validateSubcommandTree(flag, subcommand)
+				}
+			}
+
+			// Register the subcommands in LIFO order (postorder), starting from the
+			// leaf of the command tree (mimics how the commands package is used)
+			commandOrder := postorderCommands(testCase.topCmd)
+
+			for _, currCmd := range commandOrder {
+				// For the current command, register its subcommand tree
+				currCmd.cmd.AddSubCommands(getSubcommands(currCmd)...)
+
+				// Validate that the entire subcommand tree has root command flags
+				for _, subCmd := range currCmd.cmd.Subcommands {
+					// For each root command flag, validate
+					currCmd.cmd.FlagSet.VisitAll(func(f *flag.Flag) {
+						validateSubcommandTree(f.Name, subCmd)
+					})
+				}
+			}
+		})
+	}
+}