Support %s in product variable properties

Support using strings as product variable substitutions, and
add tests for printfIntoProperty.

Test: varaible_test.go
Change-Id: I06cfadfb1d3fc81da72fb71323706df20426c8b7

Android.bp

@@ -72,6 +72,7 @@ bootstrap_go_package {
         "android/expand_test.go",
         "android/paths_test.go",
         "android/prebuilt_test.go",
+        "android/variable_test.go",
     ],
 }
 

android/variable.go

@@ -230,7 +230,7 @@ func variableMutator(mctx BottomUpMutatorContext) {
 func (a *ModuleBase) setVariableProperties(ctx BottomUpMutatorContext,
 	prefix string, productVariablePropertyValue reflect.Value, variableValue interface{}) {
 
-	printfIntoProperties(productVariablePropertyValue, variableValue)
+	printfIntoProperties(ctx, prefix, productVariablePropertyValue, variableValue)
 
 	err := proptools.AppendMatchingProperties(a.generalProperties,
 		productVariablePropertyValue.Addr().Interface(), nil)
@@ -243,7 +243,17 @@ func (a *ModuleBase) setVariableProperties(ctx BottomUpMutatorContext,
 	}
 }
 
-func printfIntoProperties(productVariablePropertyValue reflect.Value, variableValue interface{}) {
+func printfIntoPropertiesError(ctx BottomUpMutatorContext, prefix string,
+	productVariablePropertyValue reflect.Value, i int, err error) {
+
+	field := productVariablePropertyValue.Type().Field(i).Name
+	property := prefix + "." + proptools.PropertyNameForField(field)
+	ctx.PropertyErrorf(property, "%s", err)
+}
+
+func printfIntoProperties(ctx BottomUpMutatorContext, prefix string,
+	productVariablePropertyValue reflect.Value, variableValue interface{}) {
+
 	for i := 0; i < productVariablePropertyValue.NumField(); i++ {
 		propertyValue := productVariablePropertyValue.Field(i)
 		kind := propertyValue.Kind()
@@ -255,36 +265,64 @@ func printfIntoProperties(productVariablePropertyValue reflect.Value, variableVa
 		}
 		switch propertyValue.Kind() {
 		case reflect.String:
-			printfIntoProperty(propertyValue, variableValue)
+			err := printfIntoProperty(propertyValue, variableValue)
+			if err != nil {
+				printfIntoPropertiesError(ctx, prefix, productVariablePropertyValue, i, err)
+			}
 		case reflect.Slice:
 			for j := 0; j < propertyValue.Len(); j++ {
-				printfIntoProperty(propertyValue.Index(j), variableValue)
+				err := printfIntoProperty(propertyValue.Index(j), variableValue)
+				if err != nil {
+					printfIntoPropertiesError(ctx, prefix, productVariablePropertyValue, i, err)
+				}
 			}
 		case reflect.Bool:
 			// Nothing
 		case reflect.Struct:
-			printfIntoProperties(propertyValue, variableValue)
+			printfIntoProperties(ctx, prefix, propertyValue, variableValue)
 		default:
 			panic(fmt.Errorf("unsupported field kind %q", propertyValue.Kind()))
 		}
 	}
 }
 
-func printfIntoProperty(propertyValue reflect.Value, variableValue interface{}) {
+func printfIntoProperty(propertyValue reflect.Value, variableValue interface{}) error {
 	s := propertyValue.String()
-	// For now, we only support int formats
+
-	var i int
+	count := strings.Count(s, "%")
+	if count == 0 {
+		return nil
+	}
+
+	if count > 1 {
+		return fmt.Errorf("product variable properties only support a single '%%'")
+	}
+
 	if strings.Contains(s, "%d") {
 		switch v := variableValue.(type) {
 		case int:
-			i = v
+			// Nothing
 		case bool:
 			if v {
-				i = 1
+				variableValue = 1
+			} else {
+				variableValue = 0
 			}
 		default:
-			panic(fmt.Errorf("unsupported type %T", variableValue))
+			return fmt.Errorf("unsupported type %T for %%d", variableValue)
 		}
-		propertyValue.Set(reflect.ValueOf(fmt.Sprintf(s, i)))
+	} else if strings.Contains(s, "%s") {
+		switch variableValue.(type) {
+		case string:
+			// Nothing
+		default:
+			return fmt.Errorf("unsupported type %T for %%s", variableValue)
+		}
+	} else {
+		return fmt.Errorf("unsupported %% in product variable property")
 	}
+
+	propertyValue.Set(reflect.ValueOf(fmt.Sprintf(s, variableValue)))
+
+	return nil
 }

android/variable_test.go

@@ -0,0 +1,124 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package android
+
+import (
+	"reflect"
+	"testing"
+)
+
+type printfIntoPropertyTestCase struct {
+	in  string
+	val interface{}
+	out string
+	err bool
+}
+
+var printfIntoPropertyTestCases = []printfIntoPropertyTestCase{
+	{
+		in:  "%d",
+		val: 0,
+		out: "0",
+	},
+	{
+		in:  "%d",
+		val: 1,
+		out: "1",
+	},
+	{
+		in:  "%d",
+		val: 2,
+		out: "2",
+	},
+	{
+		in:  "%d",
+		val: false,
+		out: "0",
+	},
+	{
+		in:  "%d",
+		val: true,
+		out: "1",
+	},
+	{
+		in:  "%d",
+		val: -1,
+		out: "-1",
+	},
+
+	{
+		in:  "-DA=%d",
+		val: 1,
+		out: "-DA=1",
+	},
+	{
+		in:  "-DA=%du",
+		val: 1,
+		out: "-DA=1u",
+	},
+	{
+		in:  "-DA=%s",
+		val: "abc",
+		out: "-DA=abc",
+	},
+	{
+		in:  `-DA="%s"`,
+		val: "abc",
+		out: `-DA="abc"`,
+	},
+
+	{
+		in:  "%%",
+		err: true,
+	},
+	{
+		in:  "%d%s",
+		err: true,
+	},
+	{
+		in:  "%d,%s",
+		err: true,
+	},
+	{
+		in:  "%d",
+		val: "",
+		err: true,
+	},
+	{
+		in:  "%d",
+		val: 1.5,
+		err: true,
+	},
+	{
+		in:  "%f",
+		val: 1.5,
+		err: true,
+	},
+}
+
+func TestPrintfIntoProperty(t *testing.T) {
+	for _, testCase := range printfIntoPropertyTestCases {
+		s := testCase.in
+		v := reflect.ValueOf(&s).Elem()
+		err := printfIntoProperty(v, testCase.val)
+		if err != nil && !testCase.err {
+			t.Errorf("unexpected error %s", err)
+		} else if err == nil && testCase.err {
+			t.Errorf("expected error")
+		} else if err == nil && v.String() != testCase.out {
+			t.Errorf("expected %q got %q", testCase.out, v.String())
+		}
+	}
+}