Make proptools functions consistently take *struct types

The proptools functions took an inconsistent variety of
struct and *struct types.  Some methods even took a struct
but returned a *struct.  Make all the exported methods
take a *struct, with internal helpers for the ones that need
to take a struct.

Test: proptools tests
Change-Id: I60ce212606e96adcef66c531d57f69c39e1a1638

context.go

@@ -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)
 	}

proptools/clone.go

@@ -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 {
@@ -89,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)
@@ -106,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) {
@@ -153,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:
@@ -161,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()))
@@ -169,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
 }
 
@@ -214,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 {

proptools/clone_test.go

@@ -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)

proptools/unpack_test.go

@@ -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:")