Merge pull request #281 from colincross/proptools_consistency

Proptools consistency
This commit is contained in:
colincross 2020-01-28 10:02:20 -08:00 committed by GitHub
commit 38e095ae8a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 109 additions and 87 deletions

View file

@ -1202,8 +1202,8 @@ func (c *Context) cloneLogicModule(origModule *moduleInfo) (Module, []interface{
}
for i := range newProperties {
dst := reflect.ValueOf(newProperties[i]).Elem()
src := reflect.ValueOf(origModule.properties[i]).Elem()
dst := reflect.ValueOf(newProperties[i])
src := reflect.ValueOf(origModule.properties[i])
proptools.CopyProperties(dst, src)
}

View file

@ -20,13 +20,32 @@ import (
"sync"
)
// CloneProperties takes a reflect.Value of a pointer to a struct and returns a reflect.Value
// of a pointer to a new struct that copies of the values for its fields. It recursively clones
// struct pointers and interfaces that contain struct pointers.
func CloneProperties(structValue reflect.Value) reflect.Value {
result := reflect.New(structValue.Type())
CopyProperties(result.Elem(), structValue)
if !isStructPtr(structValue.Type()) {
panic(fmt.Errorf("CloneProperties expected *struct, got %s", structValue.Type()))
}
result := reflect.New(structValue.Type().Elem())
copyProperties(result.Elem(), structValue.Elem())
return result
}
// CopyProperties takes destination and source reflect.Values of a pointer to structs and returns
// copies each field from the source into the destination. It recursively copies struct pointers
// and interfaces that contain struct pointers.
func CopyProperties(dstValue, srcValue reflect.Value) {
if !isStructPtr(dstValue.Type()) {
panic(fmt.Errorf("CopyProperties expected dstValue *struct, got %s", dstValue.Type()))
}
if !isStructPtr(srcValue.Type()) {
panic(fmt.Errorf("CopyProperties expected srcValue *struct, got %s", srcValue.Type()))
}
copyProperties(dstValue.Elem(), srcValue.Elem())
}
func copyProperties(dstValue, srcValue reflect.Value) {
typ := dstValue.Type()
if srcValue.Type() != typ {
panic(fmt.Errorf("can't copy mismatching types (%s <- %s)",
@ -47,7 +66,7 @@ func CopyProperties(dstValue, srcValue reflect.Value) {
case reflect.Bool, reflect.String, reflect.Int, reflect.Uint:
dstFieldValue.Set(srcFieldValue)
case reflect.Struct:
CopyProperties(dstFieldValue, srcFieldValue)
copyProperties(dstFieldValue, srcFieldValue)
case reflect.Slice:
if !srcFieldValue.IsNil() {
if srcFieldValue != dstFieldValue {
@ -67,13 +86,9 @@ func CopyProperties(dstValue, srcValue reflect.Value) {
srcFieldValue = srcFieldValue.Elem()
if srcFieldValue.Kind() != reflect.Ptr {
panic(fmt.Errorf("can't clone field %q: interface refers to a non-pointer",
field.Name))
}
if srcFieldValue.Type().Elem().Kind() != reflect.Struct {
panic(fmt.Errorf("can't clone field %q: interface points to a non-struct",
field.Name))
if !isStructPtr(srcFieldValue.Type()) {
panic(fmt.Errorf("can't clone field %q: expected interface to contain *struct, found %s",
field.Name, srcFieldValue.Type()))
}
if dstFieldValue.IsNil() || dstFieldValue.Elem().Type() != srcFieldValue.Type() {
@ -93,13 +108,11 @@ func CopyProperties(dstValue, srcValue reflect.Value) {
break
}
srcFieldValue := srcFieldValue.Elem()
switch srcFieldValue.Kind() {
switch srcFieldValue.Elem().Kind() {
case reflect.Struct:
if !dstFieldValue.IsNil() {
// Re-use the existing allocation.
CopyProperties(dstFieldValue.Elem(), srcFieldValue)
copyProperties(dstFieldValue.Elem(), srcFieldValue.Elem())
break
} else {
newValue := CloneProperties(srcFieldValue)
@ -110,21 +123,30 @@ func CopyProperties(dstValue, srcValue reflect.Value) {
}
}
case reflect.Bool, reflect.Int64, reflect.String:
newValue := reflect.New(srcFieldValue.Type())
newValue.Elem().Set(srcFieldValue)
newValue := reflect.New(srcFieldValue.Elem().Type())
newValue.Elem().Set(srcFieldValue.Elem())
origDstFieldValue.Set(newValue)
default:
panic(fmt.Errorf("can't clone field %q: points to a %s",
field.Name, srcFieldValue.Kind()))
panic(fmt.Errorf("can't clone pointer field %q type %s",
field.Name, srcFieldValue.Type()))
}
default:
panic(fmt.Errorf("unexpected kind for property struct field %q: %s",
field.Name, srcFieldValue.Kind()))
panic(fmt.Errorf("unexpected type for property struct field %q: %s",
field.Name, srcFieldValue.Type()))
}
}
}
// ZeroProperties takes a reflect.Value of a pointer to a struct and replaces all of its fields
// with zero values, recursing into struct, pointer to struct and interface fields.
func ZeroProperties(structValue reflect.Value) {
if !isStructPtr(structValue.Type()) {
panic(fmt.Errorf("ZeroProperties expected *struct, got %s", structValue.Type()))
}
zeroProperties(structValue.Elem())
}
func zeroProperties(structValue reflect.Value) {
typ := structValue.Type()
for i, field := range typeFields(typ) {
@ -146,13 +168,9 @@ func ZeroProperties(structValue reflect.Value) {
// We leave the pointer intact and zero out the struct that's
// pointed to.
fieldValue = fieldValue.Elem()
if fieldValue.Kind() != reflect.Ptr {
panic(fmt.Errorf("can't zero field %q: interface refers to a non-pointer",
field.Name))
}
if fieldValue.Type().Elem().Kind() != reflect.Struct {
panic(fmt.Errorf("can't zero field %q: interface points to a non-struct",
field.Name))
if !isStructPtr(fieldValue.Type()) {
panic(fmt.Errorf("can't zero field %q: expected interface to contain *struct, found %s",
field.Name, fieldValue.Type()))
}
fallthrough
case reflect.Ptr:
@ -161,7 +179,7 @@ func ZeroProperties(structValue reflect.Value) {
if fieldValue.IsNil() {
break
}
ZeroProperties(fieldValue.Elem())
zeroProperties(fieldValue.Elem())
case reflect.Bool, reflect.Int64, reflect.String:
fieldValue.Set(reflect.Zero(fieldValue.Type()))
default:
@ -169,7 +187,7 @@ func ZeroProperties(structValue reflect.Value) {
field.Name, fieldValue.Elem().Kind()))
}
case reflect.Struct:
ZeroProperties(fieldValue)
zeroProperties(fieldValue)
default:
panic(fmt.Errorf("unexpected kind for property struct field %q: %s",
field.Name, fieldValue.Kind()))
@ -177,9 +195,15 @@ func ZeroProperties(structValue reflect.Value) {
}
}
// CloneEmptyProperties takes a reflect.Value of a pointer to a struct and returns a reflect.Value
// of a pointer to a new struct that has the zero values for its fields. It recursively clones
// struct pointers and interfaces that contain struct pointers.
func CloneEmptyProperties(structValue reflect.Value) reflect.Value {
result := reflect.New(structValue.Type())
cloneEmptyProperties(result.Elem(), structValue)
if !isStructPtr(structValue.Type()) {
panic(fmt.Errorf("CloneEmptyProperties expected *struct, got %s", structValue.Type()))
}
result := reflect.New(structValue.Type().Elem())
cloneEmptyProperties(result.Elem(), structValue.Elem())
return result
}
@ -206,13 +230,9 @@ func cloneEmptyProperties(dstValue, srcValue reflect.Value) {
}
srcFieldValue = srcFieldValue.Elem()
if srcFieldValue.Kind() != reflect.Ptr {
panic(fmt.Errorf("can't clone empty field %q: interface refers to a non-pointer",
field.Name))
}
if srcFieldValue.Type().Elem().Kind() != reflect.Struct {
panic(fmt.Errorf("can't clone empty field %q: interface points to a non-struct",
field.Name))
if !isStructPtr(srcFieldValue.Type()) {
panic(fmt.Errorf("can't clone empty field %q: expected interface to contain *struct, found %s",
field.Name, srcFieldValue.Type()))
}
newValue := reflect.New(srcFieldValue.Type()).Elem()
@ -226,7 +246,7 @@ func cloneEmptyProperties(dstValue, srcValue reflect.Value) {
if srcFieldValue.IsNil() {
break
}
newValue := CloneEmptyProperties(srcFieldValue.Elem())
newValue := CloneEmptyProperties(srcFieldValue)
if dstFieldInterfaceValue.IsValid() {
dstFieldInterfaceValue.Set(newValue)
} else {

View file

@ -277,7 +277,7 @@ func TestCloneProperties(t *testing.T) {
for _, testCase := range clonePropertiesTestCases {
testString := fmt.Sprintf("%s", testCase.in)
got := CloneProperties(reflect.ValueOf(testCase.in).Elem()).Interface()
got := CloneProperties(reflect.ValueOf(testCase.in)).Interface()
if !reflect.DeepEqual(testCase.out, got) {
t.Errorf("test case %s", testString)
@ -499,7 +499,7 @@ func TestCloneEmptyProperties(t *testing.T) {
for _, testCase := range cloneEmptyPropertiesTestCases {
testString := fmt.Sprintf("%#v", testCase.in)
got := CloneEmptyProperties(reflect.ValueOf(testCase.in).Elem()).Interface()
got := CloneEmptyProperties(reflect.ValueOf(testCase.in)).Interface()
if !reflect.DeepEqual(testCase.out, got) {
t.Errorf("test case %s", testString)
@ -514,8 +514,8 @@ func TestZeroProperties(t *testing.T) {
for _, testCase := range cloneEmptyPropertiesTestCases {
testString := fmt.Sprintf("%#v", testCase.in)
got := CloneProperties(reflect.ValueOf(testCase.in).Elem()).Interface()
ZeroProperties(reflect.ValueOf(got).Elem())
got := CloneProperties(reflect.ValueOf(testCase.in)).Interface()
ZeroProperties(reflect.ValueOf(got))
if !reflect.DeepEqual(testCase.out, got) {
t.Errorf("test case %s", testString)

View file

@ -274,7 +274,7 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
}
// Step into source pointers to structs
if srcFieldValue.Kind() == reflect.Ptr && srcFieldValue.Type().Elem().Kind() == reflect.Struct {
if isStructPtr(srcFieldValue.Type()) {
if srcFieldValue.IsNil() {
continue
}
@ -323,7 +323,7 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
}
// Step into destination pointers to structs
if dstFieldValue.Kind() == reflect.Ptr && dstFieldValue.Type().Elem().Kind() == reflect.Struct {
if isStructPtr(dstFieldValue.Type()) {
if dstFieldValue.IsNil() {
dstFieldValue = reflect.New(dstFieldValue.Type().Elem())
origDstFieldValue.Set(dstFieldValue)
@ -501,11 +501,8 @@ func getOrCreateStruct(in interface{}) (reflect.Value, error) {
func getStruct(in interface{}) (reflect.Value, error) {
value := reflect.ValueOf(in)
if value.Kind() != reflect.Ptr {
return reflect.Value{}, fmt.Errorf("expected pointer to struct, got %T", in)
}
if value.Type().Elem().Kind() != reflect.Struct {
return reflect.Value{}, fmt.Errorf("expected pointer to struct, got %T", in)
if !isStructPtr(value.Type()) {
return reflect.Value{}, fmt.Errorf("expected pointer to struct, got %s", value.Type())
}
if value.IsNil() {
return reflect.Value{}, getStructEmptyError{}

View file

@ -50,8 +50,7 @@ func filterPropertyStructFields(fields []reflect.StructField, prefix string, max
if maxTypeNameSize > 0 && structNameSize+fieldTypeNameSize > maxTypeNameSize {
if len(filteredFields) == 0 {
if field.Type.Kind() == reflect.Struct ||
field.Type.Kind() == reflect.Ptr && field.Type.Elem().Kind() == reflect.Struct {
if isStruct(field.Type) || isStructPtr(field.Type) {
// An error fitting the nested struct should have been caught when recursing
// into the nested struct.
panic(fmt.Errorf("Shouldn't happen: can't fit nested struct %q (%d) into %d",
@ -82,12 +81,12 @@ func filterPropertyStructFields(fields []reflect.StructField, prefix string, max
}
ptrToStruct := false
if field.Type.Kind() == reflect.Ptr && field.Type.Elem().Kind() == reflect.Struct {
if isStructPtr(field.Type) {
ptrToStruct = true
}
// Recurse into structs
if ptrToStruct || field.Type.Kind() == reflect.Struct {
if ptrToStruct || isStruct(field.Type) {
subMaxTypeNameSize := maxTypeNameSize
if maxTypeNameSize > 0 {
// In the worst case where only this nested struct will fit in the outer struct, the

View file

@ -15,6 +15,7 @@
package proptools
import (
"reflect"
"unicode"
"unicode/utf8"
)
@ -97,3 +98,11 @@ func IntDefault(i *int64, def int) int {
func Int(i *int64) int {
return IntDefault(i, 0)
}
func isStruct(t reflect.Type) bool {
return t.Kind() == reflect.Struct
}
func isStructPtr(t reflect.Type) bool {
return t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Struct
}

View file

@ -36,7 +36,7 @@ func HasTag(field reflect.StructField, name, value string) bool {
// are tagged with the given key and value, including ones found in embedded structs or pointers to structs.
func PropertyIndexesWithTag(ps interface{}, key, value string) [][]int {
t := reflect.TypeOf(ps)
if t.Kind() != reflect.Ptr || t.Elem().Kind() != reflect.Struct {
if !isStructPtr(t) {
panic(fmt.Errorf("type %s is not a pointer to a struct", t))
}
t = t.Elem()
@ -49,7 +49,7 @@ func propertyIndexesWithTag(t reflect.Type, key, value string) [][]int {
for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
ft := field.Type
if ft.Kind() == reflect.Struct || (ft.Kind() == reflect.Ptr && ft.Elem().Kind() == reflect.Struct) {
if isStruct(ft) || isStructPtr(ft) {
if ft.Kind() == reflect.Ptr {
ft = ft.Elem()
}

View file

@ -49,14 +49,11 @@ func UnpackProperties(propertyDefs []*parser.Property,
for _, properties := range propertiesStructs {
propertiesValue := reflect.ValueOf(properties)
if propertiesValue.Kind() != reflect.Ptr {
panic("properties must be a pointer to a struct")
if !isStructPtr(propertiesValue.Type()) {
panic(fmt.Errorf("properties must be *struct, got %s",
propertiesValue.Type()))
}
propertiesValue = propertiesValue.Elem()
if propertiesValue.Kind() != reflect.Struct {
panic("properties must be a pointer to a struct")
}
newErrs := unpackStructValue("", propertiesValue, propertyMap)
errs = append(errs, newErrs...)
@ -212,7 +209,7 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
panic(fmt.Errorf("unsupported kind for field %s: %s", propertyName, kind))
}
if field.Anonymous && fieldValue.Kind() == reflect.Struct {
if field.Anonymous && isStruct(fieldValue.Type()) {
newErrs := unpackStructValue(namePrefix, fieldValue, propertyMap)
errs = append(errs, newErrs...)
continue
@ -239,7 +236,7 @@ func unpackStructValue(namePrefix string, structValue reflect.Value,
var newErrs []error
if fieldValue.Kind() == reflect.Struct {
if isStruct(fieldValue.Type()) {
newErrs = unpackStruct(propertyName+".", fieldValue,
packedProperty.property, propertyMap)

View file

@ -37,7 +37,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
S *string
Blank *string
Unset *string
@ -56,7 +56,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
S string
}{
S: "abc",
@ -71,7 +71,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
IsGood bool
}{
IsGood: true,
@ -87,7 +87,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
IsGood *bool
IsBad *bool
IsUgly *bool
@ -108,7 +108,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Stuff []string
Empty []string
Nil []string
@ -131,7 +131,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested struct {
S string
}
@ -152,7 +152,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested interface{}
}{
Nested: &struct{ S string }{
@ -173,7 +173,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested struct {
Foo string
}
@ -200,7 +200,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested struct {
Foo string `allowNested:"true"`
} `blueprint:"filter(allowNested:\"true\")"`
@ -229,7 +229,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
EmbeddedStruct
Nested struct {
EmbeddedStruct
@ -260,7 +260,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
EmbeddedInterface
Nested struct {
EmbeddedInterface
@ -291,7 +291,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
S string
EmbeddedStruct
Nested struct {
@ -327,7 +327,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
S string
EmbeddedInterface
Nested struct {
@ -365,7 +365,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
S string
List []string
List2 []string
@ -387,7 +387,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested struct {
S string
}
@ -396,7 +396,7 @@ var validUnpackTestCases = []struct {
S: "abc",
},
},
struct {
&struct {
Nested struct {
S string
}
@ -405,7 +405,7 @@ var validUnpackTestCases = []struct {
S: "abc",
},
},
struct {
&struct {
}{},
},
},
@ -420,7 +420,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested *struct {
S string
}
@ -449,7 +449,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
Nested interface{}
}{
Nested: &EmbeddedStruct{
@ -478,7 +478,7 @@ var validUnpackTestCases = []struct {
}
`,
output: []interface{}{
struct {
&struct {
String string
String_ptr *string
Bool bool
@ -558,7 +558,7 @@ func TestUnpackProperties(t *testing.T) {
}
for i := range output {
got := reflect.ValueOf(output[i]).Elem().Interface()
got := reflect.ValueOf(output[i]).Interface()
if !reflect.DeepEqual(got, testCase.output[i]) {
t.Errorf("test case: %s", testCase.input)
t.Errorf("incorrect output:")