From f63f2c4d726d862389e5ebefb8165be3b6c1a1d0 Mon Sep 17 00:00:00 2001 From: hexonal Date: Wed, 22 Jul 2026 01:51:31 -0400 Subject: [PATCH] Validate flags/positionals before the commands/nodes that own them Context.Validate() iterated c.Path in trace order and called each element's Validate() as encountered. Since a command's own Path entry is appended before its flags' entries (flags are parsed after the command name in the token stream), a command's Validate() ran before its flags' Validate() - so a command validator couldn't rely on its own flags already being validated. A node's own flags/positionals always appear on the path right after that node, before the next node entry. So each node/command's own validation is now deferred until that boundary (or the end of the path) is reached, running it right after its own flags/positionals rather than before them. This only reorders each node relative to its own directly- owned flags; the relative order between different nodes on the path (e.g. a parent command still validates before its child command) is unaffected. Fixes #611. --- context.go | 38 ++++++++++++++++++++++++++++++++++- kong_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) 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