From b138d49adf2799dd18c2877f8920a86b2c5c111a Mon Sep 17 00:00:00 2001 From: Yo Chiang Date: Wed, 4 Mar 2020 20:53:21 +0800 Subject: [PATCH] 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 | 133 ++++++++++++++++---- bpmodify/bpmodify_test.go | 259 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 371 insertions(+), 21 deletions(-) create mode 100644 bpmodify/bpmodify_test.go diff --git a/bpmodify/bpmodify.go b/bpmodify/bpmodify.go index 29e97d1..0190f19 100644 --- a/bpmodify/bpmodify.go +++ b/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") - write = flag.Bool("w", false, "write result to (source) file instead of stdout") - doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") - 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) - addIdents = new(identSet) - removeIdents = new(identSet) + 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") + doDiff = flag.Bool("d", false, "display diffs instead of rewriting files") + sortLists = flag.Bool("s", false, "sort touched lists, even if they were unsorted") + targetedModules = new(identSet) + targetedProperty = new(qualifiedProperty) + addIdents = 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) { - - for _, prop := range module.Properties { - if prop.Name == *parameter { - modified, errs = processParameter(prop.Value, *parameter, moduleName, file) - return + prop, err := getRecursiveProperty(module, targetedProperty.name(), targetedProperty.prefixes()) + if err != nil { + return false, []error{err} + } + 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} } } - - prop := parser.Property{Name: *parameter, Value: &parser.List{}} - modified, errs = processParameter(prop.Value, *parameter, moduleName, file) - - if modified { - module.Properties = append(module.Properties, &prop) - } - + m, errs := processParameter(prop.Value, targetedProperty.String(), moduleName, file) + modified = modified || m 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 +} diff --git a/bpmodify/bpmodify_test.go b/bpmodify/bpmodify_test.go new file mode 100644 index 0000000..a498a14 --- /dev/null +++ b/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) + } + } + } +}