Merge "Handle property structs and BpPropertySets as values to AddProperty."

android/sdk.go

@@ -237,9 +237,25 @@ type BpPropertySet interface {
 	// * string
 	// * array of the above
 	// * bool
+	// For these types it is an error if multiple properties with the same name
+	// are added.
+	//
+	// * pointer to a struct
 	// * BpPropertySet
 	//
-	// It is an error if multiple properties with the same name are added.
+	// A pointer to a Blueprint-style property struct is first converted into a
+	// BpPropertySet by traversing the fields and adding their values as
+	// properties in a BpPropertySet. A field with a struct value is itself
+	// converted into a BpPropertySet before adding.
+	//
+	// Adding a BpPropertySet is done as follows:
+	// * If no property with the name exists then the BpPropertySet is added
+	//   directly to this property. Care must be taken to ensure that it does not
+	//   introduce a cycle.
+	// * If a property exists with the name and the current value is a
+	//   BpPropertySet then every property of the new BpPropertySet is added to
+	//   the existing BpPropertySet.
+	// * Otherwise, if a property exists with the name then it is an error.
 	AddProperty(name string, value interface{})
 
 	// Add a property with an associated tag

sdk/bp.go

@@ -16,6 +16,8 @@ package sdk
 
 import (
 	"fmt"
+	"reflect"
+	"strings"
 
 	"android/soong/android"
 )
@@ -33,7 +35,82 @@ func (s *bpPropertySet) init() {
 	s.tags = make(map[string]android.BpPropertyTag)
 }
 
+// Converts the given value, which is assumed to be a struct, to a
+// bpPropertySet.
+func convertToPropertySet(value reflect.Value) *bpPropertySet {
+	res := newPropertySet()
+	structType := value.Type()
+
+	for i := 0; i < structType.NumField(); i++ {
+		field := structType.Field(i)
+		fieldVal := value.Field(i)
+
+		switch fieldVal.Type().Kind() {
+		case reflect.Ptr:
+			if fieldVal.IsNil() {
+				continue // nil pointer means the property isn't set.
+			}
+			fieldVal = fieldVal.Elem()
+		case reflect.Slice:
+			if fieldVal.IsNil() {
+				continue // Ignore a nil slice (but not one with length zero).
+			}
+		}
+
+		if fieldVal.Type().Kind() == reflect.Struct {
+			fieldVal = fieldVal.Addr() // Avoid struct copy below.
+		}
+		res.AddProperty(strings.ToLower(field.Name), fieldVal.Interface())
+	}
+
+	return res
+}
+
+// Converts the given value to something that can be set in a property.
+func coercePropertyValue(value interface{}) interface{} {
+	val := reflect.ValueOf(value)
+	switch val.Kind() {
+	case reflect.Struct:
+		// convertToPropertySet requires an addressable struct, and this is probably
+		// a mistake.
+		panic(fmt.Sprintf("Value is a struct, not a pointer to one: %v", value))
+	case reflect.Ptr:
+		if _, ok := value.(*bpPropertySet); !ok {
+			derefValue := reflect.Indirect(val)
+			if derefValue.Kind() != reflect.Struct {
+				panic(fmt.Sprintf("A pointer must be to a struct, got: %v", value))
+			}
+			return convertToPropertySet(derefValue)
+		}
+	}
+	return value
+}
+
+// Merges the fields of the given property set into s.
+func (s *bpPropertySet) mergePropertySet(propSet *bpPropertySet) {
+	for _, name := range propSet.order {
+		if tag, ok := propSet.tags[name]; ok {
+			s.AddPropertyWithTag(name, propSet.properties[name], tag)
+		} else {
+			s.AddProperty(name, propSet.properties[name])
+		}
+	}
+}
+
 func (s *bpPropertySet) AddProperty(name string, value interface{}) {
+	value = coercePropertyValue(value)
+
+	if propSetValue, ok := value.(*bpPropertySet); ok {
+		if curValue, ok := s.properties[name]; ok {
+			if curSet, ok := curValue.(*bpPropertySet); ok {
+				curSet.mergePropertySet(propSetValue)
+				return
+			}
+			// If the current value isn't a property set we got conflicting types.
+			// Continue down to the check below to complain about it.
+		}
+	}
+
 	if s.properties[name] != nil {
 		panic(fmt.Sprintf("Property %q already exists in property set", name))
 	}
@@ -48,9 +125,8 @@ func (s *bpPropertySet) AddPropertyWithTag(name string, value interface{}, tag a
 }
 
 func (s *bpPropertySet) AddPropertySet(name string) android.BpPropertySet {
-	set := newPropertySet()
-	s.AddProperty(name, set)
-	return set
+	s.AddProperty(name, newPropertySet())
+	return s.properties[name].(android.BpPropertySet)
 }
 
 func (s *bpPropertySet) getValue(name string) interface{} {

sdk/bp_test.go

@@ -18,8 +18,142 @@ import (
 	"testing"
 
 	"android/soong/android"
+
+	"github.com/google/blueprint/proptools"
 )
 
+func propertySetFixture() interface{} {
+	set := newPropertySet()
+	set.AddProperty("x", "taxi")
+	set.AddPropertyWithTag("y", 1729, "tag_y")
+	subset := set.AddPropertySet("sub")
+	subset.AddPropertyWithTag("x", "taxi", "tag_x")
+	subset.AddProperty("y", 1729)
+	return set
+}
+
+func intPtr(i int) *int { return &i }
+
+type propertyStruct struct {
+	X     *string
+	Y     *int
+	Unset *bool
+	Sub   struct {
+		X     *string
+		Y     *int
+		Unset *bool
+	}
+}
+
+func propertyStructFixture() interface{} {
+	str := &propertyStruct{}
+	str.X = proptools.StringPtr("taxi")
+	str.Y = intPtr(1729)
+	str.Sub.X = proptools.StringPtr("taxi")
+	str.Sub.Y = intPtr(1729)
+	return str
+}
+
+func checkPropertySetFixture(h *TestHelper, val interface{}, hasTags bool) {
+	set := val.(*bpPropertySet)
+	h.AssertDeepEquals("wrong x value", "taxi", set.getValue("x"))
+	h.AssertDeepEquals("wrong y value", 1729, set.getValue("y"))
+
+	subset := set.getValue("sub").(*bpPropertySet)
+	h.AssertDeepEquals("wrong sub.x value", "taxi", subset.getValue("x"))
+	h.AssertDeepEquals("wrong sub.y value", 1729, subset.getValue("y"))
+
+	if hasTags {
+		h.AssertDeepEquals("wrong y tag", "tag_y", set.getTag("y"))
+		h.AssertDeepEquals("wrong sub.x tag", "tag_x", subset.getTag("x"))
+	} else {
+		h.AssertDeepEquals("wrong y tag", nil, set.getTag("y"))
+		h.AssertDeepEquals("wrong sub.x tag", nil, subset.getTag("x"))
+	}
+}
+
+func TestAddPropertySimple(t *testing.T) {
+	h := &TestHelper{t}
+	set := newPropertySet()
+	for name, val := range map[string]interface{}{
+		"x":   "taxi",
+		"y":   1729,
+		"t":   true,
+		"f":   false,
+		"arr": []string{"a", "b", "c"},
+	} {
+		set.AddProperty(name, val)
+		h.AssertDeepEquals("wrong value", val, set.getValue(name))
+	}
+	h.AssertPanic("adding x again should panic",
+		func() { set.AddProperty("x", "taxi") })
+	h.AssertPanic("adding arr again should panic",
+		func() { set.AddProperty("arr", []string{"d"}) })
+}
+
+func TestAddPropertySubset(t *testing.T) {
+	h := &TestHelper{t}
+	getFixtureMap := map[string]func() interface{}{
+		"property set":    propertySetFixture,
+		"property struct": propertyStructFixture,
+	}
+
+	t.Run("add new subset", func(t *testing.T) {
+		for name, getFixture := range getFixtureMap {
+			t.Run(name, func(t *testing.T) {
+				set := propertySetFixture().(*bpPropertySet)
+				set.AddProperty("new", getFixture())
+				checkPropertySetFixture(h, set, true)
+				checkPropertySetFixture(h, set.getValue("new"), name == "property set")
+			})
+		}
+	})
+
+	t.Run("merge existing subset", func(t *testing.T) {
+		for name, getFixture := range getFixtureMap {
+			t.Run(name, func(t *testing.T) {
+				set := newPropertySet()
+				subset := set.AddPropertySet("sub")
+				subset.AddProperty("flag", false)
+				subset.AddPropertySet("sub")
+				set.AddProperty("sub", getFixture())
+				merged := set.getValue("sub").(*bpPropertySet)
+				h.AssertDeepEquals("wrong flag value", false, merged.getValue("flag"))
+				checkPropertySetFixture(h, merged, name == "property set")
+			})
+		}
+	})
+
+	t.Run("add conflicting subset", func(t *testing.T) {
+		set := propertySetFixture().(*bpPropertySet)
+		h.AssertPanic("adding x again should panic",
+			func() { set.AddProperty("x", propertySetFixture()) })
+	})
+
+	t.Run("add non-pointer struct", func(t *testing.T) {
+		set := propertySetFixture().(*bpPropertySet)
+		str := propertyStructFixture().(*propertyStruct)
+		h.AssertPanic("adding a non-pointer struct should panic",
+			func() { set.AddProperty("new", *str) })
+	})
+}
+
+func TestAddPropertySetNew(t *testing.T) {
+	h := &TestHelper{t}
+	set := newPropertySet()
+	subset := set.AddPropertySet("sub")
+	subset.AddProperty("new", "d^^b")
+	h.AssertDeepEquals("wrong sub.new value", "d^^b", set.getValue("sub").(*bpPropertySet).getValue("new"))
+}
+
+func TestAddPropertySetExisting(t *testing.T) {
+	h := &TestHelper{t}
+	set := propertySetFixture().(*bpPropertySet)
+	subset := set.AddPropertySet("sub")
+	subset.AddProperty("new", "d^^b")
+	h.AssertDeepEquals("wrong sub.new value", "d^^b", set.getValue("sub").(*bpPropertySet).getValue("new"))
+}
+
 type removeFredTransformation struct {
 	identityTransformation
 }

sdk/testing.go

@@ -217,6 +217,22 @@ func (h *TestHelper) AssertDeepEquals(message string, expected interface{}, actu
 	}
 }
 
+func (h *TestHelper) AssertPanic(message string, funcThatShouldPanic func()) {
+	h.t.Helper()
+	panicked := false
+	func() {
+		defer func() {
+			if x := recover(); x != nil {
+				panicked = true
+			}
+		}()
+		funcThatShouldPanic()
+	}()
+	if !panicked {
+		h.t.Error(message)
+	}
+}
+
 // Encapsulates result of processing an SDK definition. Provides support for
 // checking the state of the build structures.
 type testSdkResult struct {