Repository: jawher/mow.cli
Pull request #86 (2019-01-27 14:51:18)

Detect duplicate opt/arg names
And panic if such a case is detected

coveralls: 
[![Coverage Status](https://coveralls.io/builds/21297970/badge)](https://coveralls.io/builds/21297970)

Coverage decreased (-0.1%) to 95.89% when pulling **9be18261627b8794fdb6f973c7dc8b942a9bc600 on detect-errors** into **667b409e56614efc84e7c8937e01db7c8f772d40 on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/21297970/badge)](https://coveralls.io/builds/21297970)

Coverage decreased (-0.1%) to 95.89% when pulling **9be18261627b8794fdb6f973c7dc8b942a9bc600 on detect-errors** into **667b409e56614efc84e7c8937e01db7c8f772d40 on master**.


Patch:
diff --git a/args.go b/args.go
index faf3f2b..7234b13 100644
--- a/args.go
+++ b/args.go
@@ -2,6 +2,9 @@ package cli
 
 import (
 	"flag"
+	"fmt"
+
+	"github.com/jawher/mow.cli/internal/lexer"
 
 	"github.com/jawher/mow.cli/internal/container"
 	"github.com/jawher/mow.cli/internal/values"
@@ -205,8 +208,27 @@ func (c *Cmd) VarArg(name string, value flag.Value, desc string) {
 }
 
 func (c *Cmd) mkArg(arg container.Container) {
+	if !validArgName(arg.Name) {
+		panic(fmt.Sprintf("invalid argument name %q: must be in all caps", arg.Name))
+	}
+	if _, found := c.argsIdx[arg.Name]; found {
+		panic(fmt.Sprintf("duplicate argument name %q", arg.Name))
+	}
+
 	arg.ValueSetFromEnv = values.SetFromEnv(arg.Value, arg.EnvVar)
 
 	c.args = append(c.args, &arg)
 	c.argsIdx[arg.Name] = &arg
 }
+
+func validArgName(n string) bool {
+	tokens, err := lexer.Tokenize(n)
+	if err != nil {
+		return false
+	}
+	if len(tokens) != 1 {
+		return false
+	}
+
+	return tokens[0].Typ == lexer.TTArg
+}
diff --git a/options.go b/options.go
index da324d4..dc7084d 100644
--- a/options.go
+++ b/options.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"flag"
+	"fmt"
 	"strings"
 
 	"github.com/jawher/mow.cli/internal/container"
@@ -253,6 +254,9 @@ func (c *Cmd) mkOpt(opt container.Container) {
 
 	c.options = append(c.options, &opt)
 	for _, name := range opt.Names {
+		if _, found := c.optionsIdx[name]; found {
+			panic(fmt.Sprintf("duplicate option name %q", name))
+		}
 		c.optionsIdx[name] = &opt
 	}
 }


Test patch:
diff --git a/cli_test.go b/cli_test.go
index 112600e..bb6778c 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -91,7 +91,53 @@ func TestInvalidSpec(t *testing.T) {
 	app.Run([]string{"test", "-x", "-y", "hello"})
 
 	t.Fatalf("Should have panicked")
+}
+
+func TestDuplicateOptionName(t *testing.T) {
+	defer func() {
+		if p := recover(); p != nil {
+			t.Logf("Panicked with %v", p)
+			return
+		}
+		t.Fatalf("Should have panicked")
+	}()
+
+	app := App("test", "")
+	app.BoolOpt("f force", false, "")
+	app.StringOpt("f file", "", "")
+
+	t.Fatalf("Should have panicked")
+}
+
+func TestDuplicateArgName(t *testing.T) {
+	defer func() {
+		if p := recover(); p != nil {
+			t.Logf("Panicked with %v", p)
+			return
+		}
+		t.Fatalf("Should have panicked")
+	}()
+
+	app := App("test", "")
+	app.StringArg("ARG", "", "")
+	app.StringArg("ARG", "", "")
 
+	t.Fatalf("Should have panicked")
+}
+
+func TestInvalidArgName(t *testing.T) {
+	defer func() {
+		if p := recover(); p != nil {
+			t.Logf("Panicked with %v", p)
+			return
+		}
+		t.Fatalf("Should have panicked")
+	}()
+
+	app := App("test", "")
+	app.StringArg("arg", "", "")
+
+	t.Fatalf("Should have panicked")
 }
 
 func TestAppWithBoolOption(t *testing.T) {
@@ -485,7 +531,7 @@ func TestHelpMessage(t *testing.T) {
 
 	app.String(StringOpt{Name: "s str1", Value: "", EnvVar: "STR1", Desc: "String Option 1"})
 	app.String(StringOpt{Name: "str2", Value: "a value", Desc: "String Option 2"})
-	app.String(StringOpt{Name: "u", Value: "another value", EnvVar: "STR3", Desc: "String Option 3", HideValue: true})
+	app.String(StringOpt{Name: "v", Value: "another value", EnvVar: "STR3", Desc: "String Option 3", HideValue: true})
 
 	app.Int(IntOpt{Name: "i int1", Value: 0, EnvVar: "INT1 ALIAS_INT1"})
 	app.Int(IntOpt{Name: "int2", Value: 1, EnvVar: "INT2", Desc: "Int Option 2"})
@@ -497,7 +543,7 @@ func TestHelpMessage(t *testing.T) {
 
 	app.Ints(IntsOpt{Name: "q ints1", Value: nil, EnvVar: "INTS1", Desc: "Ints Option 1"})
 	app.Ints(IntsOpt{Name: "ints2", Value: []int{1, 2, 3}, EnvVar: "INTS2", Desc: "Ints Option 2"})
-	app.Ints(IntsOpt{Name: "s", Value: []int{1}, EnvVar: "INTS3", Desc: "Ints Option 3", HideValue: true})
+	app.Ints(IntsOpt{Name: "j", Value: []int{1}, EnvVar: "INTS3", Desc: "Ints Option 3", HideValue: true})
 
 	// Args
 	app.Bool(BoolArg{Name: "BOOL1", Value: false, EnvVar: "BOOL1", Desc: "Bool Argument 1"})
diff --git a/testdata/help-output.txt b/testdata/help-output.txt
index 1ac03c3..903748f 100644
--- a/testdata/help-output.txt
+++ b/testdata/help-output.txt
@@ -26,7 +26,7 @@ Options:
   -d            Bool Option 3 (env $BOOL3)
   -s, --str1    String Option 1 (env $STR1)
       --str2    String Option 2 (default "a value")
-  -u            String Option 3 (env $STR3)
+  -v            String Option 3 (env $STR3)
   -i, --int1    (env $INT1, $ALIAS_INT1) (default 0)
       --int2    Int Option 2 (env $INT2) (default 1)
   -k            Int Option 3 (env $INT3)
@@ -35,7 +35,7 @@ Options:
   -z            Strings Option 3 (env $STRS3)
   -q, --ints1   Ints Option 1 (env $INTS1)
       --ints2   Ints Option 2 (env $INTS2) (default [1, 2, 3])
-  -s            Ints Option 3 (env $INTS3)
+  -j            Ints Option 3 (env $INTS3)
                 
 Commands:       
   command1      command1 description