From 7c76a73572f027824fd02b8ec2a8c049c3550a2e Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 19 Mar 2020 22:26:37 -0700
Subject: [PATCH] fix: normalize options when parsing them

We introduced a bug where we forgot to do this and couldn't find the definition
for the option.

This _also_ fixes (and tests) a bug where we'd allow duplicate flags in some
cases.
---
 cli/parse.go      | 23 ++++++++++++-----------
 cli/parse_test.go |  9 ++++++++-
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/cli/parse.go b/cli/parse.go
index ba35a0d..ae74d98 100644
--- a/cli/parse.go
+++ b/cli/parse.go
@@ -140,6 +140,7 @@ L:
 			if err != nil {
 				return err
 			}
+
 			kvType, err := getOptType(k, optDefs)
 			if err != nil {
 				return err // shouldn't happen b/c k,v was parsed from optsDef
@@ -409,17 +410,17 @@ func splitkv(opt string) (k, v string, ok bool) {
 	}
 }
 
-func parseOpt(opt, value string, opts map[string]cmds.Option) (interface{}, error) {
+func parseOpt(opt, value string, opts map[string]cmds.Option) (string, interface{}, error) {
 	optDef, ok := opts[opt]
 	if !ok {
-		return nil, fmt.Errorf("unknown option %q", opt)
+		return "", nil, fmt.Errorf("unknown option %q", opt)
 	}
 
 	v, err := optDef.Parse(value)
 	if err != nil {
-		return nil, err
+		return "", nil, err
 	}
-	return v, nil
+	return optDef.Name(), v, nil
 }
 
 type kv struct {
@@ -433,7 +434,7 @@ func (st *parseState) parseShortOpts(optDefs map[string]cmds.Option) ([]kv, erro
 
 	if ok {
 		// split at = successful
-		v, err := parseOpt(k, vStr, optDefs)
+		k, v, err := parseOpt(k, vStr, optDefs)
 		if err != nil {
 			return nil, err
 		}
@@ -453,7 +454,7 @@ func (st *parseState) parseShortOpts(optDefs map[string]cmds.Option) ([]kv, erro
 			case od.Type() == cmds.Bool:
 				// single char flags for bools
 				kvs = append(kvs, kv{
-					Key:   flag,
+					Key:   od.Name(),
 					Value: true,
 				})
 				j++
@@ -462,23 +463,23 @@ func (st *parseState) parseShortOpts(optDefs map[string]cmds.Option) ([]kv, erro
 				// single char flag for non-bools (use the rest of the flag as value)
 				rest := k[j+1:]
 
-				v, err := parseOpt(flag, rest, optDefs)
+				k, v, err := parseOpt(flag, rest, optDefs)
 				if err != nil {
 					return nil, err
 				}
 
-				kvs = append(kvs, kv{Key: flag, Value: v})
+				kvs = append(kvs, kv{Key: k, Value: v})
 				break LOOP
 
 			case st.i < len(st.cmdline)-1:
 				// single char flag for non-bools (use the next word as value)
 				st.i++
-				v, err := parseOpt(flag, st.cmdline[st.i], optDefs)
+				k, v, err := parseOpt(flag, st.cmdline[st.i], optDefs)
 				if err != nil {
 					return nil, err
 				}
 
-				kvs = append(kvs, kv{Key: flag, Value: v})
+				kvs = append(kvs, kv{Key: k, Value: v})
 				break LOOP
 
 			default:
@@ -507,7 +508,7 @@ func (st *parseState) parseLongOpt(optDefs map[string]cmds.Option) (string, inte
 		}
 	}
 
-	optval, err := parseOpt(k, v, optDefs)
+	k, optval, err := parseOpt(k, v, optDefs)
 	return k, optval, err
 }
 
diff --git a/cli/parse_test.go b/cli/parse_test.go
index 7fce8d3..247e5a3 100644
--- a/cli/parse_test.go
+++ b/cli/parse_test.go
@@ -80,6 +80,7 @@ func TestOptionParsing(t *testing.T) {
 	cmd := &cmds.Command{
 		Options: []cmds.Option{
 			cmds.StringOption("string", "s", "a string"),
+			cmds.StringOption("flag", "alias", "multiple long"),
 			cmds.BoolOption("bool", "b", "a bool"),
 			cmds.StringsOption("strings", "r", "strings array"),
 		},
@@ -156,6 +157,11 @@ func TestOptionParsing(t *testing.T) {
 	test("defaults", kvs{"opt": "def"}, words{})
 	test("defaults -o foo", kvs{"opt": "foo"}, words{})
 
+	test("--flag=foo", kvs{"flag": "foo"}, words{})
+	test("--alias=foo", kvs{"flag": "foo"}, words{})
+	testFail("--flag=bar --alias=foo")
+	testFail("--alias=bar --flag=foo")
+
 	testFail("--bad-flag")
 	testFail("--bad-flag=")
 	testFail("--bad-flag=xyz")
@@ -636,7 +642,8 @@ func TestFileArgs(t *testing.T) {
 			cmd: words{"fileOp", "--ignore", path.Base(tmpFile2.Name()), tmpDir1, tmpFile1.Name()}, f: nil,
 			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
 			parseErr: fmt.Errorf(notRecursiveFmtStr, tmpDir1, "r"),
-		}, {
+		},
+		{
 			cmd: words{"fileOp", tmpFile1.Name(), "--ignore", path.Base(tmpFile2.Name()), "--ignore"}, f: nil,
 			args:     words{tmpDir1, tmpFile1.Name(), tmpFile3.Name()},
 			parseErr: fmt.Errorf("missing argument for option %q", "ignore"),
-- 
GitLab