fix: check duplicated sub command name and alias

Author: linrl3

app.go

@@ -314,7 +314,6 @@ func (a *App) Run(arguments []string) (err error) {
 // propagate timeouts and cancellation requests
 func (a *App) RunContext(ctx context.Context, arguments []string) (err error) {
 	a.Setup()
-
 	// handle the completion flag separately from the flagset since
 	// completion could be attempted after a flag, but before its value was put
 	// on the command line. this causes the flagset to interpret the completion
@@ -328,7 +327,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...)
 }
 

app_test.go

@@ -3087,3 +3087,93 @@ 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: "p7",
+				Commands: []*Command{
+					{Name: "sub1",
+						Subcommands: []*Command{
+							{Name: "sub1_a"},
+							{Name: "sub1_b"},
+						},
+					},
+					{Name: "sub2"}},
+			},
+			"sub1",
+			true},
+		{&App{
+			Name: "p8",
+			Commands: []*Command{
+				{Name: "sub1",
+					Subcommands: []*Command{
+						{Name: "sub1_a"},
+						{Name: "sub1_a"},
+					},
+				},
+				{Name: "sub2"}},
+		},
+			"sub1",
+			false},
+	}
+
+	for index, tt := range testNested {
+		err := tt.app.Run([]string{tt.app.Name, tt.subcommandTocheck})
+		if tt.expectNoError {
+			expect(t, err, nil)
+		} else {
+			fmt.Println(index)
+			expectNotEqual(t, err, nil)
+		}
+	}
+}

command.go

@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"reflect"
@@ -149,6 +150,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 +408,29 @@ func hasCommand(commands []*Command, command *Command) bool {
 
 	return false
 }
+
+func checkDuplicatedCmds(parent *Command) error {
+	if parent == nil {
+		return nil
+	}
+	if len(parent.Subcommands) == 0 {
+		return nil
+	}
+
+	var (
+		seen = make(map[string]struct{})
+	)
+	for _, c := range parent.Subcommands {
+		if c == nil {
+			continue
+		}
+		for _, name := range c.Names() {
+			if _, exists := seen[name]; exists {
+				errMsg := fmt.Sprintf("parent command [%s] has duplicated subcommand name or alias: %s", parent.Name, name)
+				return errors.New(errMsg)
+			}
+			seen[name] = struct{}{}
+		}
+	}
+	return nil
+}

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))
+	}
+}