Skip to content

Commit

Permalink
fix: When a Grammar combines flags with passthrough args, see if an u…
Browse files Browse the repository at this point in the history
…nrecognized 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()`
  • Loading branch information
boblail authored Jul 5, 2024
1 parent d315006 commit fcb5e05
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 3 deletions.
4 changes: 4 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
26 changes: 23 additions & 3 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
48 changes: 48 additions & 0 deletions kong_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit fcb5e05

Please sign in to comment.