From b8a7d7243321d1adaf116368872b41a86f7da082 Mon Sep 17 00:00:00 2001
From: Evan Fiordeliso <evan.fiordeliso@gmail.com>
Date: Sun, 12 Nov 2023 17:06:43 -0500
Subject: [PATCH] Return error when value is not specified and add more test
 cases

---
 opts/parser.go      | 26 ++++++++++++++++----------
 opts/parser_test.go | 28 +++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 13 deletions(-)

diff --git a/opts/parser.go b/opts/parser.go
index 0b8c900..d9b35ba 100644
--- a/opts/parser.go
+++ b/opts/parser.go
@@ -7,8 +7,9 @@ import (
 )
 
 var (
-	ErrUnknownOption     = errors.New("unknown option")
-	ErrCannotChainOption = errors.New("cannot chain option")
+	ErrUnknownOption       = errors.New("unknown option")
+	ErrCannotChainOption   = errors.New("cannot chain option")
+	ErrOptionRequiresValue = errors.New("option requires value")
 )
 
 type Parser struct {
@@ -76,12 +77,12 @@ func (p *Parser) peek() string {
 	return p.args[p.curr+1]
 }
 
-func (p *Parser) value() string {
+func (p *Parser) value() (string, bool) {
 	if !p.hasNext() || (p.peek() != "-" && strings.HasPrefix(p.peek(), "-")) {
-		return ""
+		return "", false
 	}
 
-	return p.next()
+	return p.next(), true
 }
 
 func (p *Parser) restArgs() []string {
@@ -109,7 +110,11 @@ func (p *Parser) parseLongOption(longName string) error {
 	}
 
 	if value == "" && opt.TakesArg() {
-		value = p.value()
+		var ok bool
+		value, ok = p.value()
+		if !ok {
+			return fmt.Errorf("%w: %s", ErrOptionRequiresValue, "--"+longName)
+		}
 	}
 
 	if err := opt.Parse(value); err != nil {
@@ -121,7 +126,7 @@ func (p *Parser) parseLongOption(longName string) error {
 
 func (p *Parser) parseShortOption(shortNames string) error {
 	if len(shortNames) == 1 {
-		value := p.value()
+		value, valOk := p.value()
 
 		opt, ok := p.opts.GetByShortName(shortNames)
 		if !ok {
@@ -132,6 +137,10 @@ func (p *Parser) parseShortOption(shortNames string) error {
 			return nil // Ignore unknown option. Continue parsing.
 		}
 
+		if !valOk && opt.TakesArg() {
+			return fmt.Errorf("%w: %s", ErrOptionRequiresValue, "-"+shortNames)
+		}
+
 		if err := opt.Parse(value); err != nil {
 			return err
 		}
@@ -156,9 +165,6 @@ func (p *Parser) parseShortOption(shortNames string) error {
 
 				value = shortNames[1:]
 				value = strings.TrimPrefix(value, "=")
-				if len(value) == 0 {
-					value = p.value()
-				}
 				j = len(shortNames)
 			}
 
diff --git a/opts/parser_test.go b/opts/parser_test.go
index 96fb5eb..036a84e 100644
--- a/opts/parser_test.go
+++ b/opts/parser_test.go
@@ -168,15 +168,15 @@ func TestParseLongOptionWithValueAndSpace(t *testing.T) {
 func TestParseOptionTerminator(t *testing.T) {
 	opt := opts.String("fruit", "f", "banana", "")
 	set := opts.Set{opt}
-	args := []string{"--fruit", "--", "hello", "world"}
+	args := []string{"--fruit", "apple", "--", "hello", "world"}
 	parser := opts.NewParser(args, set, false)
 	restArgs, err := parser.Parse()
 	if err != nil {
 		t.Errorf("Expected no error, got: %s", err.Error())
 	}
 
-	if opt.Value() != "banana" {
-		t.Errorf("Expected fruit to be 'banana', got: %s", opt.Value())
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
 	}
 
 	if len(restArgs) != 2 {
@@ -311,3 +311,25 @@ func TestParseShortOptionWithEqualBadValue(t *testing.T) {
 		t.Error("Expected error")
 	}
 }
+
+func TestParseLongOptionMissingValue(t *testing.T) {
+	opt := opts.Int("fruit", "f", 0, "")
+	set := opts.Set{opt}
+	args := []string{"--fruit"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	}
+}
+
+func TestParseShortOptionMissingValue(t *testing.T) {
+	opt := opts.Int("fruit", "f", 0, "")
+	set := opts.Set{opt}
+	args := []string{"-f"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	}
+}