refactoring build_conversion.go to remove panic

Returning errors throughout instead of calling panic()
Errors will be more useful for testing

Test: bp2build tests
Change-Id: I3b03f0a30e7a80878e91c7f0e2df5a94d9d6b780

bp2build/build_conversion.go

@@ -289,7 +289,9 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 						return
 					}
 				}
-				targets = generateBazelTargets(bpCtx, aModule)
+				var targetErrs []error
+				targets, targetErrs = generateBazelTargets(bpCtx, aModule)
+				errs = append(errs, targetErrs...)
 				for _, t := range targets {
 					// A module can potentially generate more than 1 Bazel
 					// target, each of a different rule class.
@@ -306,7 +308,10 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 				// be mapped cleanly to a bazel label.
 				return
 			}
-			t := generateSoongModuleTarget(bpCtx, m)
+			t, err := generateSoongModuleTarget(bpCtx, m)
+			if err != nil {
+				errs = append(errs, err)
+			}
 			targets = append(targets, t)
 		default:
 			errs = append(errs, fmt.Errorf("Unknown code-generation mode: %s", ctx.Mode()))
@@ -347,12 +352,18 @@ func GenerateBazelTargets(ctx *CodegenContext, generateFilegroups bool) (convers
 	}, errs
 }
 
-func generateBazelTargets(ctx bpToBuildContext, m android.Module) []BazelTarget {
+func generateBazelTargets(ctx bpToBuildContext, m android.Module) ([]BazelTarget, []error) {
 	var targets []BazelTarget
+	var errs []error
 	for _, m := range m.Bp2buildTargets() {
-		targets = append(targets, generateBazelTarget(ctx, m))
+		target, err := generateBazelTarget(ctx, m)
+		if err != nil {
+			errs = append(errs, err)
+			return targets, errs
 		}
-	return targets
+		targets = append(targets, target)
+	}
+	return targets, errs
 }
 
 type bp2buildModule interface {
@@ -363,13 +374,16 @@ type bp2buildModule interface {
 	BazelAttributes() []interface{}
 }
 
-func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget {
+func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) (BazelTarget, error) {
 	ruleClass := m.BazelRuleClass()
 	bzlLoadLocation := m.BazelRuleLoadLocation()
 
 	// extract the bazel attributes from the module.
 	attrs := m.BazelAttributes()
-	props := extractModuleProperties(attrs, true)
+	props, err := extractModuleProperties(attrs, true)
+	if err != nil {
+		return BazelTarget{}, err
+	}
 
 	// name is handled in a special manner
 	delete(props.Attrs, "name")
@@ -389,13 +403,13 @@ func generateBazelTarget(ctx bpToBuildContext, m bp2buildModule) BazelTarget {
 			targetName,
 			attributes,
 		),
-	}
+	}, nil
 }
 
 // Convert a module and its deps and props into a Bazel macro/rule
 // representation in the BUILD file.
-func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) BazelTarget {
+func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) (BazelTarget, error) {
-	props := getBuildProperties(ctx, m)
+	props, err := getBuildProperties(ctx, m)
 
 	// TODO(b/163018919): DirectDeps can have duplicate (module, variant)
 	// items, if the modules are added using different DependencyTag. Figure
@@ -429,21 +443,21 @@ func generateSoongModuleTarget(ctx bpToBuildContext, m blueprint.Module) BazelTa
 			ctx.ModuleSubDir(m),
 			depLabelList,
 			attributes),
-	}
+	}, err
 }
 
