Add android:replace_instead_of_append

If a property is a pointer to a bool/string, that property would be
replaced instead of appended to when calling ExtendProperties().

This is confusing behavior, and I don't want to give anyone any reason
to use a pointer to a configurable property, so add this relflection
tag to recreate the functionality.

Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I6b9e4dfee1d2685e0c9dc6a646cf45724cc04756

3 files changed, 59 insertions, 25 deletions

diff --git a/proptools/configurable.go b/proptools/configurable.go
index 53cdf23..b42afc7 100644
--- a/proptools/configurable.go
+++ b/proptools/configurable.go
@@ -79,7 +79,8 @@ var _ configurableMarker = Configurable[string]{}.marker
 // appendWrapper exists so that we can set the value of append
 // from a non-pointer method receiver. (setAppend)
 type appendWrapper[T ConfigurableElements] struct {
-	append Configurable[T]
+	append  Configurable[T]
+	replace bool
 }
 
 func (c *Configurable[T]) GetType() parser.SelectType {
@@ -96,12 +97,17 @@ func (c *Configurable[T]) Evaluate(evaluator ConfigurableEvaluator) *T {
 	if c == nil || c.appendWrapper == nil {
 		return nil
 	}
-	return mergeConfiguredValues(
-		c.evaluateNonTransitive(evaluator),
-		c.appendWrapper.append.Evaluate(evaluator),
-		c.propertyName,
-		evaluator,
-	)
+	if c.appendWrapper.replace {
+		return replaceConfiguredValues(
+			c.evaluateNonTransitive(evaluator),
+			c.appendWrapper.append.Evaluate(evaluator),
+		)
+	} else {
+		return appendConfiguredValues(
+			c.evaluateNonTransitive(evaluator),
+			c.appendWrapper.append.Evaluate(evaluator),
+		)
+	}
 }
 
 func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator) *T {
@@ -142,7 +148,7 @@ func (c *Configurable[T]) evaluateNonTransitive(evaluator ConfigurableEvaluator)
 	return nil
 }
 
-func mergeConfiguredValues[T ConfigurableElements](a, b *T, propertyName string, evalutor ConfigurableEvaluator) *T {
+func appendConfiguredValues[T ConfigurableElements](a, b *T) *T {
 	if a == nil && b == nil {
 		return nil
 	}
@@ -188,12 +194,19 @@ func mergeConfiguredValues[T ConfigurableElements](a, b *T, propertyName string,
 	}
 }
 
+func replaceConfiguredValues[T ConfigurableElements](a, b *T) *T {
+	if b != nil {
+		return b
+	}
+	return a
+}
+
 // configurableReflection is an interface that exposes some methods that are
 // helpful when working with reflect.Values of Configurable objects, used by
 // the property unpacking code. You can't call unexported methods from reflection,
 // (at least without unsafe pointer trickery) so this is the next best thing.
 type configurableReflection interface {
-	setAppend(append any)
+	setAppend(append any, replace bool)
 	configuredType() reflect.Type
 	cloneToReflectValuePtr() reflect.Value
 	isEmpty() bool
@@ -216,11 +229,12 @@ func (c *Configurable[T]) initialize(propertyName string, typ parser.SelectType,
 	c.appendWrapper = &appendWrapper[T]{}
 }
 
-func (c Configurable[T]) setAppend(append any) {
+func (c Configurable[T]) setAppend(append any, replace bool) {
 	if c.appendWrapper.append.isEmpty() {
 		c.appendWrapper.append = append.(Configurable[T])
+		c.appendWrapper.replace = replace
 	} else {
-		c.appendWrapper.append.setAppend(append)
+		c.appendWrapper.append.setAppend(append, replace)
 	}
 }
 
@@ -248,6 +262,7 @@ func (c *Configurable[T]) clone() *Configurable[T] {
 		inner = &appendWrapper[T]{}
 		if !c.appendWrapper.append.isEmpty() {
 			inner.append = *c.appendWrapper.append.clone()
+			inner.replace = c.appendWrapper.replace
 		}
 	}
 
diff --git a/proptools/extend.go b/proptools/extend.go
index af4e185..110fb24 100644
--- a/proptools/extend.go
+++ b/proptools/extend.go
@@ -154,9 +154,18 @@ func ExtendMatchingProperties(dst []interface{}, src interface{},
 type Order int
 
 const (
+	// When merging properties, strings and lists will be concatenated, and booleans will be OR'd together
 	Append Order = iota
+	// Same as append, but acts as if the arguments to the extend* functions were swapped. The src value will be
+	// prepended to the dst value instead of appended.
 	Prepend
+	// Instead of concatenating/ORing properties, the dst value will be completely replaced by the src value.
+	// Replace currently only works for slices, maps, and configurable properties. Due to legacy behavior,
+	// pointer properties will always act as if they're using replace ordering.
 	Replace
+	// Same as replace, but acts as if the arguments to the extend* functions were swapped. The src value will be
+	// used only if the dst value was unset.
+	Prepend_replace
 )
 
 type ExtendPropertyFilterFunc func(dstField, srcField reflect.StructField) (bool, error)
@@ -420,6 +429,14 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 				}
 			}
 
+			if HasTag(dstField, "android", "replace_instead_of_append") {
+				if order == Append {
+					order = Replace
+				} else if order == Prepend {
+					order = Prepend_replace
+				}
+			}
+
 			ExtendBasicType(dstFieldValue, srcFieldValue, order)
 		}
 
@@ -438,7 +455,7 @@ func extendPropertiesRecursive(dstValues []reflect.Value, srcValue reflect.Value
 }
 
 func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) {
-	prepend := order == Prepend
+	prepend := order == Prepend || order == Prepend_replace
 
 	switch srcFieldValue.Kind() {
 	case reflect.Struct:
@@ -447,11 +464,18 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) {
 		}
 		if dstFieldValue.Interface().(configurableReflection).isEmpty() {
 			dstFieldValue.Set(srcFieldValue)
-		} else if prepend {
-			srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface())
+		} else if order == Prepend {
+			srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), false)
+			dstFieldValue.Set(srcFieldValue)
+		} else if order == Append {
+			dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface(), false)
+		} else if order == Replace {
+			dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface(), true)
+		} else if order == Prepend_replace {
+			srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface(), true)
 			dstFieldValue.Set(srcFieldValue)
 		} else {
-			dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface())
+			panic(fmt.Sprintf("Unexpected order: %d", order))
 		}
 	case reflect.Bool:
 		// Boolean OR
