Skip to content

Commit 59826d1

Browse files
costelaclaude
andauthored
Invoke Validate() on embedded structs and plugin elements (#607)
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) <noreply@anthropic.com>
1 parent 6a2ac8d commit 59826d1

3 files changed

Lines changed: 112 additions & 20 deletions

File tree

callbacks.go

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -132,43 +132,48 @@ func getMethod(value reflect.Value, name string) reflect.Value {
132132
//
133133
// Returns a slice of bound methods that can be called directly.
134134
func getMethods(value reflect.Value, name string) (methods []reflect.Value) {
135-
if value.Kind() == reflect.Ptr {
135+
walkEmbedded(value, func(v reflect.Value) {
136+
if method := getMethod(v, name); method.IsValid() {
137+
methods = append(methods, method)
138+
}
139+
})
140+
return
141+
}
142+
143+
// walkEmbedded calls visit on v and recursively on every exported field
144+
// of v that is either a standard Go anonymous field or tagged `embed:""`.
145+
// Pointer values are dereferenced before traversal; nil/invalid pointers
146+
// are skipped. [Plugins] are descended into element-by-element, matching how
147+
// [flattenedFields] treats them at build time.
148+
func walkEmbedded(value reflect.Value, visit func(reflect.Value)) {
149+
if value.Kind() == reflect.Pointer {
136150
value = value.Elem()
137151
}
138152
if !value.IsValid() {
139153
return
140154
}
141-
142-
if method := getMethod(value, name); method.IsValid() {
143-
methods = append(methods, method)
155+
visit(value)
156+
if value.Type() == reflect.TypeOf(Plugins{}) {
157+
for i := 0; i < value.Len(); i++ {
158+
walkEmbedded(value.Index(i).Elem(), visit)
159+
}
160+
return
144161
}
145-
146162
if value.Kind() != reflect.Struct {
147163
return
148164
}
149-
// If the current value is a struct, also consider embedded fields.
150-
// Two kinds of embedded fields are considered if they're exported:
151-
//
152-
// - standard Go embedded fields
153-
// - fields tagged with `embed:""`
154165
t := value.Type()
155166
for i := 0; i < value.NumField(); i++ {
156-
fieldValue := value.Field(i)
157167
field := t.Field(i)
158-
159168
if !field.IsExported() {
160169
continue
161170
}
162-
163-
// Consider a field embedded if it's actually embedded
164-
// or if it's tagged with `embed:""`.
165171
_, isEmbedded := field.Tag.Lookup("embed")
166-
isEmbedded = isEmbedded || field.Anonymous
167-
if isEmbedded {
168-
methods = append(methods, getMethods(fieldValue, name)...)
172+
if !isEmbedded && !field.Anonymous {
173+
continue
169174
}
175+
walkEmbedded(value.Field(i), visit)
170176
}
171-
return
172177
}
173178

174179
func callFunction(f reflect.Value, bindings bindings) error {

context.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func (c *Context) Validate() error { //nolint: gocyclo
232232
value = node.Target
233233
desc = node.Path()
234234
}
235-
if validate := isValidatable(value); validate != nil {
235+
for _, validate := range getValidators(value) {
236236
if err := validate.Validate(c); err != nil {
237237
if desc != "" {
238238
return fmt.Errorf("%s: %w", desc, err)
@@ -1178,6 +1178,17 @@ func isValidatable(v reflect.Value) extendedValidatable {
11781178
return nil
11791179
}
11801180

1181+
// getValidators returns validators implemented by v and by any embedded fields,
1182+
// matching how hooks are discovered (see getMethods).
1183+
func getValidators(v reflect.Value) (validators []extendedValidatable) {
1184+
walkEmbedded(v, func(v reflect.Value) {
1185+
if validate := isValidatable(v); validate != nil {
1186+
validators = append(validators, validate)
1187+
}
1188+
})
1189+
return
1190+
}
1191+
11811192
func atLeastOneEnvSet(envs []string) bool {
11821193
for _, env := range envs {
11831194
if _, ok := os.LookupEnv(env); ok {

kong_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,6 +1600,82 @@ func TestExtendedValidateFlag(t *testing.T) {
16001600
assert.EqualError(t, err, "--flag: flag error")
16011601
}
16021602

1603+
type embeddedValidate struct {
1604+
Flag string
1605+
}
1606+
1607+
func (v *embeddedValidate) Validate() error { return errors.New("embedded error") }
1608+
1609+
func TestValidateEmbed(t *testing.T) {
1610+
cli := struct {
1611+
Embedded embeddedValidate `embed:""`
1612+
}{}
1613+
p := mustNew(t, &cli)
1614+
_, err := p.Parse([]string{})
1615+
assert.EqualError(t, err, "embedded error")
1616+
}
1617+
1618+
func TestValidateEmbedOnCommand(t *testing.T) {
1619+
type cmd struct {
1620+
Embedded embeddedValidate `embed:""`
1621+
}
1622+
cli := struct {
1623+
Cmd cmd `cmd:""`
1624+
}{}
1625+
p := mustNew(t, &cli)
1626+
_, err := p.Parse([]string{"cmd"})
1627+
assert.EqualError(t, err, "cmd: embedded error")
1628+
}
1629+
1630+
type pluginValidate struct {
1631+
PluginFlag string
1632+
}
1633+
1634+
func (v *pluginValidate) Validate() error { return errors.New("plugin error") }
1635+
1636+
func TestValidatePlugin(t *testing.T) {
1637+
plugin := &pluginValidate{}
1638+
cli := struct {
1639+
Base string
1640+
kong.Plugins
1641+
}{
1642+
Plugins: kong.Plugins{plugin},
1643+
}
1644+
p := mustNew(t, &cli)
1645+
_, err := p.Parse([]string{})
1646+
assert.EqualError(t, err, "plugin error")
1647+
}
1648+
1649+
type nestedEmbedOuter struct {
1650+
Inner embeddedValidate `embed:""`
1651+
}
1652+
1653+
func TestValidateNestedEmbed(t *testing.T) {
1654+
cli := struct {
1655+
Outer nestedEmbedOuter `embed:""`
1656+
}{}
1657+
p := mustNew(t, &cli)
1658+
_, err := p.Parse([]string{})
1659+
assert.EqualError(t, err, "embedded error")
1660+
}
1661+
1662+
type extendedEmbeddedValidate struct {
1663+
Flag string
1664+
}
1665+
1666+
func (v *extendedEmbeddedValidate) Validate(kctx *kong.Context) error {
1667+
return errors.New("extended embedded error")
1668+
}
1669+
1670+
func TestExtendedValidateEmbed(t *testing.T) {
1671+
cli := struct {
1672+
Embedded extendedEmbeddedValidate `embed:""`
1673+
}{}
1674+
p := mustNew(t, &cli)
1675+
_, err := p.Parse([]string{})
1676+
assert.EqualError(t, err, "extended embedded error")
1677+
}
1678+
16031679
func TestPointers(t *testing.T) {
16041680
cli := struct {
16051681
Mapped *mappedValue

0 commit comments

Comments
 (0)