From fcb5e05c0706be6be5027c32691c29d61dc4e6d4 Mon Sep 17 00:00:00 2001
From: Bob Lail <bob.lailfamily@gmail.com>
Date: Fri, 5 Jul 2024 05:48:33 -0700
Subject: [PATCH] fix: When a Grammar combines flags with passthrough args, see
 if an unrecognized flag may be treated as a positional argument (#435)

* ci: Add a test for positional args that are passthrough on a command that isn't passthrough

* fix: When a Grammar combines flags with passthrough args, see if an unrecognized flag may be treated as a positional argument

Given a grammar like this:
```golang
var cli struct {
	Args []string `arg:"" optional:"" passthrough:""`
}
```

The first positional argument implies that it was preceded by `--`, so subsequent flags are not parsed.

If Kong parses `cli 1 --unknown 3`, it will populate `Args` with `[]string{"1", "--unknown", "3"}`.
However, if Kong parses `cli --unknown 2 3`, it will fail saying that `--unknown` is an unrecognized flag.

This commit changes the parser so that if an unknown flag _could_ be treated as the first passthrough argument, it is.

After this change, if Kong parses `cli --unknown 2 3`, it will populate `Args` with `[]string{"--unknown", "2", "3"}`.

* ci: Skip the `maintidx` linter for `trace()`
---
 .golangci.yml |  4 ++++
 context.go    | 26 +++++++++++++++++++++++---
 kong_test.go  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/.golangci.yml b/.golangci.yml
index e8980bf..2c5bfe3 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -87,3 +87,7 @@ issues:
     # Duplicate words are okay in tests.
     - linters: [dupword]
       path: _test\.go
+
+    - linters: [maintidx]
+      path: context\.go
+      text: 'Function name: trace'
diff --git a/context.go b/context.go
index b2bfea6..4f82813 100644
--- a/context.go
+++ b/context.go
@@ -420,12 +420,22 @@ func (c *Context) trace(node *Node) (err error) { //nolint: gocyclo
 
 		case FlagToken:
 			if err := c.parseFlag(flags, token.String()); err != nil {
-				return err
+				if isUnknownFlagError(err) && positional < len(node.Positional) && node.Positional[positional].Passthrough {
+					c.scan.Pop()
+					c.scan.PushTyped(token.String(), PositionalArgumentToken)
+				} else {
+					return err
+				}
 			}
 
 		case ShortFlagToken:
 			if err := c.parseFlag(flags, token.String()); err != nil {
-				return err
+				if isUnknownFlagError(err) && positional < len(node.Positional) && node.Positional[positional].Passthrough {
+					c.scan.Pop()
+					c.scan.PushTyped(token.String(), PositionalArgumentToken)
+				} else {
+					return err
+				}
 			}
 
 		case FlagValueToken:
@@ -728,9 +738,19 @@ func (c *Context) parseFlag(flags []*Flag, match string) (err error) {
 		c.Path = append(c.Path, &Path{Flag: flag})
 		return nil
 	}
-	return findPotentialCandidates(match, candidates, "unknown flag %s", match)
+	return &unknownFlagError{Cause: findPotentialCandidates(match, candidates, "unknown flag %s", match)}
 }
 
+func isUnknownFlagError(err error) bool {
+	var unknown *unknownFlagError
+	return errors.As(err, &unknown)
+}
+
+type unknownFlagError struct{ Cause error }
+
+func (e *unknownFlagError) Unwrap() error { return e.Cause }
+func (e *unknownFlagError) Error() string { return e.Cause.Error() }
+
 // Call an arbitrary function filling arguments with bound values.
 func (c *Context) Call(fn any, binds ...interface{}) (out []interface{}, err error) {
 	fv := reflect.ValueOf(fn)
diff --git a/kong_test.go b/kong_test.go
index 0e9fa9a..f90f86d 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -1526,6 +1526,54 @@ func TestEnumValidation(t *testing.T) {
 	}
 }
 
+func TestPassthroughArgs(t *testing.T) {
+	tests := []struct {
+		name    string
+		args    []string
+		flag    string
+		cmdArgs []string
+	}{
+		{
+			"NoArgs",
+			[]string{},
+			"",
+			[]string(nil),
+		},
+		{
+			"RecognizedFlagAndArgs",
+			[]string{"--flag", "foobar", "something"},
+			"foobar",
+			[]string{"something"},
+		},
+		{
+			"DashDashBeforeRecognizedFlag",
+			[]string{"--", "--flag", "foobar"},
+			"",
+			[]string{"--flag", "foobar"},
+		},
+		{
+			"UnrecognizedFlagAndArgs",
+			[]string{"--unrecognized-flag", "something"},
+			"",
+			[]string{"--unrecognized-flag", "something"},
+		},
+	}
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			var cli struct {
+				Flag string
+				Args []string `arg:"" optional:"" passthrough:""`
+			}
+			p := mustNew(t, &cli)
+			_, err := p.Parse(test.args)
+			assert.NoError(t, err)
+			assert.Equal(t, test.flag, cli.Flag)
+			assert.Equal(t, test.cmdArgs, cli.Args)
+		})
+	}
+}
+
 func TestPassthroughCmd(t *testing.T) {
 	tests := []struct {
 		name    string