diff --git a/context.go b/context.go index 7ce7d7e..2a62e51 100644 --- a/context.go +++ b/context.go @@ -204,7 +204,7 @@ func (c *Context) Validate() error { //nolint: gocyclo } } } - for _, el := range c.Path { + validateEl := func(el *Path) error { var ( value reflect.Value desc string @@ -234,6 +234,42 @@ func (c *Context) Validate() error { //nolint: gocyclo return err } } + return nil + } + // Each command/node's own flags and positionals are validated immediately + // before that command/node itself, so its Validate() runs against + // already-validated flag values rather than raw input. A node's own + // flags/positionals appear right after it on the path, so its validation + // is deferred until the next node boundary (or the end of the path), + // preserving the original relative order between different nodes. + // + // Note: Resolve() appends resolver-derived flag values to the end of + // c.Path without recording which node each one belongs to, so a node + // whose own resolved (not command-line-supplied) flags come from a + // Resolver can end up deferred past other nodes' resolved flags too. + var pendingNode *Path + flushPendingNode := func() error { + if pendingNode == nil { + return nil + } + node := pendingNode + pendingNode = nil + return validateEl(node) + } + for _, el := range c.Path { + if el.Flag != nil || el.Positional != nil { + if err := validateEl(el); err != nil { + return err + } + continue + } + if err := flushPendingNode(); err != nil { + return err + } + pendingNode = el + } + if err := flushPendingNode(); err != nil { + return err } for _, resolver := range c.combineResolvers() { if err := resolver.Validate(c.Model); err != nil { diff --git a/kong_test.go b/kong_test.go index 34e89c9..41f55a0 100644 --- a/kong_test.go +++ b/kong_test.go @@ -1618,6 +1618,63 @@ func TestValidateCmd(t *testing.T) { assert.EqualError(t, err, "cmd: cmd error") } +var validateOrderLog []string + +type validateOrderCmd struct { + Flag validateOrderFlag +} + +func (v *validateOrderCmd) Validate() error { + validateOrderLog = append(validateOrderLog, "cmd") + return nil +} + +type validateOrderFlag string + +func (v *validateOrderFlag) Validate() error { + validateOrderLog = append(validateOrderLog, "flag") + return nil +} + +// A command's Validate() should run against already-validated flag values, +// so its own flags must be validated first. +func TestValidateCmdRunsAfterItsFlags(t *testing.T) { + validateOrderLog = nil + cli := struct { + Cmd validateOrderCmd `cmd:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{"cmd", "--flag=value"}) + assert.NoError(t, err) + assert.Equal(t, []string{"flag", "cmd"}, validateOrderLog) +} + +type validateOrderParent struct { + ParentFlag validateOrderFlag + Cmd validateOrderCmd `cmd:""` +} + +func (v *validateOrderParent) Validate() error { + validateOrderLog = append(validateOrderLog, "parent") + return nil +} + +// Each command's own flags validate immediately before it, and the relative +// order between different commands on the path (app before parent before +// cmd) is unaffected - a command's own flags don't get held back by an +// unrelated descendant command's flags, or vice versa. +func TestValidateNestedCmdRunsAfterOwnFlagsOnly(t *testing.T) { + validateOrderLog = nil + cli := struct { + AppFlag validateOrderFlag + Parent validateOrderParent `cmd:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{"--app-flag=a", "parent", "--parent-flag=b", "cmd", "--flag=c"}) + assert.NoError(t, err) + assert.Equal(t, []string{"flag", "flag", "parent", "flag", "cmd"}, validateOrderLog) +} + func TestValidateFlag(t *testing.T) { cli := struct { Flag validateFlag