Skip to content

Commit

Permalink
fix: check duplicated sub command name and alias
Browse files Browse the repository at this point in the history
  • Loading branch information
linrl3 committed Aug 20, 2023
1 parent b686f4f commit 8572933
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 0 deletions.
3 changes: 3 additions & 0 deletions app.go
Expand Up @@ -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...)
}

Expand Down
94 changes: 94 additions & 0 deletions app_test.go
Expand Up @@ -3087,3 +3087,97 @@ 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 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)
}
}
}

func Test_checkDuplicatedCmds_nilInput(t *testing.T) {
expect(t, checkDuplicatedCmds(nil), nil)
}
25 changes: 25 additions & 0 deletions command.go
@@ -1,6 +1,7 @@
package cli

import (
"errors"
"flag"
"fmt"
"reflect"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -404,3 +408,24 @@ 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
}

seen := make(map[string]struct{})
for _, c := range parent.Subcommands {
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
}
8 changes: 8 additions & 0 deletions helpers_test.go
Expand Up @@ -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))
}
}

0 comments on commit 8572933

Please sign in to comment.