Fix issues with bp transformation

Returning nil from transformPropertySet in order to remove it did
not work because it ends up comparing as (*bpPropertySet, nil) and
not an untyped nil which causes the test against nil to fail.

This change adds tests to check that returning nil will delete a
property/property set from the containing property set and fixes the
code so that it passes the tests. It extracts common code to transform
a property set and its contents as well as code for creating new
property sets.

Bug: 148933848
Test: m nothing
Change-Id: I35dc3c39c76e701821891622615c09b094cf697f

Android.bp

@@ -536,6 +536,7 @@ bootstrap_go_package {
         "sdk/update.go",
     ],
     testSrcs: [
+        "sdk/bp_test.go",
         "sdk/cc_sdk_test.go",
         "sdk/exports_test.go",
         "sdk/java_sdk_test.go",

sdk/bp.go

@@ -48,8 +48,7 @@ func (s *bpPropertySet) AddPropertyWithTag(name string, value interface{}, tag a
 }
 
 func (s *bpPropertySet) AddPropertySet(name string) android.BpPropertySet {
-	set := &bpPropertySet{}
+	set := newPropertySet()
-	set.init()
 	s.AddProperty(name, set)
 	return set
 }
@@ -62,7 +61,7 @@ func (s *bpPropertySet) getTag(name string) interface{} {
 	return s.tags[name]
 }
 
-func (s *bpPropertySet) transform(transformer bpPropertyTransformer) {
+func (s *bpPropertySet) transformContents(transformer bpPropertyTransformer) {
 	var newOrder []string
 	for _, name := range s.order {
 		value := s.properties[name]
@@ -70,7 +69,13 @@ func (s *bpPropertySet) transform(transformer bpPropertyTransformer) {
 		var newValue interface{}
 		var newTag android.BpPropertyTag
 		if propertySet, ok := value.(*bpPropertySet); ok {
-			newValue, newTag = transformer.transformPropertySet(name, propertySet, tag)
+			var newPropertySet *bpPropertySet
+			newPropertySet, newTag = transformPropertySet(transformer, name, propertySet, tag)
+			if newPropertySet == nil {
+				newValue = nil
+			} else {
+				newValue = newPropertySet
+			}
 		} else {
 			newValue, newTag = transformer.transformProperty(name, value, tag)
 		}
@@ -88,6 +93,14 @@ func (s *bpPropertySet) transform(transformer bpPropertyTransformer) {
 	s.order = newOrder
 }
 
+func transformPropertySet(transformer bpPropertyTransformer, name string, propertySet *bpPropertySet, tag android.BpPropertyTag) (*bpPropertySet, android.BpPropertyTag) {
+	newPropertySet, newTag := transformer.transformPropertySet(name, propertySet, tag)
+	if newPropertySet != nil {
+		newPropertySet.transformContents(transformer)
+	}
+	return newPropertySet, newTag
+}
+
 func (s *bpPropertySet) setProperty(name string, value interface{}) {
 	if s.properties[name] == nil {
 		s.AddProperty(name, value)
@@ -180,8 +193,7 @@ func (m *bpModule) deepCopy() *bpModule {
 func (m *bpModule) transform(transformer bpTransformer) *bpModule {
 	transformedModule := transformer.transformModule(m)
 	// Copy the contents of the returned property set into the module and then transform that.
-	transformedModule.bpPropertySet, _ = transformer.transformPropertySet("", transformedModule.bpPropertySet, nil)
+	transformedModule.bpPropertySet, _ = transformPropertySet(transformer, "", transformedModule.bpPropertySet, nil)
-	transformedModule.bpPropertySet.transform(transformer)
 	return transformedModule
 }
 
@@ -253,10 +265,19 @@ func (f *bpFile) AddModule(module android.BpModule) {
 }
 
 func (f *bpFile) newModule(moduleType string) *bpModule {
+	return newModule(moduleType)
+}
+
+func newModule(moduleType string) *bpModule {
 	module := &bpModule{
 		moduleType:    moduleType,
-		bpPropertySet: &bpPropertySet{},
+		bpPropertySet: newPropertySet(),
 	}
-	module.bpPropertySet.init()
 	return module
 }
+
+func newPropertySet() *bpPropertySet {
+	set := &bpPropertySet{}
+	set.init()
+	return set
+}

sdk/bp_test.go

@@ -0,0 +1,69 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// 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 sdk
+
+import (
+	"testing"
+
+	"android/soong/android"
+)
+
+type removeFredTransformation struct {
+	identityTransformation
+}
+
+func (t removeFredTransformation) transformProperty(name string, value interface{}, tag android.BpPropertyTag) (interface{}, android.BpPropertyTag) {
+	if name == "fred" {
+		return nil, nil
+	}
+	return value, tag
+}
+
+func (t removeFredTransformation) transformPropertySet(name string, propertySet *bpPropertySet, tag android.BpPropertyTag) (*bpPropertySet, android.BpPropertyTag) {
+	if name == "fred" {
+		return nil, nil
+	}
+	return propertySet, tag
+}
+
+func TestTransformRemoveProperty(t *testing.T) {
+
+	helper := &TestHelper{t}
+
+	set := newPropertySet()
+	set.AddProperty("name", "name")
+	set.AddProperty("fred", "12")
+
+	set.transformContents(removeFredTransformation{})
+
+	contents := &generatedContents{}
+	outputPropertySet(contents, set)
+	helper.AssertTrimmedStringEquals("removing property failed", "name: \"name\",\\n", contents.content.String())
+}
+
+func TestTransformRemovePropertySet(t *testing.T) {
+
+	helper := &TestHelper{t}
+
+	set := newPropertySet()
+	set.AddProperty("name", "name")
+	set.AddPropertySet("fred")
+
+	set.transformContents(removeFredTransformation{})
+
+	contents := &generatedContents{}
+	outputPropertySet(contents, set)
+	helper.AssertTrimmedStringEquals("removing property set failed", "name: \"name\",\\n", contents.content.String())
+}