fix: handle pkl sub classes / extending structs in custom classes #33
Conversation
add support for unmarshalling into anonymous go struct fields from pkl sub-classes
| var fields map[string]structField | ||
| switch field.Type.Kind() { | ||
| case reflect.Ptr: | ||
| fields = getStructFields(field.Type.Elem()) | ||
| case reflect.Struct: | ||
| fields = getStructFields(field.Type) | ||
| } | ||
| for k, v := range fields { |
There was a problem hiding this comment.
Not very passionate, but shouldn't the difference be the "smallest"?
| var fields map[string]structField | |
| switch field.Type.Kind() { | |
| case reflect.Ptr: | |
| fields = getStructFields(field.Type.Elem()) | |
| case reflect.Struct: | |
| fields = getStructFields(field.Type) | |
| } | |
| for k, v := range fields { | |
| fieldType := field.Type | |
| for fieldType.Kind() == reflect.Ptr { | |
| fieldType = fieldType.Elem() | |
| } | |
| for k, v := range getStructFields(fieldType) { |
(as a side-effect; this also traverses nested pointers)
| if field.Anonymous && field.Type.Kind() == reflect.Ptr { | ||
| fieldValue := reflect.New(field.Type.Elem()) | ||
| // Assertion: all embedded fields are pointers to structs. |
There was a problem hiding this comment.
This comment is now somewhat out of place. Shouldn't there rather be a comment at the top to say no output is written to non-pointer fields? Is it expected to no longer include non-pointer fields in getOutputValue? (cc @bioball)
There was a problem hiding this comment.
This is just some logic that initializes embedded pointer values, if they exist. If the embedded value is a plain struct, it makes sense that there's nothing to do (the zero value of a struct type also a struct).
I think it's enough to just remove this comment altogether and move on.
bioball
left a comment
bioball
left a comment
There was a problem hiding this comment.
Some minor nits, but overall looks good!
| class House extends Shape { | ||
| bedrooms: Int | ||
| bathrooms: Int | ||
| } |
| "context" | ||
| _ "embed" | ||
| "github.com/apple/pkl-go/pkl/test_fixtures/custom" | ||
| "github.com/stretchr/testify/require" |
There was a problem hiding this comment.
Can you run goimports -w . to update the import order here?
If you don't have goimports, you can run go install golang.org/x/tools/cmd/goimports@latest to install it.
Details on goimports: https://pkg.go.dev/golang.org/x/tools/cmd/goimports
| if field.Anonymous && field.Type.Kind() == reflect.Ptr { | ||
| fieldValue := reflect.New(field.Type.Elem()) | ||
| // Assertion: all embedded fields are pointers to structs. |
There was a problem hiding this comment.
This is just some logic that initializes embedded pointer values, if they exist. If the embedded value is a plain struct, it makes sense that there's nothing to do (the zero value of a struct type also a struct).
I think it's enough to just remove this comment altogether and move on.
3 participants
add support for unmarshalling into anonymous go struct fields from pkl sub-classes
so we can create custom go structs using inheritence and unmarshal pkl to them