bpmodify: handle nested properties

Add an option "-property", which is an alias to the option "-parameter".

For example if Android.bp contains:

  cc_foo {
    name: "foo",
  }

Then `bpmodify -m foo -a bar -property baz.buz Android.bp` outputs:

  cc_foo {
    name: "foo",
    baz: {
      buz: ["bar"],
    },
  }

Bug: 149715904
Test: go test -v
Change-Id: I9660cff1b5239ccf5aa9ef1a41835b8ac6cd4b9f

bpmodify/bpmodify.go

@@ -22,18 +22,20 @@ import (
 
 var (
 	// main operation modes
-	list            = flag.Bool("l", false, "list files that would be modified by bpmodify")
+	list             = flag.Bool("l", false, "list files that would be modified by bpmodify")
-	write           = flag.Bool("w", false, "write result to (source) file instead of stdout")
+	write            = flag.Bool("w", false, "write result to (source) file instead of stdout")
-	doDiff          = flag.Bool("d", false, "display diffs instead of rewriting files")
+	doDiff           = flag.Bool("d", false, "display diffs instead of rewriting files")
-	sortLists       = flag.Bool("s", false, "sort touched lists, even if they were unsorted")
+	sortLists        = flag.Bool("s", false, "sort touched lists, even if they were unsorted")
-	parameter       = flag.String("parameter", "deps", "name of parameter to modify on each module")
+	targetedModules  = new(identSet)
-	targetedModules = new(identSet)
+	targetedProperty = new(qualifiedProperty)
-	addIdents       = new(identSet)
+	addIdents        = new(identSet)
-	removeIdents    = new(identSet)
+	removeIdents     = new(identSet)
 )
 
 func init() {
 	flag.Var(targetedModules, "m", "comma or whitespace separated list of modules on which to operate")
+	flag.Var(targetedProperty, "parameter", "alias to -property=`name`")
+	flag.Var(targetedProperty, "property", "fully qualified `name` of property to modify (default \"deps\")")
 	flag.Var(addIdents, "a", "comma or whitespace separated list of identifiers to add")
 	flag.Var(removeIdents, "r", "comma or whitespace separated list of identifiers to remove")
 	flag.Usage = usage
@@ -140,24 +142,74 @@ func findModules(file *parser.File) (modified bool, errs []error) {
 
 func processModule(module *parser.Module, moduleName string,
 	file *parser.File) (modified bool, errs []error) {
-
+	prop, err := getRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes())
-	for _, prop := range module.Properties {
+	if err != nil {
-		if prop.Name == *parameter {
+		return false, []error{err}
-			modified, errs = processParameter(prop.Value, *parameter, moduleName, file)
+	}
-			return
+	if prop == nil {
+		if len(addIdents.idents) == 0 {
+			// We cannot find an existing prop, and we aren't adding anything to the prop,
+			// which means we must be removing something from a non-existing prop,
+			// which means this is a noop.
+			return false, nil
+		}
+		// Else we are adding something to a non-existing prop, so we need to create it first.
+		prop, modified, err = createRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes())
+		if err != nil {
+			// Here should be unreachable, but still handle it for completeness.
+			return false, []error{err}
 		}
 	}
-
+	m, errs := processParameter(prop.Value, targetedProperty.String(), moduleName, file)
-	prop := parser.Property{Name: *parameter, Value: &parser.List{}}
+	modified = modified || m
-	modified, errs = processParameter(prop.Value, *parameter, moduleName, file)
-
-	if modified {
-		module.Properties = append(module.Properties, &prop)
-	}
-
 	return modified, errs
 }
 
+func getRecursiveProperty(module *parser.Module, name string, prefixes []string) (prop *parser.Property, err error) {
+	prop, _, err = getOrCreateRecursiveProperty(module, name, prefixes, false)
+	return prop, err
+}
+
+func createRecursiveProperty(module *parser.Module, name string, prefixes []string) (prop *parser.Property, modified bool, err error) {
+	return getOrCreateRecursiveProperty(module, name, prefixes, true)
+}
+
+func getOrCreateRecursiveProperty(module *parser.Module, name string, prefixes []string,
+	createIfNotFound bool) (prop *parser.Property, modified bool, err error) {
+	m := &module.Map
+	for i, prefix := range prefixes {
+		if prop, found := m.GetProperty(prefix); found {
+			if mm, ok := prop.Value.Eval().(*parser.Map); ok {
+				m = mm
+			} else {
+				// We've found a property in the AST and such property is not of type
+				// *parser.Map, which must mean we didn't modify the AST.
+				return nil, false, fmt.Errorf("Expected property %q to be a map, found %s",
+					strings.Join(prefixes[:i+1], "."), prop.Value.Type())
+			}
+		} else if createIfNotFound {
+			mm := &parser.Map{}
+			m.Properties = append(m.Properties, &parser.Property{Name: prefix, Value: mm})
+			m = mm
+			// We've created a new node in the AST. This means the m.GetProperty(name)
+			// check after this for loop must fail, because the node we inserted is an
+			// empty parser.Map, thus this function will return |modified| is true.
+		} else {
+			return nil, false, nil
+		}
+	}
+	if prop, found := m.GetProperty(name); found {
+		// We've found a property in the AST, which must mean we didn't modify the AST.
+		return prop, false, nil
+	} else if createIfNotFound {
+		prop = &parser.Property{Name: name, Value: &parser.List{}}
+		m.Properties = append(m.Properties, prop)
+		return prop, true, nil
+	} else {
+		return nil, false, nil
+	}
+}
+
 func processParameter(value parser.Expression, paramName, moduleName string,
 	file *parser.File) (modified bool, errs []error) {
 	if _, ok := value.(*parser.Variable); ok {
@@ -232,6 +284,10 @@ func main() {
 
 	flag.Parse()
 
+	if len(targetedProperty.parts) == 0 {
+		targetedProperty.Set("deps")
+	}
+
 	if flag.NArg() == 0 {
 		if *write {
 			report(fmt.Errorf("error: cannot use -w with standard input"))
@@ -318,3 +374,38 @@ func (m *identSet) Set(s string) error {
 func (m *identSet) Get() interface{} {
 	return m.idents
 }
+
+type qualifiedProperty struct {
+	parts []string
+}
+
+var _ flag.Getter = (*qualifiedProperty)(nil)
+
+func (p *qualifiedProperty) name() string {
+	return p.parts[len(p.parts)-1]
+}
+
+func (p *qualifiedProperty) prefixes() []string {
+	return p.parts[:len(p.parts)-1]
+}
+
+func (p *qualifiedProperty) String() string {
+	return strings.Join(p.parts, ".")
+}
+
+func (p *qualifiedProperty) Set(s string) error {
+	p.parts = strings.Split(s, ".")
+	if len(p.parts) == 0 {
+		return fmt.Errorf("%q is not a valid property name", s)
+	}
+	for _, part := range p.parts {
+		if part == "" {
+			return fmt.Errorf("%q is not a valid property name", s)
+		}
+	}
+	return nil
+}
+
+func (p *qualifiedProperty) Get() interface{} {
+	return p.parts
+}

bpmodify/bpmodify_test.go

@@ -0,0 +1,259 @@
+// Copyright 2020 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 main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/google/blueprint/parser"
+)
+
+var testCases = []struct {
+	input     string
+	output    string
+	property  string
+	addSet    string
+	removeSet string
+}{
+	{
+		`
+		cc_foo {
+			name: "foo",
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			deps: ["bar"],
+		}
+		`,
+		"deps",
+		"bar",
+		"",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+			deps: ["bar"],
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			deps: [],
+		}
+		`,
+		"deps",
+		"",
+		"bar",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"dep2",
+						"nested_dep",],
+				},
+			},
+		}
+		`,
+		"arch.arm.deps",
+		"nested_dep,dep2",
+		"",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"dep2",
+						"nested_dep",
+					],
+				},
+			},
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+					],
+				},
+			},
+		}
+		`,
+		"arch.arm.deps",
+		"",
+		"nested_dep,dep2",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"nested_dep",
+						"dep2",
+					],
+				},
+			},
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"nested_dep",
+						"dep2",
+					],
+				},
+			},
+		}
+		`,
+		"arch.arm.deps",
+		"dep2,dep2",
+		"",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"nested_dep",
+						"dep2",
+					],
+				},
+			},
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			arch: {
+				arm: {
+					deps: [
+						"nested_dep",
+						"dep2",
+					],
+				},
+			},
+		}
+		`,
+		"arch.arm.deps",
+		"",
+		"dep3,dep4",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+		}
+		`,
+		"deps",
+		"",
+		"bar",
+	},
+	{
+		`
+		cc_foo {
+			name: "foo",
+			arch: {},
+		}
+		`,
+		`
+		cc_foo {
+			name: "foo",
+			arch: {},
+		}
+		`,
+		"arch.arm.deps",
+		"",
+		"dep3,dep4",
+	},
+}
+
+func simplifyModuleDefinition(def string) string {
+	var result string
+	for _, line := range strings.Split(def, "\n") {
+		result += strings.TrimSpace(line)
+	}
+	return result
+}
+
+func TestProcessModule(t *testing.T) {
+	for i, testCase := range testCases {
+		targetedProperty.Set(testCase.property)
+		addIdents.Set(testCase.addSet)
+		removeIdents.Set(testCase.removeSet)
+
+		inAst, errs := parser.ParseAndEval("", strings.NewReader(testCase.input), parser.NewScope(nil))
+		if len(errs) > 0 {
+			t.Errorf("test case %d:", i)
+			for _, err := range errs {
+				t.Errorf("  %s", err)
+			}
+			t.Errorf("failed to parse:")
+			t.Errorf("%+v", testCase)
+			continue
+		}
+
+		if inModule, ok := inAst.Defs[0].(*parser.Module); !ok {
+			t.Errorf("test case %d:", i)
+			t.Errorf("  input must only contain a single module definition: %s", testCase.input)
+			continue
+		} else {
+			_, errs := processModule(inModule, "", inAst)
+			if len(errs) > 0 {
+				t.Errorf("test case %d:", i)
+				for _, err := range errs {
+					t.Errorf("  %s", err)
+				}
+			}
+			inModuleText, _ := parser.Print(inAst)
+			inModuleString := string(inModuleText)
+			if simplifyModuleDefinition(inModuleString) != simplifyModuleDefinition(testCase.output) {
+				t.Errorf("test case %d:", i)
+				t.Errorf("expected module definition:")
+				t.Errorf("  %s", testCase.output)
+				t.Errorf("actual module definition:")
+				t.Errorf("  %s", inModuleString)
+			}
+		}
+	}
+}