From 36ebd0a7b665c3b89ac0cd3d7bf414ffa10be60c Mon Sep 17 00:00:00 2001
From: linrunlong <linrunlong@bytedance.com>
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))
+	}
+}