From 5dadd0e08aeda0d8a4d55b66e08f31a3828d967d Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 22 Mar 2018 12:43:16 -0700 Subject: 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. Bug: 73724997 Test: bpfix_test.go Change-Id: I30955b6efbb767c02ba77f2f18d44951ef094bad Merged-In: I30955b6efbb767c02ba77f2f18d44951ef094bad (cherry picked from commit 9c55d237f6b72896209344aee18a1702c2f9ac3e) --- androidmk/cmd/androidmk/androidmk_test.go | 26 ++---- bpfix/bpfix/bpfix.go | 133 ++++++++++++++++++++++++++++++ bpfix/bpfix/bpfix_test.go | 128 ++++++++++++++++++++++++++++ 3 files changed, 266 insertions(+), 21 deletions(-) diff --git a/androidmk/cmd/androidmk/androidmk_test.go b/androidmk/cmd/androidmk/androidmk_test.go index 45df1a57..dd646efe 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 @@ include $(call all-makefiles-under,$(LOCAL_PATH)) }, } -func reformatBlueprint(input string) string { - file, errs := bpparser.Parse("", 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("", i), bytes.NewBufferString(test.in)) if len(errs) > 0 { diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index 4300e81b..4ab42a4e 100644 --- a/bpfix/bpfix/bpfix.go +++ b/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("", 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 every item present in 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 1e1e6933..51708eb6 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 @@ 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("", 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) + } + + }) + } +} -- cgit v1.2.3