Skip to content

Commit 72c92c4

Browse files
raphaelraphael
authored
Fix retrieval of transform helpers (goadesign#2137)
in the case where the transformed type have a inner attribute of type array. Cleanup transform helper code to be more readable. Add comments.
1 parent c29d3a6 commit 72c92c4

File tree

3 files changed

+91
-147
lines changed

3 files changed

+91
-147
lines changed

‎codegen/go_transform.go

+84-147
Original file line numberDiff line numberDiff line change
@@ -63,40 +63,30 @@ func GoTransform(source, target *expr.AttributeExpr, sourceVar, targetVar string
6363
// transformAttribute returns the code to transform source attribute to target
6464
// attribute. It returns an error if source and target are not compatible for
6565
// transformation.
66-
func transformAttribute(source, target *expr.AttributeExpr, sourceVar, targetVar string, newVar bool, ta *TransformAttrs) (string, error) {
67-
var err error
68-
{
69-
if err = IsCompatible(source.Type, target.Type, sourceVar, targetVar); err != nil {
70-
return "", err
71-
}
66+
func transformAttribute(source, target *expr.AttributeExpr, sourceVar, targetVar string, newVar bool, ta *TransformAttrs) (code string, err error) {
67+
if err = IsCompatible(source.Type, target.Type, sourceVar, targetVar); err != nil {
68+
return
7269
}
73-
74-
var code string
75-
{
76-
switch {
77-
case expr.IsArray(source.Type):
78-
code, err = transformArray(expr.AsArray(source.Type), expr.AsArray(target.Type), sourceVar, targetVar, newVar, ta)
79-
case expr.IsMap(source.Type):
80-
code, err = transformMap(expr.AsMap(source.Type), expr.AsMap(target.Type), sourceVar, targetVar, newVar, ta)
81-
case expr.IsObject(source.Type):
82-
code, err = transformObject(source, target, sourceVar, targetVar, newVar, ta)
83-
default:
84-
assign := "="
85-
if newVar {
86-
assign = ":="
87-
}
88-
if _, ok := target.Type.(expr.UserType); ok {
89-
// Primitive user type, these are used for error results
90-
cast := ta.TargetCtx.Scope.Ref(target, ta.TargetCtx.Pkg)
91-
return fmt.Sprintf("%s %s %s(%s)\n", targetVar, assign, cast, sourceVar), nil
92-
}
93-
code = fmt.Sprintf("%s %s %s\n", targetVar, assign, sourceVar)
70+
switch {
71+
case expr.IsArray(source.Type):
72+
code, err = transformArray(expr.AsArray(source.Type), expr.AsArray(target.Type), sourceVar, targetVar, newVar, ta)
73+
case expr.IsMap(source.Type):
74+
code, err = transformMap(expr.AsMap(source.Type), expr.AsMap(target.Type), sourceVar, targetVar, newVar, ta)
75+
case expr.IsObject(source.Type):
76+
code, err = transformObject(source, target, sourceVar, targetVar, newVar, ta)
77+
default:
78+
assign := "="
79+
if newVar {
80+
assign = ":="
9481
}
82+
if _, ok := target.Type.(expr.UserType); ok {
83+
// Primitive user type, these are used for error results
84+
cast := ta.TargetCtx.Scope.Ref(target, ta.TargetCtx.Pkg)
85+
return fmt.Sprintf("%s %s %s(%s)\n", targetVar, assign, cast, sourceVar), nil
86+
}
87+
code = fmt.Sprintf("%s %s %s\n", targetVar, assign, sourceVar)
9588
}
96-
if err != nil {
97-
return "", err
98-
}
99-
return code, nil
89+
return
10090
}
10191

10292
// transformObject generates Go code to transform source object to target
@@ -335,141 +325,86 @@ func transformMapElem(source, target *expr.Map, sourceVar, targetVar string, new
335325
// transformAttributeHelpers returns the Go transform functions and their definitions
336326
// that may be used in code produced by Transform. It returns an error if source and
337327
// target are incompatible (different types, fields of different type etc).
328+
// transformAttributeHelpers recurses through the attribute types and calls
329+
// collectHelpers for each child attribute. collectHelpers actually produces the
330+
// transform helper functions for the given attribute.
338331
//
339332
// source, target are the source and target attributes used in transformation
340333
//
341334
// ta holds the transform attributes
342335
//
343-
// seen keeps track of generated transform functions to avoid recursion
336+
// seen keeps track of generated transform functions to avoid infinite recursion.
344337
//
345-
func transformAttributeHelpers(source, target *expr.AttributeExpr, ta *TransformAttrs, seen map[string]*TransformFunctionData) ([]*TransformFunctionData, error) {
346-
var (
347-
helpers []*TransformFunctionData
348-
err error
349-
)
350-
{
351-
// Do not generate a transform function for the top most user type.
352-
switch {
353-
case expr.IsArray(source.Type):
354-
source = expr.AsArray(source.Type).ElemType
355-
target = expr.AsArray(target.Type).ElemType
356-
helpers, err = transformAttributeHelpers(source, target, ta, seen)
357-
case expr.IsMap(source.Type):
358-
sm := expr.AsMap(source.Type)
359-
tm := expr.AsMap(target.Type)
360-
helpers, err = transformAttributeHelpers(sm.ElemType, tm.ElemType, ta, seen)
361-
if err == nil {
362-
var other []*TransformFunctionData
363-
other, err = transformAttributeHelpers(sm.KeyType, tm.KeyType, ta, seen)
364-
helpers = append(helpers, other...)
365-
}
366-
case expr.IsObject(source.Type):
367-
walkMatches(source, target, func(srcMatt, tgtMatt *expr.MappedAttributeExpr, srcc, tgtc *expr.AttributeExpr, n string) {
368-
if err != nil {
369-
return
370-
}
371-
h, err2 := collectHelpers(srcc, tgtc, srcMatt.IsRequired(n), ta, seen)
372-
if err2 != nil {
373-
err = err2
374-
return
375-
}
376-
helpers = append(helpers, h...)
377-
})
338+
func transformAttributeHelpers(source, target *expr.AttributeExpr, ta *TransformAttrs, seen map[string]*TransformFunctionData) (helpers []*TransformFunctionData, err error) {
339+
// Do not generate a transform function for the top most user type.
340+
switch {
341+
case expr.IsArray(source.Type):
342+
helpers, err = transformAttributeHelpers(expr.AsArray(source.Type).ElemType, expr.AsArray(target.Type).ElemType, ta, seen)
343+
case expr.IsMap(source.Type):
344+
sm, tm := expr.AsMap(source.Type), expr.AsMap(target.Type)
345+
helpers, err = transformAttributeHelpers(sm.ElemType, tm.ElemType, ta, seen)
346+
if err == nil {
347+
var other []*TransformFunctionData
348+
other, err = collectHelpers(sm.KeyType, tm.KeyType, true, ta, seen)
349+
helpers = append(helpers, other...)
378350
}
351+
case expr.IsObject(source.Type):
352+
walkMatches(source, target, func(srcMatt, _ *expr.MappedAttributeExpr, srcc, tgtc *expr.AttributeExpr, n string) {
353+
if err != nil {
354+
return
355+
}
356+
var hs []*TransformFunctionData
357+
hs, err = collectHelpers(srcc, tgtc, srcMatt.IsRequired(n), ta, seen)
358+
helpers = append(helpers, hs...)
359+
})
379360
}
380-
if err != nil {
381-
return nil, err
382-
}
383-
return helpers, nil
361+
return
384362
}
385363

386-
// collectHelpers recursively traverses the given attributes and return the
387-
// transform helper functions required to generate the transform code.
388-
func collectHelpers(source, target *expr.AttributeExpr, req bool, ta *TransformAttrs, seen map[string]*TransformFunctionData) ([]*TransformFunctionData, error) {
389-
var (
390-
data []*TransformFunctionData
391-
)
364+
// collectHelpers recurses through the given attributes and returns the transform
365+
// helper functions required to generate the transform code. If the attributes type
366+
// is array or map then the recursion is done via transformAttributeHelpers so that
367+
// the tope level conversion function is skipped as the generate code does not make
368+
// use of it (since it inlines that top-level transformation).
369+
func collectHelpers(source, target *expr.AttributeExpr, req bool, ta *TransformAttrs, seen map[string]*TransformFunctionData) (helpers []*TransformFunctionData, err error) {
370+
name := transformHelperName(source, target, ta)
371+
if _, ok := seen[name]; ok {
372+
return
373+
}
374+
if _, ok := source.Type.(expr.UserType); ok {
375+
var h *TransformFunctionData
376+
if h, err = generateHelper(source, target, req, ta, seen); h != nil {
377+
helpers = append(helpers, h)
378+
}
379+
}
392380
switch {
393381
case expr.IsArray(source.Type):
394-
st, tt := expr.AsArray(source.Type).ElemType, expr.AsArray(target.Type).ElemType
395-
if _, ok := st.Type.(expr.UserType); ok {
396-
// Handle array of user types explicitly to avoid infinite recursions
397-
// when the user type has an attribute of type array of itself.
398-
d, err := generateHelper(st, tt, req, ta, seen)
399-
if err != nil {
400-
return nil, err
401-
}
402-
if d != nil {
403-
data = append(data, d)
404-
}
405-
} else {
406-
helpers, err := transformAttributeHelpers(st, tt, ta, seen)
407-
if err != nil {
408-
return nil, err
409-
}
410-
data = append(data, helpers...)
411-
}
382+
helpers, err = collectHelpers(expr.AsArray(source.Type).ElemType, expr.AsArray(target.Type).ElemType, req, ta, seen)
412383
case expr.IsMap(source.Type):
413-
se, te := expr.AsMap(source.Type).ElemType, expr.AsMap(target.Type).ElemType
414-
sk, tk := expr.AsMap(source.Type).KeyType, expr.AsMap(target.Type).KeyType
415-
helpers, err := transformAttributeHelpers(sk, tk, ta, seen)
416-
if err != nil {
417-
return nil, err
418-
}
419-
data = append(data, helpers...)
420-
if _, ok := se.Type.(expr.UserType); ok {
421-
// Handle map of user types explicitly to avoid infinite recursions
422-
// when the user type has an attribute of type map of itself.
423-
d, err := generateHelper(se, te, req, ta, seen)
424-
if err != nil {
425-
return nil, err
426-
}
427-
if d != nil {
428-
data = append(data, d)
429-
}
430-
} else {
431-
helpers, err = transformAttributeHelpers(se, te, ta, seen)
432-
if err != nil {
433-
return nil, err
434-
}
435-
data = append(data, helpers...)
384+
sm, tm := expr.AsMap(source.Type), expr.AsMap(target.Type)
385+
helpers, err = collectHelpers(sm.ElemType, tm.ElemType, req, ta, seen)
386+
if err == nil {
387+
var other []*TransformFunctionData
388+
other, err = collectHelpers(sm.KeyType, tm.KeyType, req, ta, seen)
389+
helpers = append(helpers, other...)
436390
}
437391
case expr.IsObject(source.Type):
438-
if _, ok := source.Type.(expr.UserType); ok {
439-
d, err := generateHelper(source, target, req, ta, seen)
392+
walkMatches(source, target, func(srcMatt, _ *expr.MappedAttributeExpr, srcc, tgtc *expr.AttributeExpr, n string) {
440393
if err != nil {
441-
return nil, err
442-
}
443-
if d != nil {
444-
data = append(data, d)
394+
return
445395
}
446-
}
447-
448-
// collect helpers
449-
var err error
450-
{
451-
walkMatches(source, target, func(srcMatt, _ *expr.MappedAttributeExpr, srcc, tgtc *expr.AttributeExpr, n string) {
452-
name := transformHelperName(srcc, tgtc, ta)
453-
if _, ok := seen[name]; ok {
454-
return
455-
}
456-
var helpers []*TransformFunctionData
457-
helpers, err = collectHelpers(srcc, tgtc, srcMatt.IsRequired(n), ta, seen)
458-
if err != nil {
459-
return
460-
}
461-
data = append(data, helpers...)
462-
})
463-
}
464-
if err != nil {
465-
return nil, err
466-
}
396+
var hs []*TransformFunctionData
397+
hs, err = collectHelpers(srcc, tgtc, srcMatt.IsRequired(n), ta, seen)
398+
helpers = append(helpers, hs...)
399+
})
467400
}
468-
return data, nil
401+
return
469402
}
470403

471404
// generateHelper generates the code that transform instances of source into
472-
// target.
405+
// target. Both source and targe must be user types or generateHelper panics.
406+
// generateHelper returns nil if a helper has already been generated for the
407+
// pair source, target.
473408
func generateHelper(source, target *expr.AttributeExpr, req bool, ta *TransformAttrs, seen map[string]*TransformFunctionData) (*TransformFunctionData, error) {
474409
name := transformHelperName(source, target, ta)
475410
if _, ok := seen[name]; ok {
@@ -492,8 +427,10 @@ func generateHelper(source, target *expr.AttributeExpr, req bool, ta *TransformA
492427
return tfd, nil
493428
}
494429

495-
// walkMatches iterates through the source attribute expression and executes
496-
// the walker function.
430+
// walkMatches iterates through the attributes of source and looks for
431+
// attributes with identical names in target. walkMatches calls the walker
432+
// function for each pair of matched attributes. Both source and target must be
433+
// objects or else walkMatches panics.
497434
func walkMatches(source, target *expr.AttributeExpr, walker func(src, tgt *expr.MappedAttributeExpr, srcc, tgtc *expr.AttributeExpr, n string)) {
498435
srcMatt := expr.NewMappedAttributeExpr(source)
499436
tgtMatt := expr.NewMappedAttributeExpr(target)

‎codegen/go_transform_helpers_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ func TestGoTransformHelpers(t *testing.T) {
1616
recursive = root.UserType("Recursive")
1717
composite = root.UserType("Composite")
1818
deep = root.UserType("Deep")
19+
deepArray = root.UserType("DeepArray")
1920
// attribute contexts used in test cases
2021
defaultCtx = NewAttributeContext(false, false, true, "", scope)
2122
)
@@ -28,6 +29,7 @@ func TestGoTransformHelpers(t *testing.T) {
2829
{"recursive", recursive, []string{"transformRecursiveToRecursive"}},
2930
{"composite", composite, []string{"transformSimpleToSimple"}},
3031
{"deep", deep, []string{"transformCompositeToComposite", "transformSimpleToSimple"}},
32+
{"deep-array", deepArray, []string{"transformCompositeToComposite", "transformSimpleToSimple"}},
3133
}
3234
for _, c := range tc {
3335
t.Run(c.Name, func(t *testing.T) {

‎codegen/testdata/types_dsl.go

+5
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ var TestTypesDSL = func() {
103103
Attribute("inner", Composite)
104104
})
105105

106+
_ = Type("DeepArray", func() {
107+
Attribute("string", String)
108+
Attribute("inner", ArrayOf(Composite))
109+
})
110+
106111
_ = Type("CompositeWithCustomField", func() {
107112
Attribute("required_string", String, func() {
108113
Meta("struct:field:name", "my_string")

0 commit comments

Comments
 (0)