From 8ab994ef4d9f395f3451ad894d4f5e6b272c6f68 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Jul 2026 00:46:01 -0500 Subject: [PATCH] refactor(cmd): use cobra MarkFlags for declarative flag validation Replace manual flag-group validation with cobra's built-in helpers: - MarkFlagsOneRequired in config/read and internal/run-user-commands - MarkFlagsMutuallyExclusive for --push/--skip-push in workspace/build --- cmd/config/read.go | 10 ++-------- cmd/internal/runusercommands.go | 9 ++------- cmd/internal/runusercommands_test.go | 8 ++------ cmd/workspace/build.go | 5 +---- cmd/workspace/build_test.go | 8 ++++++++ 5 files changed, 15 insertions(+), 25 deletions(-) diff --git a/cmd/config/read.go b/cmd/config/read.go index 243e66041..5eb2a0077 100644 --- a/cmd/config/read.go +++ b/cmd/config/read.go @@ -59,6 +59,8 @@ func NewReadCmd(f *flags.GlobalFlags) *cobra.Command { "Include the merged configuration in the output"), ) + readConfigCmd.MarkFlagsOneRequired(names.WorkspaceFolder, names.ContainerID, names.IDLabel) + return readConfigCmd } @@ -122,14 +124,6 @@ func (cmd *ReadCmd) resolve(ctx context.Context) ( string, error, ) { - if cmd.ContainerID == "" && cmd.WorkspaceFolder == "" && len(cmd.IDLabels) == 0 { - return nil, "", fmt.Errorf( - "either %s, %s, or %s must be provided", - names.Flag(names.WorkspaceFolder), - names.Flag(names.ContainerID), - names.Flag(names.IDLabel), - ) - } if cmd.ContainerID != "" { return cmd.resolveConfigFromContainer(ctx) } diff --git a/cmd/internal/runusercommands.go b/cmd/internal/runusercommands.go index f8bd26c73..4b7b2742c 100644 --- a/cmd/internal/runusercommands.go +++ b/cmd/internal/runusercommands.go @@ -137,6 +137,8 @@ func NewRunUserCommandsCmd(f *flags.GlobalFlags) *cobra.Command { ), ) + runCmd.MarkFlagsOneRequired(names.WorkspaceFolder, names.ContainerID) + return runCmd } @@ -180,13 +182,6 @@ func (cmd *RunUserCommandsCmd) Run(ctx context.Context) error { } func (cmd *RunUserCommandsCmd) validate() error { - if cmd.WorkspaceFolder == "" && cmd.ContainerID == "" { - return fmt.Errorf( - "either %s or %s must be provided", - names.Flag(names.WorkspaceFolder), - names.Flag(names.ContainerID), - ) - } if cmd.ContainerID != "" && cmd.WorkspaceFolder == "" && cmd.Config == "" { return fmt.Errorf( "--config is required when --container-id is used without --workspace-folder", diff --git a/cmd/internal/runusercommands_test.go b/cmd/internal/runusercommands_test.go index 142f031ca..357a280ca 100644 --- a/cmd/internal/runusercommands_test.go +++ b/cmd/internal/runusercommands_test.go @@ -56,7 +56,8 @@ func TestNewRunUserCommandsCmd_RequiresWorkspaceFolderOrContainerID(t *testing.T cmd.SetArgs([]string{}) err := cmd.Execute() assert.Error(t, err) - assert.Contains(t, err.Error(), "either --workspace-folder or --container-id must be provided") + assert.Contains(t, err.Error(), + "at least one of the flags in the group [workspace-folder container-id] is required") } func TestNewRunUserCommandsCmd_ContainerIDWithoutConfigFails(t *testing.T) { @@ -297,11 +298,6 @@ func TestRunUserCommandsCmd_Validate(t *testing.T) { false, "", }, - { - "neither provided", - &RunUserCommandsCmd{GlobalFlags: &flags.GlobalFlags{}}, - true, "either --workspace-folder or --container-id", - }, { "container-id without config or workspace", &RunUserCommandsCmd{GlobalFlags: &flags.GlobalFlags{}, ContainerID: testContainerID}, diff --git a/cmd/workspace/build.go b/cmd/workspace/build.go index b59802212..31c1ebf59 100644 --- a/cmd/workspace/build.go +++ b/cmd/workspace/build.go @@ -120,6 +120,7 @@ func (cmd *BuildCmd) registerImageFlags(buildCmd *cobra.Command) { cliflags.Bool(&cmd.Pull, names.Pull, false, "Always attempt to pull a newer version of the base image when building"), ) + buildCmd.MarkFlagsMutuallyExclusive(names.Push, names.SkipPush) } func (cmd *BuildCmd) registerTestingFlags(buildCmd *cobra.Command) { @@ -194,10 +195,6 @@ func (cmd *BuildCmd) prepareBuild(ctx context.Context) (*config.Config, error) { } func (cmd *BuildCmd) validateBuildFlags() error { - if cmd.PushDuringBuild && cmd.SkipPush { - return fmt.Errorf("cannot use %s and %s together", - names.Flag(names.Push), names.Flag(names.SkipPush)) - } if cmd.PushDuringBuild && cmd.Repository == "" { return fmt.Errorf("%s requires %s to be specified", names.Flag(names.Push), names.Flag(names.Repository)) diff --git a/cmd/workspace/build_test.go b/cmd/workspace/build_test.go index a43a1f5f5..b9481ff49 100644 --- a/cmd/workspace/build_test.go +++ b/cmd/workspace/build_test.go @@ -59,6 +59,14 @@ func TestBuildCmd_NoBuildFlagParsesValue(t *testing.T) { assert.True(t, val) } +func TestBuildCmd_PushAndSkipPushMutuallyExclusive(t *testing.T) { + buildCmd := NewBuildCmd(&flags.GlobalFlags{}) + buildCmd.SetArgs([]string{"--" + names.Push, "--" + names.SkipPush}) + err := buildCmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "none of the others can be") +} + func TestBuildCmd_NoCacheDefaultFalse(t *testing.T) { buildCmd := NewBuildCmd(&flags.GlobalFlags{}) err := buildCmd.ParseFlags([]string{})