cli: deprecate VisitAll, DisableFlagsInUseLine utilities

These utilities were only used internally; create a local copy
where used, and deprecate the ones in cli.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
(cherry picked from commit 6bd8a4b)
Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

cli-plugins/plugin/plugin.go

@@ -175,11 +175,24 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta
 		newMetadataSubcommand(plugin, meta),
 	)
 
-	cli.DisableFlagsInUseLine(cmd)
+	visitAll(cmd,
+		// prevent adding "[flags]" to the end of the usage line.
+		func(c *cobra.Command) { c.DisableFlagsInUseLine = true },
+	)
 
 	return cli.NewTopLevelCommand(cmd, dockerCli, opts, cmd.Flags())
 }
 
+// visitAll traverses all commands from the root.
+func visitAll(root *cobra.Command, fns ...func(*cobra.Command)) {
+	for _, cmd := range root.Commands() {
+		visitAll(cmd, fns...)
+	}
+	for _, fn := range fns {
+		fn(root)
+	}
+}
+
 func newMetadataSubcommand(plugin *cobra.Command, meta metadata.Metadata) *cobra.Command {
 	if meta.ShortDescription == "" {
 		meta.ShortDescription = plugin.Short

cli-plugins/plugin/plugin_test.go

@@ -0,0 +1,28 @@
+package plugin
+
+import (
+	"slices"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestVisitAll(t *testing.T) {
+	root := &cobra.Command{Use: "root"}
+	sub1 := &cobra.Command{Use: "sub1"}
+	sub1sub1 := &cobra.Command{Use: "sub1sub1"}
+	sub1sub2 := &cobra.Command{Use: "sub1sub2"}
+	sub2 := &cobra.Command{Use: "sub2"}
+
+	root.AddCommand(sub1, sub2)
+	sub1.AddCommand(sub1sub1, sub1sub2)
+
+	var visited []string
+	visitAll(root, func(ccmd *cobra.Command) {
+		visited = append(visited, ccmd.Name())
+	})
+	expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
+	if !slices.Equal(expected, visited) {
+		t.Errorf("expected %#v, got %#v", expected, visited)
+	}
+}

cli/cobra.go

@@ -168,19 +168,25 @@ func (tcmd *TopLevelCommand) Initialize(ops ...command.CLIOption) error {
 }
 
 // VisitAll will traverse all commands from the root.
-// This is different from the VisitAll of cobra.Command where only parents
-// are checked.
+//
+// Deprecated: this utility was only used internally and will be removed in the next release.
 func VisitAll(root *cobra.Command, fn func(*cobra.Command)) {
+	visitAll(root, fn)
+}
+
+func visitAll(root *cobra.Command, fn func(*cobra.Command)) {
 	for _, cmd := range root.Commands() {
-		VisitAll(cmd, fn)
+		visitAll(cmd, fn)
 	}
 	fn(root)
 }
 
 // DisableFlagsInUseLine sets the DisableFlagsInUseLine flag on all
 // commands within the tree rooted at cmd.
+//
+// Deprecated: this utility was only used internally and will be removed in the next release.
 func DisableFlagsInUseLine(cmd *cobra.Command) {
-	VisitAll(cmd, func(ccmd *cobra.Command) {
+	visitAll(cmd, func(ccmd *cobra.Command) {
 		// do not add a `[flags]` to the end of the usage line.
 		ccmd.DisableFlagsInUseLine = true
 	})

cli/cobra_test.go

@@ -10,28 +10,6 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
-func TestVisitAll(t *testing.T) {
-	root := &cobra.Command{Use: "root"}
-	sub1 := &cobra.Command{Use: "sub1"}
-	sub1sub1 := &cobra.Command{Use: "sub1sub1"}
-	sub1sub2 := &cobra.Command{Use: "sub1sub2"}
-	sub2 := &cobra.Command{Use: "sub2"}
-
-	root.AddCommand(sub1, sub2)
-	sub1.AddCommand(sub1sub1, sub1sub2)
-
-	// Take the opportunity to test DisableFlagsInUseLine too
-	DisableFlagsInUseLine(root)
-
-	var visited []string
-	VisitAll(root, func(ccmd *cobra.Command) {
-		visited = append(visited, ccmd.Name())
-		assert.Assert(t, ccmd.DisableFlagsInUseLine, "DisableFlagsInUseLine not set on %q", ccmd.Name())
-	})
-	expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
-	assert.DeepEqual(t, expected, visited)
-}
-
 func TestVendorAndVersion(t *testing.T) {
 	// Non plugin.
 	assert.Equal(t, vendorAndVersion(&cobra.Command{Use: "test"}), "")

cmd/docker/docker.go

@@ -163,8 +163,11 @@ func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand {
 	cmd.SetOut(dockerCli.Out())
 	commands.AddCommands(cmd, dockerCli)
 
-	cli.DisableFlagsInUseLine(cmd)
-	setValidateArgs(dockerCli, cmd)
+	visitAll(cmd,
+		setValidateArgs(dockerCli),
+		// prevent adding "[flags]" to the end of the usage line.
+		func(c *cobra.Command) { c.DisableFlagsInUseLine = true },
+	)
 
 	// flags must be the top-level command flags, not cmd.Flags()
 	return cli.NewTopLevelCommand(cmd, dockerCli, opts, cmd.Flags())
@@ -265,14 +268,29 @@ func setHelpFunc(dockerCli command.Cli, cmd *cobra.Command) {
 	})
 }
 
-func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
-	// The Args is handled by ValidateArgs in cobra, which does not allows a pre-hook.
-	// As a result, here we replace the existing Args validation func to a wrapper,
-	// where the wrapper will check to see if the feature is supported or not.
-	// The Args validation error will only be returned if the feature is supported.
-	cli.VisitAll(cmd, func(ccmd *cobra.Command) {
+// visitAll traverses all commands from the root.
+func visitAll(root *cobra.Command, fns ...func(*cobra.Command)) {
+	for _, cmd := range root.Commands() {
+		visitAll(cmd, fns...)
+	}
+	for _, fn := range fns {
+		fn(root)
+	}
+}
+
+// The Args is handled by ValidateArgs in cobra, which does not allows a pre-hook.
+// As a result, here we replace the existing Args validation func to a wrapper,
+// where the wrapper will check to see if the feature is supported or not.
+// The Args validation error will only be returned if the feature is supported.
+func setValidateArgs(dockerCLI versionDetails) func(*cobra.Command) {
+	return func(ccmd *cobra.Command) {
 		// if there is no tags for a command or any of its parent,
 		// there is no need to wrap the Args validation.
+		//
+		// FIXME(thaJeztah): can we memoize properties of the parent?
+		//  visitAll traverses root -> all childcommands, and hasTags
+		//  goes the reverse (cmd -> visit all parents), so we may
+		//  end traversing two directions.
 		if !hasTags(ccmd) {
 			return
 		}
@@ -283,12 +301,12 @@ func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
 
 		cmdArgs := ccmd.Args
 		ccmd.Args = func(cmd *cobra.Command, args []string) error {
-			if err := isSupported(cmd, dockerCli); err != nil {
+			if err := isSupported(cmd, dockerCLI); err != nil {
 				return err
 			}
 			return cmdArgs(cmd, args)
 		}
-	})
+	}
 }
 
 func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command, subcommand string, envs []string) error {

cmd/docker/docker_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/cli/cli/debug"
 	platformsignals "github.com/docker/cli/cmd/docker/internal/signals"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
@@ -108,3 +109,21 @@ func TestUserTerminatedError(t *testing.T) {
 
 	assert.Equal(t, getExitCode(context.Cause(notifyCtx)), 143)
 }
+
+func TestVisitAll(t *testing.T) {
+	root := &cobra.Command{Use: "root"}
+	sub1 := &cobra.Command{Use: "sub1"}
+	sub1sub1 := &cobra.Command{Use: "sub1sub1"}
+	sub1sub2 := &cobra.Command{Use: "sub1sub2"}
+	sub2 := &cobra.Command{Use: "sub2"}
+
+	root.AddCommand(sub1, sub2)
+	sub1.AddCommand(sub1sub1, sub1sub2)
+
+	var visited []string
+	visitAll(root, func(ccmd *cobra.Command) {
+		visited = append(visited, ccmd.Name())
+	})
+	expected := []string{"sub1sub1", "sub1sub2", "sub1", "sub2", "root"}
+	assert.DeepEqual(t, expected, visited)
+}