@@ -550,14 +574,9 @@ func ExtendBasicType(dstFieldValue, srcFieldValue reflect.Value, order Order) {
 			if !isConfigurable(srcFieldValue.Type()) {
 				panic("Should be unreachable")
 			}
-			if dstFieldValue.Interface().(configurableReflection).isEmpty() {
-				dstFieldValue.Set(srcFieldValue)
-			} else if prepend {
-				srcFieldValue.Interface().(configurableReflection).setAppend(dstFieldValue.Interface())
-				dstFieldValue.Set(srcFieldValue)
-			} else {
-				dstFieldValue.Interface().(configurableReflection).setAppend(srcFieldValue.Interface())
-			}
+			panic("Don't use pointers to Configurable properties. All Configurable properties can be unset, " +
+				"and the 'replacing' behavior can be accomplished with the `blueprint:\"replace_instead_of_append\" " +
+				"struct field tag. There's no reason to have a pointer configurable property.")
 		default:
 			panic(fmt.Errorf("unexpected pointer kind %s", ptrKind))
 		}
diff --git a/proptools/unpack.go b/proptools/unpack.go
index 019bdc9..85709cc 100644
--- a/proptools/unpack.go
+++ b/proptools/unpack.go
@@ -307,7 +307,7 @@ func (ctx *unpackContext) unpackToStruct(namePrefix string, structValue reflect.
 			configurableType := fieldValue.Type()
 			configuredType := fieldValue.Interface().(configurableReflection).configuredType()
 			if unpackedValue, ok := ctx.unpackToConfigurable(propertyName, property, configurableType, configuredType); ok {
-				ExtendBasicType(fieldValue, unpackedValue, Append)
+				ExtendBasicType(fieldValue, unpackedValue.Elem(), Append)
 			}
 			if len(ctx.errs) >= maxUnpackErrors {
 				return
@@ -476,7 +476,7 @@ func (ctx *unpackContext) unpackToConfigurable(propertyName string, property *pa
 			if !ok {
 				return reflect.New(configurableType), false
 			}
-			result.Interface().(configurableReflection).setAppend(val.Elem().Interface())
+			result.Interface().(configurableReflection).setAppend(val.Elem().Interface(), false)
 		}
 		return resultPtr, true
 	default: