From 36ebd0a7b665c3b89ac0cd3d7bf414ffa10be60c Mon Sep 17 00:00:00 2001 From: linrunlong Date: Mon, 21 Aug 2023 23:01:21 +0800 Subject: [PATCH] fix: check duplicated sub command name and alias --- app.go | 3 ++ app_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ command.go | 16 +++++++++ helpers_test.go | 8 +++++ 4 files changed, 116 insertions(+) diff --git a/app.go b/app.go index 19b76c9b63..e07dd7b192 100644 --- a/app.go +++ b/app.go @@ -329,6 +329,9 @@ func (a *App) RunContext(ctx context.Context, arguments []string) (err error) { a.rootCommand = a.newRootCommand() cCtx.Command = a.rootCommand + if err := checkDuplicatedCmds(a.rootCommand); err != nil { + return err + } return a.rootCommand.Run(cCtx, arguments...) } diff --git a/app_test.go b/app_test.go index 981d074697..d07d2c0735 100644 --- a/app_test.go +++ b/app_test.go @@ -3087,3 +3087,92 @@ func TestFlagAction(t *testing.T) { }) } } + +func TestDuplicateSubcommand(t *testing.T) { + var testdata = []struct { + app *App + expectNoError bool + }{ + {&App{ + Name: "p1", + }, true}, + {&App{ + Name: "p2", + Commands: []*Command{}, + }, true}, + {&App{ + Name: "p3", + Commands: []*Command{{Name: "sub1"}}, + }, true}, + {&App{ + Name: "p4", + Commands: []*Command{{Name: "sub1"}, {Name: "sub1"}}, + }, false}, + {&App{ + Name: "p5", + Commands: []*Command{{Name: "sub1"}, {Name: "sub2", Aliases: []string{"sub1"}}}, + }, false}, + {&App{ + Name: "p6", + Commands: []*Command{{Name: "sub1"}, {Name: "sub2"}}, + }, true}, + } + for _, tt := range testdata { + err := tt.app.Run([]string{}) + if tt.expectNoError { + expect(t, err, nil) + } else { + expectNotEqual(t, err, nil) + } + + err = checkDuplicatedCmds(tt.app.rootCommand) + if tt.expectNoError { + expect(t, err, nil) + } else { + expectNotEqual(t, err, nil) + } + } + + var testNested = []struct { + app *App + subcommandTocheck string + expectNoError bool + }{ + { + &App{ + Name: "nested-0", + Commands: []*Command{ + {Name: "sub1", + Subcommands: []*Command{ + {Name: "sub1_a"}, + {Name: "sub1_b"}, + }, + }, + {Name: "sub2"}}, + }, + "sub1", + true}, + {&App{ + Name: "nested-1", + Commands: []*Command{ + {Name: "sub1", + Subcommands: []*Command{ + {Name: "sub1_a"}, + {Name: "sub1_a"}, + }, + }, + {Name: "sub2"}}, + }, + "sub1", + false}, + } + + for _, tt := range testNested { + err := tt.app.Run([]string{tt.app.Name, tt.subcommandTocheck}) + if tt.expectNoError { + expect(t, err, nil) + } else { + expectNotEqual(t, err, nil) + } + } +} diff --git a/command.go b/command.go index 69a0fdf6dc..c318da989b 100644 --- a/command.go +++ b/command.go @@ -149,6 +149,9 @@ func (c *Command) Run(cCtx *Context, arguments ...string) (err error) { if !c.isRoot { c.setup(cCtx) + if err := checkDuplicatedCmds(c); err != nil { + return err + } } a := args(arguments) @@ -404,3 +407,16 @@ func hasCommand(commands []*Command, command *Command) bool { return false } + +func checkDuplicatedCmds(parent *Command) error { + seen := make(map[string]struct{}) + for _, c := range parent.Subcommands { + for _, name := range c.Names() { + if _, exists := seen[name]; exists { + return fmt.Errorf("parent command [%s] has duplicated subcommand name or alias: %s", parent.Name, name) + } + seen[name] = struct{}{} + } + } + return nil +} diff --git a/helpers_test.go b/helpers_test.go index 9ecd8e18ae..30981e54f0 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -17,3 +17,11 @@ func expect(t *testing.T, a interface{}, b interface{}) { t.Errorf("Expected %v (type %v) - Got %v (type %v)", b, reflect.TypeOf(b), a, reflect.TypeOf(a)) } } + +func expectNotEqual(t *testing.T, a interface{}, b interface{}) { + t.Helper() + + if reflect.DeepEqual(a, b) { + t.Errorf("Expected not equal, but got: %v (type %v), %v (type %v) ", b, reflect.TypeOf(b), a, reflect.TypeOf(a)) + } +}