applyconfiguration-gen: handle conflicting members

With embedded structs, objects can end up with conflicting members.
These aren't ambiguous in Go, but applyconfiguration-gen's generated
functions ignore this possibility and produce ambiguous member
accesses and duplicate methods.

This resolves the problem by qualifying embedded member accesses with
their types when appropriate, and by memoizing generated function
names to avoid duplicates. When conflicting members are encountered,
functions are generated for the first one; this is assumed to produce
correct results.

For applyconfiguration-gen, a more correct approach would be to only
generate With... and Get... functions for the specific members the
generator is interested in; but that would be a breaking change for
any client code relying on the other With... functions.

Signed-off-by: Stephen Kitt <skitt@redhat.com>
This commit is contained in:
Stephen Kitt 2024-08-29 18:03:30 +02:00
parent 63439abb40
commit 68bad250b6
No known key found for this signature in database
GPG Key ID: 1CC5FA453662A71D

View File

@ -19,6 +19,7 @@ package generators
import ( import (
"io" "io"
"path" "path"
"slices"
"strings" "strings"
"k8s.io/gengo/v2/generator" "k8s.io/gengo/v2/generator"
@ -114,7 +115,7 @@ func (g *applyConfigurationGenerator) GenerateType(c *generator.Context, t *type
sw.Do(constructor, typeParams) sw.Do(constructor, typeParams)
} }
} }
g.generateWithFuncs(t, typeParams, sw, nil) g.generateWithFuncs(t, typeParams, sw, nil, &[]string{})
g.generateGetters(t, typeParams, sw, nil) g.generateGetters(t, typeParams, sw, nil)
return sw.Error() return sw.Error()
} }
@ -176,7 +177,8 @@ func (g *applyConfigurationGenerator) generateGetters(t *types.Type, typeParams
} }
} }
func (g *applyConfigurationGenerator) generateWithFuncs(t *types.Type, typeParams TypeParams, sw *generator.SnippetWriter, embed *memberParams) { func (g *applyConfigurationGenerator) generateWithFuncs(t *types.Type, typeParams TypeParams, sw *generator.SnippetWriter, embed *memberParams,
generated *[]string) {
for _, member := range t.Members { for _, member := range t.Members {
if blocklisted(t, member) { if blocklisted(t, member) {
continue continue
@ -186,6 +188,11 @@ func (g *applyConfigurationGenerator) generateWithFuncs(t *types.Type, typeParam
memberType = &types.Type{Kind: types.Pointer, Elem: memberType} memberType = &types.Type{Kind: types.Pointer, Elem: memberType}
} }
if jsonTags, ok := lookupJSONTags(member); ok { if jsonTags, ok := lookupJSONTags(member); ok {
if slices.Contains(*generated, member.Name) {
klog.V(5).Infof("With%s already generated on %s, skipping\n", member.Name, t.Name)
continue
}
*generated = append(*generated, member.Name)
memberParams := memberParams{ memberParams := memberParams{
TypeParams: typeParams, TypeParams: typeParams,
Member: member, Member: member,
@ -194,7 +201,7 @@ func (g *applyConfigurationGenerator) generateWithFuncs(t *types.Type, typeParam
EmbeddedIn: embed, EmbeddedIn: embed,
} }
if memberParams.Member.Embedded { if memberParams.Member.Embedded {
g.generateWithFuncs(member.Type, typeParams, sw, &memberParams) g.generateWithFuncs(member.Type, typeParams, sw, &memberParams, generated)
if !jsonTags.inline { if !jsonTags.inline {
// non-inlined embeds are nillable and need a "ensure exists" utility function // non-inlined embeds are nillable and need a "ensure exists" utility function
sw.Do(ensureEmbedExists, memberParams) sw.Do(ensureEmbedExists, memberParams)
@ -281,9 +288,9 @@ func (g *applyConfigurationGenerator) generateMemberWith(sw *generator.SnippetWr
sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) With$.Member.Name$(value $.MemberType|raw$) *$.ApplyConfig.ApplyConfiguration|public$ {\n", memberParams) sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) With$.Member.Name$(value $.MemberType|raw$) *$.ApplyConfig.ApplyConfiguration|public$ {\n", memberParams)
g.ensureEmbedExistsIfApplicable(sw, memberParams) g.ensureEmbedExistsIfApplicable(sw, memberParams)
if g.refGraph.isApplyConfig(memberParams.Member.Type) || isNillable(memberParams.Member.Type) { if g.refGraph.isApplyConfig(memberParams.Member.Type) || isNillable(memberParams.Member.Type) {
sw.Do("b.$.Member.Name$ = value\n", memberParams) sw.Do("b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = value\n", memberParams)
} else { } else {
sw.Do("b.$.Member.Name$ = &value\n", memberParams) sw.Do("b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = &value\n", memberParams)
} }
sw.Do(" return b\n", memberParams) sw.Do(" return b\n", memberParams)
sw.Do("}\n", memberParams) sw.Do("}\n", memberParams)
@ -297,7 +304,7 @@ func (g *applyConfigurationGenerator) generateMemberGetter(sw *generator.Snippet
sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) Get$.Member.Name$() *$.MemberType|raw$ {\n", memberParams) sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) Get$.Member.Name$() *$.MemberType|raw$ {\n", memberParams)
} }
g.ensureEmbedExistsIfApplicable(sw, memberParams) g.ensureEmbedExistsIfApplicable(sw, memberParams)
sw.Do(" return b.$.Member.Name$\n", memberParams) sw.Do(" return b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$\n", memberParams)
sw.Do("}\n", memberParams) sw.Do("}\n", memberParams)
} }
@ -324,15 +331,15 @@ func (g *applyConfigurationGenerator) generateMemberWithForSlice(sw *generator.S
sw.Do("}\n", memberParams) sw.Do("}\n", memberParams)
if memberIsPointerToSlice { if memberIsPointerToSlice {
sw.Do("*b.$.Member.Name$ = append(*b.$.Member.Name$, *values[i])\n", memberParams) sw.Do("*b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = append(*b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$, *values[i])\n", memberParams)
} else { } else {
sw.Do("b.$.Member.Name$ = append(b.$.Member.Name$, *values[i])\n", memberParams) sw.Do("b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = append(b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$, *values[i])\n", memberParams)
} }
} else { } else {
if memberIsPointerToSlice { if memberIsPointerToSlice {
sw.Do("*b.$.Member.Name$ = append(*b.$.Member.Name$, values[i])\n", memberParams) sw.Do("*b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = append(*b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$, values[i])\n", memberParams)
} else { } else {
sw.Do("b.$.Member.Name$ = append(b.$.Member.Name$, values[i])\n", memberParams) sw.Do("b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = append(b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$, values[i])\n", memberParams)
} }
} }
sw.Do(" }\n", memberParams) sw.Do(" }\n", memberParams)
@ -347,11 +354,11 @@ func (g *applyConfigurationGenerator) generateMemberWithForMap(sw *generator.Sni
sw.Do("// overwriting an existing map entries in $.Member.Name$ field with the same key.\n", memberParams) sw.Do("// overwriting an existing map entries in $.Member.Name$ field with the same key.\n", memberParams)
sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) With$.Member.Name$(entries $.MemberType|raw$) *$.ApplyConfig.ApplyConfiguration|public$ {\n", memberParams) sw.Do("func (b *$.ApplyConfig.ApplyConfiguration|public$) With$.Member.Name$(entries $.MemberType|raw$) *$.ApplyConfig.ApplyConfiguration|public$ {\n", memberParams)
g.ensureEmbedExistsIfApplicable(sw, memberParams) g.ensureEmbedExistsIfApplicable(sw, memberParams)
sw.Do(" if b.$.Member.Name$ == nil && len(entries) > 0 {\n", memberParams) sw.Do(" if b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ == nil && len(entries) > 0 {\n", memberParams)
sw.Do(" b.$.Member.Name$ = make($.MemberType|raw$, len(entries))\n", memberParams) sw.Do(" b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$ = make($.MemberType|raw$, len(entries))\n", memberParams)
sw.Do(" }\n", memberParams) sw.Do(" }\n", memberParams)
sw.Do(" for k, v := range entries {\n", memberParams) sw.Do(" for k, v := range entries {\n", memberParams)
sw.Do(" b.$.Member.Name$[k] = v\n", memberParams) sw.Do(" b$if ne .EmbeddedIn nil$.$.EmbeddedIn.MemberType.Elem.Name.Name$$end$.$.Member.Name$[k] = v\n", memberParams)
sw.Do(" }\n", memberParams) sw.Do(" }\n", memberParams)
sw.Do(" return b\n", memberParams) sw.Do(" return b\n", memberParams)
sw.Do("}\n", memberParams) sw.Do("}\n", memberParams)