Add a bunch of test cases for the parser and add benchmarks

.gitignore

@@ -9,3 +9,5 @@ devenv.local.nix
 # pre-commit
 .pre-commit-config.yaml
 
+coverage.html
+coverage.out

devenv.nix

@@ -16,14 +16,18 @@
     echo -n "Golang version: $(go version | cut -d ' ' -f 3), "
   '';
 
-  scripts.test.exec = ''
+  scripts.run-tests.exec = ''
     gotest ./...
   '';
 
   scripts.cover.exec = ''
     go test -race -cover -covermode=atomic -coverprofile=coverage.out ./... 
     go tool cover -html=coverage.out -o=coverage.html
-    open coverage.html
+  '';
+
+  scripts.cover-open.exec = ''
+    cover
+    xdg-open coverage.html &>/dev/null
   '';
 
   enterShell = "welcome-banner";

go.mod

@@ -1,3 +1,5 @@
 module go.fifitido.net/cmd
 
 go 1.21.3
+
+require github.com/spf13/pflag v1.0.5

go.sum

@@ -0,0 +1,2 @@
+github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
+github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=

opts/benchmarks_test.go

@@ -0,0 +1,52 @@
+package opts_test
+
+import (
+	"flag"
+	"testing"
+
+	"github.com/spf13/pflag"
+	"go.fifitido.net/cmd/opts"
+)
+
+var (
+	_ = flag.Int("port", 80, "port")
+	_ = flag.String("host", "localhost", "host")
+	_ = flag.Float64("float", 3.14, "float")
+	_ = flag.Bool("bool", true, "bool")
+
+	_ = pflag.IntP("port", "p", 80, "port")
+	_ = pflag.StringP("host", "h", "localhost", "host")
+	_ = pflag.Float64P("float", "f", 3.14, "float")
+	_ = pflag.BoolP("bool", "b", true, "bool")
+
+	io  = opts.Int("port", "p", 80, "port")
+	so  = opts.String("host", "h", "localhost", "host")
+	fo  = opts.Float("float", "f", 3.14, "float")
+	bo  = opts.Bool("bool", "b", true, "bool")
+	set = opts.Set{io, so, fo, bo}
+)
+
+func BenchmarkFlag(b *testing.B) {
+	args := []string{"-port", "8080", "-host", "localhost", "-float", "3.14", "-bool"}
+
+	for i := 0; i < b.N; i++ {
+		flag.CommandLine.Parse(args)
+	}
+}
+
+func BenchmarkPflag(b *testing.B) {
+	args := []string{"-p", "8080", "-h", "localhost", "-f", "3.14", "-b"}
+
+	for i := 0; i < b.N; i++ {
+		pflag.CommandLine.Parse(args)
+	}
+}
+
+func BenchmarkOpts(b *testing.B) {
+	args := []string{"-p", "8080", "-h", "localhost", "-f", "3.14", "-b"}
+
+	for i := 0; i < b.N; i++ {
+		parser := opts.NewParser(args, set, false)
+		parser.Parse()
+	}
+}

opts/parser.go

