Merge matching properties in bpfix
androidmk will start to generate multiple static_libs: properties
in a single module, add a pass in bpfix to fix them up into a
single property.
Test: bpfix_test.go
Change-Id: I30955b6efbb767c02ba77f2f18d44951ef094bad
diff --git a/androidmk/cmd/androidmk/androidmk_test.go b/androidmk/cmd/androidmk/androidmk_test.go
index 45df1a5..dd646ef 100644
--- a/androidmk/cmd/androidmk/androidmk_test.go
+++ b/androidmk/cmd/androidmk/androidmk_test.go
@@ -15,13 +15,11 @@
package main
import (
+ "android/soong/bpfix/bpfix"
"bytes"
"fmt"
- "os"
"strings"
"testing"
-
- bpparser "github.com/google/blueprint/parser"
)
var testCases = []struct {
@@ -524,26 +522,12 @@
},
}
-func reformatBlueprint(input string) string {
- file, errs := bpparser.Parse("<testcase>", bytes.NewBufferString(input), bpparser.NewScope(nil))
- if len(errs) > 0 {
- for _, err := range errs {
- fmt.Fprintln(os.Stderr, err)
- }
- panic(fmt.Sprintf("%d parsing errors in testcase:\n%s", len(errs), input))
- }
-
- res, err := bpparser.Print(file)
- if err != nil {
- panic(fmt.Sprintf("Error printing testcase: %q", err))
- }
-
- return string(res)
-}
-
func TestEndToEnd(t *testing.T) {
for i, test := range testCases {
- expected := reformatBlueprint(test.expected)
+ expected, err := bpfix.Reformat(test.expected)
+ if err != nil {
+ t.Error(err)
+ }
got, errs := convertFile(fmt.Sprintf("<testcase %d>", i), bytes.NewBufferString(test.in))
if len(errs) > 0 {
diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go
index 4300e81..4ab42a4 100644
--- a/bpfix/bpfix/bpfix.go
+++ b/bpfix/bpfix/bpfix.go
@@ -26,11 +26,27 @@
"github.com/google/blueprint/parser"
)
+// Reformat takes a blueprint file as a string and returns a formatted version
+func Reformat(input string) (string, error) {
+ tree, err := parse("<string>", bytes.NewBufferString(input))
+ if err != nil {
+ return "", err
+ }
+
+ res, err := parser.Print(tree)
+ if err != nil {
+ return "", err
+ }
+
+ return string(res), nil
+}
+
// A FixRequest specifies the details of which fixes to apply to an individual file
// A FixRequest doesn't specify whether to do a dry run or where to write the results; that's in cmd/bpfix.go
type FixRequest struct {
simplifyKnownRedundantVariables bool
rewriteIncorrectAndroidmkPrebuilts bool
+ mergeMatchingModuleProperties bool
}
func NewFixRequest() FixRequest {
@@ -41,6 +57,7 @@
result = r
result.simplifyKnownRedundantVariables = true
result.rewriteIncorrectAndroidmkPrebuilts = true
+ result.mergeMatchingModuleProperties = true
return result
}
@@ -136,6 +153,13 @@
return err
}
}
+
+ if config.mergeMatchingModuleProperties {
+ err := f.mergeMatchingModuleProperties()
+ if err != nil {
+ return err
+ }
+ }
return nil
}
@@ -179,6 +203,115 @@
return nil
}
+func (f *Fixer) mergeMatchingModuleProperties() error {
+ // Make sure all the offsets are accurate
+ buf, err := f.reparse()
+ if err != nil {
+ return err
+ }
+
+ var patchlist parser.PatchList
+ for _, def := range f.tree.Defs {
+ mod, ok := def.(*parser.Module)
+ if !ok {
+ continue
+ }
+
+ err := mergeMatchingProperties(&mod.Properties, buf, &patchlist)
+ if err != nil {
+ return err
+ }
+ }
+
+ newBuf := new(bytes.Buffer)
+ err = patchlist.Apply(bytes.NewReader(buf), newBuf)
+ if err != nil {
+ return err
+ }
+
+ newTree, err := parse(f.tree.Name, newBuf)
+ if err != nil {
+ return err
+ }
+
+ f.tree = newTree
+
+ return nil
+}
+
+func mergeMatchingProperties(properties *[]*parser.Property, buf []byte, patchlist *parser.PatchList) error {
+ seen := make(map[string]*parser.Property)
+ for i := 0; i < len(*properties); i++ {
+ property := (*properties)[i]
+ if prev, exists := seen[property.Name]; exists {
+ err := mergeProperties(prev, property, buf, patchlist)
+ if err != nil {
+ return err
+ }
+ *properties = append((*properties)[:i], (*properties)[i+1:]...)
+ } else {
+ seen[property.Name] = property
+ if mapProperty, ok := property.Value.(*parser.Map); ok {
+ err := mergeMatchingProperties(&mapProperty.Properties, buf, patchlist)
+ if err != nil {
+ return err
+ }
+ }
+ }
+ }
+ return nil
+}
+
+func mergeProperties(a, b *parser.Property, buf []byte, patchlist *parser.PatchList) error {
+ if a.Value.Type() != b.Value.Type() {
+ return fmt.Errorf("type mismatch when merging properties %q: %s and %s", a.Name, a.Value.Type(), b.Value.Type())
+ }
+
+ switch a.Value.Type() {
+ case parser.StringType:
+ return fmt.Errorf("conflicting definitions of string property %q", a.Name)
+ case parser.ListType:
+ return mergeListProperties(a, b, buf, patchlist)
+ }
+
+ return nil
+}
+
+func mergeListProperties(a, b *parser.Property, buf []byte, patchlist *parser.PatchList) error {
+ aval, oka := a.Value.(*parser.List)
+ bval, okb := b.Value.(*parser.List)
+ if !oka || !okb {
+ // Merging expressions not supported yet
+ return nil
+ }
+
+ s := string(buf[bval.LBracePos.Offset+1 : bval.RBracePos.Offset])
+ if bval.LBracePos.Line != bval.RBracePos.Line {
+ if s[0] != '\n' {
+ panic("expected \n")
+ }
+ // If B is a multi line list, skip the first "\n" in case A already has a trailing "\n"
+ s = s[1:]
+ }
+ if aval.LBracePos.Line == aval.RBracePos.Line {
+ // A is a single line list with no trailing comma
+ if len(aval.Values) > 0 {
+ s = "," + s
+ }
+ }
+
+ err := patchlist.Add(aval.RBracePos.Offset, aval.RBracePos.Offset, s)
+ if err != nil {
+ return err
+ }
+ err = patchlist.Add(b.NamePos.Offset, b.End().Offset+2, "")
+ if err != nil {
+ return err
+ }
+
+ return nil
+}
+
// removes from <items> every item present in <removals>
func filterExpressionList(items *parser.List, removals *parser.List) {
writeIndex := 0
diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go
index 1e1e693..51708eb 100644
--- a/bpfix/bpfix/bpfix_test.go
+++ b/bpfix/bpfix/bpfix_test.go
@@ -17,6 +17,7 @@
package bpfix
import (
+ "bytes"
"fmt"
"strings"
"testing"
@@ -115,3 +116,130 @@
implFilterListTest(t, []string{}, []string{"include"}, []string{})
implFilterListTest(t, []string{}, []string{}, []string{})
}
+
+func TestMergeMatchingProperties(t *testing.T) {
+ tests := []struct {
+ name string
+ in string
+ out string
+ }{
+ {
+ name: "empty",
+ in: `
+ java_library {
+ name: "foo",
+ static_libs: [],
+ static_libs: [],
+ }
+ `,
+ out: `
+ java_library {
+ name: "foo",
+ static_libs: [],
+ }
+ `,
+ },
+ {
+ name: "single line into multiline",
+ in: `
+ java_library {
+ name: "foo",
+ static_libs: [
+ "a",
+ "b",
+ ],
+ //c1
+ static_libs: ["c" /*c2*/],
+ }
+ `,
+ out: `
+ java_library {
+ name: "foo",
+ static_libs: [
+ "a",
+ "b",
+ "c", /*c2*/
+ ],
+ //c1
+ }
+ `,
+ },
+ {
+ name: "multiline into multiline",
+ in: `
+ java_library {
+ name: "foo",
+ static_libs: [
+ "a",
+ "b",
+ ],
+ //c1
+ static_libs: [
+ //c2
+ "c", //c3
+ "d",
+ ],
+ }
+ `,
+ out: `
+ java_library {
+ name: "foo",
+ static_libs: [
+ "a",
+ "b",
+ //c2
+ "c", //c3
+ "d",
+ ],
+ //c1
+ }
+ `,
+ },
+ }
+
+ for _, test := range tests {
+ t.Run(test.name, func(t *testing.T) {
+ expected, err := Reformat(test.out)
+ if err != nil {
+ t.Error(err)
+ }
+
+ in, err := Reformat(test.in)
+ if err != nil {
+ t.Error(err)
+ }
+
+ tree, errs := parser.Parse("<testcase>", bytes.NewBufferString(in), parser.NewScope(nil))
+ if errs != nil {
+ t.Fatal(errs)
+ }
+
+ fixer := NewFixer(tree)
+
+ got := ""
+ prev := "foo"
+ passes := 0
+ for got != prev && passes < 10 {
+ err := fixer.mergeMatchingModuleProperties()
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ out, err := parser.Print(fixer.tree)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ prev = got
+ got = string(out)
+ passes++
+ }
+
+ if got != expected {
+ t.Errorf("failed testcase '%s'\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n",
+ test.name, in, expected, got)
+ }
+
+ })
+ }
+}