-func getBuildProperties(ctx bpToBuildContext, m blueprint.Module) BazelAttributes {
+func getBuildProperties(ctx bpToBuildContext, m blueprint.Module) (BazelAttributes, error) {
 	// TODO: this omits properties for blueprint modules (blueprint_go_binary,
 	// bootstrap_go_binary, bootstrap_go_package), which will have to be handled separately.
 	if aModule, ok := m.(android.Module); ok {
 		return extractModuleProperties(aModule.GetProperties(), false)
 	}
 
-	return BazelAttributes{}
+	return BazelAttributes{}, nil
 }
 
 // Generically extract module properties and types into a map, keyed by the module property name.
-func extractModuleProperties(props []interface{}, checkForDuplicateProperties bool) BazelAttributes {
+func extractModuleProperties(props []interface{}, checkForDuplicateProperties bool) (BazelAttributes, error) {
 	ret := map[string]string{}
 
 	// Iterate over this android.Module's property structs.
@@ -456,24 +470,29 @@ func extractModuleProperties(props []interface{}, checkForDuplicateProperties bo
 		// manipulate internal props, if needed.
 		if isStructPtr(propertiesValue.Type()) {
 			structValue := propertiesValue.Elem()
-			for k, v := range extractStructProperties(structValue, 0) {
+			ok, err := extractStructProperties(structValue, 0)
+			if err != nil {
+				return BazelAttributes{}, err
+			}
+			for k, v := range ok {
 				if existing, exists := ret[k]; checkForDuplicateProperties && exists {
-					panic(fmt.Errorf(
+					return BazelAttributes{}, fmt.Errorf(
 						"%s (%v) is present in properties whereas it should be consolidated into a commonAttributes",
-						k, existing))
+						k, existing)
 				}
 				ret[k] = v
 			}
 		} else {
-			panic(fmt.Errorf(
+			return BazelAttributes{},
+				fmt.Errorf(
 					"properties must be a pointer to a struct, got %T",
-				propertiesValue.Interface()))
+					propertiesValue.Interface())
 		}
 	}
 
 	return BazelAttributes{
 		Attrs: ret,
-	}
+	}, nil
 }
 
 func isStructPtr(t reflect.Type) bool {
@@ -531,7 +550,12 @@ func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (
 		}
 
 		// Sort and print the struct props by the key.
-		structProps := extractStructProperties(propertyValue, indent)
+		structProps, err := extractStructProperties(propertyValue, indent)
+
+		if err != nil {
+			return "", err
+		}
+
 		if len(structProps) == 0 {
 			return "", nil
 		}
@@ -550,11 +574,13 @@ func prettyPrint(propertyValue reflect.Value, indent int, emitZeroValues bool) (
 // which each property value correctly pretty-printed and indented at the right nest level,
 // since property structs can be nested. In Starlark, nested structs are represented as nested
 // dicts: https://docs.bazel.build/skylark/lib/dict.html
-func extractStructProperties(structValue reflect.Value, indent int) map[string]string {
+func extractStructProperties(structValue reflect.Value, indent int) (map[string]string, error) {
 	if structValue.Kind() != reflect.Struct {
-		panic(fmt.Errorf("Expected a reflect.Struct type, but got %s", structValue.Kind()))
+		return map[string]string{}, fmt.Errorf("Expected a reflect.Struct type, but got %s", structValue.Kind())
 	}
 
+	var err error
+
 	ret := map[string]string{}
 	structType := structValue.Type()
 	for i := 0; i < structValue.NumField(); i++ {
@@ -575,7 +601,10 @@ func extractStructProperties(structValue reflect.Value, indent int) map[string]s
 				fieldValue = fieldValue.Elem()
 			}
 			if fieldValue.Type().Kind() == reflect.Struct {
-				propsToMerge := extractStructProperties(fieldValue, indent)
+				propsToMerge, err := extractStructProperties(fieldValue, indent)
+				if err != nil {
+					return map[string]string{}, err
+				}
 				for prop, value := range propsToMerge {
 					ret[prop] = value
 				}
@@ -584,20 +613,20 @@ func extractStructProperties(structValue reflect.Value, indent int) map[string]s
 		}
 
 		propertyName := proptools.PropertyNameForField(field.Name)
-		prettyPrintedValue, err := prettyPrint(fieldValue, indent+1, false)
+		var prettyPrintedValue string
+		prettyPrintedValue, err = prettyPrint(fieldValue, indent+1, false)
 		if err != nil {
-			panic(
+			return map[string]string{}, fmt.Errorf(
-				fmt.Errorf(
 				"Error while parsing property: %q. %s",
 				propertyName,
-					err))
+				err)
 		}
 		if prettyPrintedValue != "" {
 			ret[propertyName] = prettyPrintedValue
 		}
 	}
 
-	return ret
+	return ret, nil
 }
 
 func isZero(value reflect.Value) bool {

bp2build/testing.go

@@ -384,6 +384,9 @@ func customBp2buildOneToMany(ctx android.TopDownMutatorContext, m *customModule)
 func generateBazelTargetsForDir(codegenCtx *CodegenContext, dir string) (BazelTargets, []error) {
 	// TODO: Set generateFilegroups to true and/or remove the generateFilegroups argument completely
 	res, err := GenerateBazelTargets(codegenCtx, false)
+	if err != nil {
+		return BazelTargets{}, err
+	}
 	return res.buildFileToTargets[dir], err
 }