From 03bb6779a6ad8349aaa2fc269fc72ce1366f301d Mon Sep 17 00:00:00 2001 From: Leo Antunes Date: Sun, 21 Jun 2026 00:59:37 +0200 Subject: [PATCH] Invoke Validate() on embedded structs and plugin elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context.Validate iterated c.Path and called isValidatable on each entry's target. Fields tagged embed:"" are flattened into their parent at build time and have no path entry, so any Validate() method on the embedded struct itself was never invoked. The same issue affects kong.Plugins, whose elements are also flattened. Extracts the embedded-field walk that already existed in getMethods into a shared walkEmbedded helper, used by both hooks (getMethods) and validators (new getValidators), so the rules for "what counts as embedded" live in one place. walkEmbedded also descends into Plugins elements, matching what flattenedFields does in build.go — this incidentally fixes the same latent bug for hooks on plugin structs. Co-Authored-By: Claude Opus 4.7 (1M context) --- callbacks.go | 43 ++++++++++++++++------------- context.go | 13 ++++++++- kong_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+), 20 deletions(-) diff --git a/callbacks.go b/callbacks.go index 6096a26..ecc5353 100644 --- a/callbacks.go +++ b/callbacks.go @@ -132,43 +132,48 @@ func getMethod(value reflect.Value, name string) reflect.Value { // // Returns a slice of bound methods that can be called directly. func getMethods(value reflect.Value, name string) (methods []reflect.Value) { - if value.Kind() == reflect.Ptr { + walkEmbedded(value, func(v reflect.Value) { + if method := getMethod(v, name); method.IsValid() { + methods = append(methods, method) + } + }) + return +} + +// walkEmbedded calls visit on v and recursively on every exported field +// of v that is either a standard Go anonymous field or tagged `embed:""`. +// Pointer values are dereferenced before traversal; nil/invalid pointers +// are skipped. [Plugins] are descended into element-by-element, matching how +// [flattenedFields] treats them at build time. +func walkEmbedded(value reflect.Value, visit func(reflect.Value)) { + if value.Kind() == reflect.Pointer { value = value.Elem() } if !value.IsValid() { return } - - if method := getMethod(value, name); method.IsValid() { - methods = append(methods, method) + visit(value) + if value.Type() == reflect.TypeOf(Plugins{}) { + for i := 0; i < value.Len(); i++ { + walkEmbedded(value.Index(i).Elem(), visit) + } + return } - if value.Kind() != reflect.Struct { return } - // If the current value is a struct, also consider embedded fields. - // Two kinds of embedded fields are considered if they're exported: - // - // - standard Go embedded fields - // - fields tagged with `embed:""` t := value.Type() for i := 0; i < value.NumField(); i++ { - fieldValue := value.Field(i) field := t.Field(i) - if !field.IsExported() { continue } - - // Consider a field embedded if it's actually embedded - // or if it's tagged with `embed:""`. _, isEmbedded := field.Tag.Lookup("embed") - isEmbedded = isEmbedded || field.Anonymous - if isEmbedded { - methods = append(methods, getMethods(fieldValue, name)...) + if !isEmbedded && !field.Anonymous { + continue } + walkEmbedded(value.Field(i), visit) } - return } func callFunction(f reflect.Value, bindings bindings) error { diff --git a/context.go b/context.go index 784e8cc..1e38bf5 100644 --- a/context.go +++ b/context.go @@ -232,7 +232,7 @@ func (c *Context) Validate() error { //nolint: gocyclo value = node.Target desc = node.Path() } - if validate := isValidatable(value); validate != nil { + for _, validate := range getValidators(value) { if err := validate.Validate(c); err != nil { if desc != "" { return fmt.Errorf("%s: %w", desc, err) @@ -1178,6 +1178,17 @@ func isValidatable(v reflect.Value) extendedValidatable { return nil } +// getValidators returns validators implemented by v and by any embedded fields, +// matching how hooks are discovered (see getMethods). +func getValidators(v reflect.Value) (validators []extendedValidatable) { + walkEmbedded(v, func(v reflect.Value) { + if validate := isValidatable(v); validate != nil { + validators = append(validators, validate) + } + }) + return +} + func atLeastOneEnvSet(envs []string) bool { for _, env := range envs { if _, ok := os.LookupEnv(env); ok { diff --git a/kong_test.go b/kong_test.go index 00add0a..ef2300c 100644 --- a/kong_test.go +++ b/kong_test.go @@ -1600,6 +1600,82 @@ func TestExtendedValidateFlag(t *testing.T) { assert.EqualError(t, err, "--flag: flag error") } +type embeddedValidate struct { + Flag string +} + +func (v *embeddedValidate) Validate() error { return errors.New("embedded error") } + +func TestValidateEmbed(t *testing.T) { + cli := struct { + Embedded embeddedValidate `embed:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{}) + assert.EqualError(t, err, "embedded error") +} + +func TestValidateEmbedOnCommand(t *testing.T) { + type cmd struct { + Embedded embeddedValidate `embed:""` + } + cli := struct { + Cmd cmd `cmd:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{"cmd"}) + assert.EqualError(t, err, "cmd: embedded error") +} + +type pluginValidate struct { + PluginFlag string +} + +func (v *pluginValidate) Validate() error { return errors.New("plugin error") } + +func TestValidatePlugin(t *testing.T) { + plugin := &pluginValidate{} + cli := struct { + Base string + kong.Plugins + }{ + Plugins: kong.Plugins{plugin}, + } + p := mustNew(t, &cli) + _, err := p.Parse([]string{}) + assert.EqualError(t, err, "plugin error") +} + +type nestedEmbedOuter struct { + Inner embeddedValidate `embed:""` +} + +func TestValidateNestedEmbed(t *testing.T) { + cli := struct { + Outer nestedEmbedOuter `embed:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{}) + assert.EqualError(t, err, "embedded error") +} + +type extendedEmbeddedValidate struct { + Flag string +} + +func (v *extendedEmbeddedValidate) Validate(kctx *kong.Context) error { + return errors.New("extended embedded error") +} + +func TestExtendedValidateEmbed(t *testing.T) { + cli := struct { + Embedded extendedEmbeddedValidate `embed:""` + }{} + p := mustNew(t, &cli) + _, err := p.Parse([]string{}) + assert.EqualError(t, err, "extended embedded error") +} + func TestPointers(t *testing.T) { cli := struct { Mapped *mappedValue