@@ -8,8 +8,7 @@ import (
 
 var (
 	ErrUnknownOption     = errors.New("unknown option")
-	ErrInvalidShortOption = errors.New("invalid short option")
+	ErrCannotChainOption = errors.New("cannot chain option")
-	ErrCannotChainOption  = errors.New("cannot chain option as it takes an argument")
 )
 
 type Parser struct {
@@ -20,7 +19,7 @@ type Parser struct {
 	ignoreUnknown bool
 }
 
-func NewParser(args []string, opts []Option, ignoreUnknown bool) *Parser {
+func NewParser(args []string, opts Set, ignoreUnknown bool) *Parser {
 	return &Parser{
 		opts: append(opts, globalOpts...),
 		args: args,
@@ -34,7 +33,7 @@ func (p *Parser) Parse() (restArgs []string, err error) {
 	for p.hasNext() {
 		arg := p.next()
 
-		if !strings.HasPrefix(arg, "-") { // Regular argument
+		if !strings.HasPrefix(arg, "-") || arg == "-" { // Regular argument
 			restArgs = append(restArgs, arg)
 		} else if arg == "--" { // Options terminator
 			restArgs = append(restArgs, p.restArgs()...)
@@ -69,15 +68,20 @@ func (p *Parser) next() string {
 	return p.args[p.curr]
 }
 
-func (p *Parser) value() string {
+func (p *Parser) peek() string {
-	p.curr++
+	if p.curr+1 >= len(p.args) {
-
-	if p.curr >= len(p.args) {
 		return ""
 	}
 
-	arg := p.args[p.curr]
+	return p.args[p.curr+1]
-	return arg
+}
+
+func (p *Parser) value() string {
+	if !p.hasNext() || (p.peek() != "-" && strings.HasPrefix(p.peek(), "-")) {
+		return ""
+	}
+
+	return p.next()
 }
 
 func (p *Parser) restArgs() []string {
@@ -89,13 +93,10 @@ func (p *Parser) restArgs() []string {
 
 func (p *Parser) parseLongOption(longName string) error {
 	value := ""
-
 	equals := strings.Index(longName, "=")
 	if equals >= 0 {
-		longName = longName[:equals]
 		value = longName[equals+1:]
-	} else {
+		longName = longName[:equals]
-		value = p.value()
 	}
 
 	opt, ok := p.opts.GetByLongName(longName)
@@ -107,6 +108,10 @@ func (p *Parser) parseLongOption(longName string) error {
 		return nil // Ignore unknown option. Continue parsing.
 	}
 
+	if value == "" && opt.TakesArg() {
+		value = p.value()
+	}
+
 	if err := opt.Parse(value); err != nil {
 		return err
 	}
@@ -115,9 +120,7 @@ func (p *Parser) parseLongOption(longName string) error {
 }
 
 func (p *Parser) parseShortOption(shortNames string) error {
-	if len(shortNames) == 0 {
+	if len(shortNames) == 1 {
-		return ErrInvalidShortOption
-	} else if len(shortNames) == 1 {
 		value := p.value()
 
 		opt, ok := p.opts.GetByShortName(shortNames)
@@ -148,7 +151,7 @@ func (p *Parser) parseShortOption(shortNames string) error {
 
 			if opt.TakesArg() {
 				if j > 0 {
-					return ErrCannotChainOption
+					return fmt.Errorf("%w: %s", ErrCannotChainOption, "-"+string(shortName))
 				}
 
 				value = shortNames[1:]

opts/parser_test.go

@@ -0,0 +1,267 @@
+package opts_test
+
+import (
+	"testing"
+
+	"go.fifitido.net/cmd/opts"
+)
+
+func TestParseUnknownLongOption(t *testing.T) {
+	set := opts.Set{}
+	args := []string{"--unknown"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "unknown option: --unknown" {
+		t.Errorf("Expected error: unknown option, got: %s", err.Error())
+	}
+}
+
+func TestParseUnknownShortOption(t *testing.T) {
+	set := opts.Set{}
+	args := []string{"-u"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "unknown option: -u" {
+		t.Errorf("Expected error: unknown option, got: %s", err.Error())
+	}
+}
+
+func TestParseUnknownShortChainedOption1(t *testing.T) {
+	set := opts.Set{
+		opts.Bool("banana", "b", false, ""),
+		opts.Bool("cucumber", "c", false, ""),
+	}
+	args := []string{"-abc"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "unknown option: -a" {
+		t.Errorf("Expected error: unknown option, got: %s", err.Error())
+	}
+}
+
+func TestParseUnknownShortChainedOption2(t *testing.T) {
+	set := opts.Set{
+		opts.Bool("apple", "a", false, ""),
+		opts.Bool("cucumber", "c", false, ""),
+	}
+	args := []string{"-abc"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "unknown option: -b" {
+		t.Errorf("Expected error: unknown option, got: %s", err.Error())
+	}
+}
+
+func TestParseUnknownShortChainedOption3(t *testing.T) {
+	set := opts.Set{
+		opts.Bool("apple", "a", false, ""),
+		opts.Bool("banana", "b", false, ""),
+	}
+	args := []string{"-abc"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "unknown option: -c" {
+		t.Errorf("Expected error: unknown option, got: %s", err.Error())
+	}
+}
+
+func TestParseUnchaninableOption(t *testing.T) {
+	set := opts.Set{
+		opts.Bool("apple", "a", false, ""),
+		opts.String("banana", "b", "", ""),
+	}
+	args := []string{"-ab"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err == nil {
+		t.Error("Expected error")
+	} else if err.Error() != "cannot chain option: -b" {
+		t.Errorf("Expected error: cannot chain option, got: %s", err.Error())
+	}
+}
+
+func TestParseShortOptionWithValueAndNoSpace(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"-fapple"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
+	}
+}
+
+func TestParseShortOptionWithValueAndSpace(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"-f", "apple"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
+	}
+}
+
+func TestParseShortOptionWithValueAndEqual(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"-f=apple"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
+	}
+}
+
+func TestParseLongOptionWithValueAndEqual(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"--fruit=apple"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
+	}
+}
+
+func TestParseLongOptionWithValueAndSpace(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"--fruit", "apple"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "apple" {
+		t.Errorf("Expected fruit to be 'apple', got: %s", opt.Value())
+	}
+}
+
+func TestParseOptionTerminator(t *testing.T) {
+	opt := opts.String("fruit", "f", "banana", "")
+	set := opts.Set{opt}
+	args := []string{"--fruit", "--", "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 len(restArgs) != 2 {
+		t.Errorf("Expected restArgs to be 2, got: %d", len(restArgs))
+	}
+
+	if restArgs[0] != "hello" {
+		t.Errorf("Expected restArgs[0] to be 'hello', got: %s", restArgs[0])
+	}
+
+	if restArgs[1] != "world" {
+		t.Errorf("Expected restArgs[1] to be 'world', got: %s", restArgs[1])
+	}
+}
+
+func TestParseLongOptionIgnoreUnknown(t *testing.T) {
+	set := opts.Set{}
+	args := []string{"--unknown", "hello", "world"}
+	parser := opts.NewParser(args, set, true)
+	restArgs, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if len(restArgs) != 2 {
+		t.Errorf("Expected restArgs to be 2, got: %d", len(restArgs))
+	}
+
+	if restArgs[0] != "hello" {
+		t.Errorf("Expected restArgs[0] to be 'hello', got: %s", restArgs[0])
+	}
+
+	if restArgs[1] != "world" {
+		t.Errorf("Expected restArgs[1] to be 'world', got: %s", restArgs[1])
+	}
+}
+
+func TestParseShortOptionIgnoreUnknown(t *testing.T) {
+	set := opts.Set{}
+	args := []string{"-q"}
+	parser := opts.NewParser(args, set, true)
+
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+}
+
+func TestParseChainedShortOptionIgnoreUnknown(t *testing.T) {
+	apple := opts.Bool("apple", "a", false, "")
+	banana := opts.Bool("banana", "b", false, "")
+	cucumber := opts.Bool("cucumber", "c", false, "")
+	set := opts.Set{apple, banana, cucumber}
+	args := []string{"-adc"}
+	parser := opts.NewParser(args, set, true)
+
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if apple.Value() != true {
+		t.Errorf("Expected apple to be true, got: %t", apple.Value())
+	}
+
+	if banana.Value() != false {
+		t.Errorf("Expected banana to be false, got: %t", banana.Value())
+	}
+
+	if cucumber.Value() != true {
+		t.Errorf("Expected cucumber to be true, got: %t", cucumber.Value())
+	}
+}
+
+func TestParseSingleDashValue(t *testing.T) {
+	opt := opts.String("fruit", "f", "", "")
+	set := opts.Set{opt}
+	args := []string{"-f-"}
+	parser := opts.NewParser(args, set, false)
+	_, err := parser.Parse()
+	if err != nil {
+		t.Errorf("Expected no error, got: %s", err.Error())
+	}
+
+	if opt.Value() != "-" {
+		t.Errorf("Expected fruit to be '-', got: %s", opt.Value())
+	}
+}