Skip to content

Commit a107ee4

Browse files
authored
Merge pull request #2274 from siutsin/fix-MutuallyExclusiveFlags
Fix: check MutuallyExclusiveFlags across parent command chain
2 parents f109a21 + 4d68d26 commit a107ee4

File tree

2 files changed

+100
-35
lines changed

2 files changed

+100
-35
lines changed

command_run.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -237,14 +237,18 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context
237237
}()
238238
}
239239

240-
for _, grp := range cmd.MutuallyExclusiveFlags {
241-
if err := grp.check(cmd); err != nil {
242-
if cmd.OnUsageError != nil {
243-
err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil)
244-
} else {
245-
_ = ShowSubcommandHelp(cmd)
240+
// Walk the parent chain to check mutually exclusive flag groups
241+
// defined on ancestor commands, since persistent flags are inherited.
242+
for pCmd := cmd; pCmd != nil; pCmd = pCmd.parent {
243+
for _, grp := range pCmd.MutuallyExclusiveFlags {
244+
if err := grp.check(cmd); err != nil {
245+
if cmd.OnUsageError != nil {
246+
err = cmd.OnUsageError(ctx, cmd, err, cmd.parent != nil)
247+
} else {
248+
_ = ShowSubcommandHelp(cmd)
249+
}
250+
return ctx, err
246251
}
247-
return ctx, err
248252
}
249253
}
250254

command_test.go

Lines changed: 89 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5445,39 +5445,100 @@ func TestCommand_ParallelRun(t *testing.T) {
54455445
}
54465446
}
54475447

5448-
func TestCommand_ExclusiveFlagsPersistentPropagation(t *testing.T) {
5449-
var subCmdAlphaValue string
5448+
func TestCommand_ExclusiveFlagsPersistent(t *testing.T) {
5449+
exclusiveGroup := func(flags ...string) []MutuallyExclusiveFlags {
5450+
grp := MutuallyExclusiveFlags{}
5451+
for _, name := range flags {
5452+
grp.Flags = append(grp.Flags, []Flag{&StringFlag{Name: name}})
5453+
}
5454+
return []MutuallyExclusiveFlags{grp}
5455+
}
54505456

5451-
cmd := &Command{
5452-
Name: "root",
5453-
MutuallyExclusiveFlags: []MutuallyExclusiveFlags{
5454-
{
5455-
Flags: [][]Flag{
5456-
{
5457-
&StringFlag{
5458-
Name: "alpha",
5459-
},
5460-
},
5461-
{
5462-
&StringFlag{
5463-
Name: "beta",
5464-
},
5465-
},
5466-
},
5457+
noop := func(_ context.Context, _ *Command) error { return nil }
5458+
5459+
newBaseCmd := func() *Command {
5460+
return &Command{
5461+
Name: "root",
5462+
MutuallyExclusiveFlags: exclusiveGroup("alpha", "beta"),
5463+
Commands: []*Command{{Name: "sub", Action: noop}},
5464+
}
5465+
}
5466+
5467+
tests := []struct {
5468+
name string
5469+
setup func() *Command
5470+
args []string
5471+
wantErr string
5472+
}{
5473+
{
5474+
name: "single flag propagated to subcommand",
5475+
setup: newBaseCmd,
5476+
args: []string{"root", "sub", "--alpha", "hello"},
5477+
},
5478+
{
5479+
name: "both exclusive flags on subcommand errors",
5480+
setup: newBaseCmd,
5481+
args: []string{"root", "sub", "--alpha", "hello", "--beta", "world"},
5482+
wantErr: "cannot be set along with",
5483+
},
5484+
{
5485+
name: "neither flag set without required is ok",
5486+
setup: newBaseCmd,
5487+
args: []string{"root", "sub"},
5488+
},
5489+
{
5490+
name: "exclusive flags checked on grandchild",
5491+
setup: func() *Command {
5492+
cmd := newBaseCmd()
5493+
sub := cmd.Commands[0]
5494+
sub.Name = "mid"
5495+
sub.Action = nil
5496+
sub.Commands = []*Command{{Name: "leaf", Action: noop}}
5497+
return cmd
5498+
},
5499+
args: []string{"root", "mid", "leaf", "--alpha", "hello", "--beta", "world"},
5500+
wantErr: "cannot be set along with",
5501+
},
5502+
{
5503+
name: "subcommand own group checked alongside parent group",
5504+
setup: func() *Command {
5505+
cmd := newBaseCmd()
5506+
cmd.Commands[0].MutuallyExclusiveFlags = exclusiveGroup("gamma", "delta")
5507+
return cmd
54675508
},
5509+
args: []string{"root", "sub", "--gamma", "hello", "--delta", "world"},
5510+
wantErr: "cannot be set along with",
54685511
},
5469-
Commands: []*Command{
5470-
{
5471-
Name: "sub",
5472-
Action: func(_ context.Context, cmd *Command) error {
5473-
subCmdAlphaValue = cmd.String("alpha")
5474-
return nil
5475-
},
5512+
{
5513+
name: "parent group violation detected when subcommand has own group",
5514+
setup: func() *Command {
5515+
cmd := newBaseCmd()
5516+
cmd.Commands[0].MutuallyExclusiveFlags = exclusiveGroup("gamma", "delta")
5517+
return cmd
5518+
},
5519+
args: []string{"root", "sub", "--alpha", "hello", "--beta", "world"},
5520+
wantErr: "cannot be set along with",
5521+
},
5522+
{
5523+
name: "parent and subcommand groups both pass independently",
5524+
setup: func() *Command {
5525+
cmd := newBaseCmd()
5526+
cmd.Commands[0].MutuallyExclusiveFlags = exclusiveGroup("gamma", "delta")
5527+
return cmd
54765528
},
5529+
args: []string{"root", "sub", "--alpha", "hello", "--gamma", "world"},
54775530
},
54785531
}
54795532

5480-
err := cmd.Run(buildTestContext(t), []string{"root", "sub", "--alpha", "hello"})
5481-
require.NoError(t, err)
5482-
assert.Equal(t, "hello", subCmdAlphaValue)
5533+
for _, tt := range tests {
5534+
t.Run(tt.name, func(t *testing.T) {
5535+
err := tt.setup().Run(buildTestContext(t), tt.args)
5536+
if tt.wantErr != "" {
5537+
require.Error(t, err)
5538+
assert.Contains(t, err.Error(), tt.wantErr)
5539+
} else {
5540+
require.NoError(t, err)
5541+
}
5542+
})
5543+
}
54835544
}

0 commit comments

Comments
 (0)