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

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 @@ include $(call all-makefiles-under,$(LOCAL_PATH))
 	},
 }
 
-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 {

bpfix/bpfix/bpfix.go

@@ -26,11 +26,27 @@ import (
 	"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 @@ func (r FixRequest) AddAll() (result FixRequest) {
 	result = r
 	result.simplifyKnownRedundantVariables = true
 	result.rewriteIncorrectAndroidmkPrebuilts = true
+	result.mergeMatchingModuleProperties = true
 	return result
 }
 
@@ -136,6 +153,13 @@ func (f *Fixer) fixTreeOnce(config FixRequest) error {
 			return err
 		}
 	}
+
+	if config.mergeMatchingModuleProperties {
+		err := f.mergeMatchingModuleProperties()
+		if err != nil {
+			return err
+		}
+	}
 	return nil
 }
 
@@ -179,6 +203,115 @@ func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error {
 	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

bpfix/bpfix/bpfix_test.go

@@ -17,6 +17,7 @@
 package bpfix
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 	"testing"
@@ -115,3 +116,130 @@ func TestSimplifyKnownVariablesDuplicatingEachOther(t *testing.T) {
 	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)
+			}
+
+		})
+	}
+}