From 36c69ee42bb0f27435db992f973f40322e3a31ad Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Tue, 8 May 2018 11:18:15 -0700 Subject: bpfix: Add reorderCommonProperties This will move the common properties: name defaults device_supported host_supported To be listed first (and in that order) for every module. Other properties are untouched. This also adds a helper to test individual passes in an end-to-end manner, and a helper to run passes that use PatchLists. Test: `m blueprint_tools` to add the new tests Test: Diff bpfix results over all of aosp before/after this change Change-Id: I746a00a3731cb7597d2613ef2dc45a99654cd122 --- bpfix/bpfix/bpfix.go | 87 ++++++++++++++++++++++-- bpfix/bpfix/bpfix_test.go | 167 +++++++++++++++++++++++++++++++++++----------- 2 files changed, 212 insertions(+), 42 deletions(-) (limited to 'bpfix') diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index a610a178..9a9c78b7 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -49,6 +49,7 @@ type FixRequest struct { rewriteIncorrectAndroidmkPrebuilts bool rewriteIncorrectAndroidmkAndroidLibraries bool mergeMatchingModuleProperties bool + reorderCommonProperties bool } func NewFixRequest() FixRequest { @@ -61,6 +62,7 @@ func (r FixRequest) AddAll() (result FixRequest) { result.rewriteIncorrectAndroidmkPrebuilts = true result.rewriteIncorrectAndroidmkAndroidLibraries = true result.mergeMatchingModuleProperties = true + result.reorderCommonProperties = true return result } @@ -150,6 +152,7 @@ func (f *Fixer) fixTreeOnce(config FixRequest) error { return err } } + if config.rewriteIncorrectAndroidmkPrebuilts { err := f.rewriteIncorrectAndroidmkPrebuilts() if err != nil { @@ -165,7 +168,14 @@ func (f *Fixer) fixTreeOnce(config FixRequest) error { } if config.mergeMatchingModuleProperties { - err := f.mergeMatchingModuleProperties() + err := f.runPatchListMod(mergeMatchingModuleProperties) + if err != nil { + return err + } + } + + if config.reorderCommonProperties { + err := f.runPatchListMod(reorderCommonProperties) if err != nil { return err } @@ -249,7 +259,7 @@ func (f *Fixer) rewriteIncorrectAndroidmkAndroidLibraries() error { return nil } -func (f *Fixer) mergeMatchingModuleProperties() error { +func (f *Fixer) runPatchListMod(modFunc func(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error) error { // Make sure all the offsets are accurate buf, err := f.reparse() if err != nil { @@ -263,7 +273,7 @@ func (f *Fixer) mergeMatchingModuleProperties() error { continue } - err := mergeMatchingProperties(&mod.Properties, buf, &patchlist) + err := modFunc(mod, buf, &patchlist) if err != nil { return err } @@ -275,9 +285,12 @@ func (f *Fixer) mergeMatchingModuleProperties() error { return err } + // Save a copy of the buffer to print for errors below + bufCopy := append([]byte(nil), newBuf.Bytes()...) + newTree, err := parse(f.tree.Name, newBuf) if err != nil { - return err + return fmt.Errorf("Failed to parse: %v\nBuffer:\n%s", err, string(bufCopy)) } f.tree = newTree @@ -285,6 +298,63 @@ func (f *Fixer) mergeMatchingModuleProperties() error { return nil } +var commonPropertyPriorities = []string{ + "name", + "defaults", + "device_supported", + "host_supported", +} + +func reorderCommonProperties(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error { + if len(mod.Properties) == 0 { + return nil + } + + pos := mod.LBracePos.Offset + 1 + stage := "" + + for _, name := range commonPropertyPriorities { + idx := propertyIndex(mod.Properties, name) + if idx == -1 { + continue + } + if idx == 0 { + err := patchlist.Add(pos, pos, stage) + if err != nil { + return err + } + stage = "" + + pos = mod.Properties[0].End().Offset + 1 + mod.Properties = mod.Properties[1:] + continue + } + + prop := mod.Properties[idx] + mod.Properties = append(mod.Properties[:idx], mod.Properties[idx+1:]...) + + stage += string(buf[prop.Pos().Offset : prop.End().Offset+1]) + + err := patchlist.Add(prop.Pos().Offset, prop.End().Offset+2, "") + if err != nil { + return err + } + } + + if stage != "" { + err := patchlist.Add(pos, pos, stage) + if err != nil { + return err + } + } + + return nil +} + +func mergeMatchingModuleProperties(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error { + return mergeMatchingProperties(&mod.Properties, buf, patchlist) +} + func mergeMatchingProperties(properties *[]*parser.Property, buf []byte, patchlist *parser.PatchList) error { seen := make(map[string]*parser.Property) for i := 0; i < len(*properties); i++ { @@ -419,6 +489,15 @@ func getLiteralListProperty(mod *parser.Module, name string) (list *parser.List, return list, ok } +func propertyIndex(props []*parser.Property, propertyName string) int { + for i, prop := range props { + if prop.Name == propertyName { + return i + } + } + return -1 +} + func renameProperty(mod *parser.Module, from, to string) { for _, prop := range mod.Properties { if prop.Name == from { diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index 8fed0a2d..7aba39e6 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -125,6 +125,49 @@ func TestSimplifyKnownVariablesDuplicatingEachOther(t *testing.T) { implFilterListTest(t, []string{}, []string{}, []string{}) } +func runPass(t *testing.T, in, out string, innerTest func(*Fixer) error) { + expected, err := Reformat(out) + if err != nil { + t.Fatal(err) + } + + in, err = Reformat(in) + if err != nil { + t.Fatal(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 := innerTest(fixer) + 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("output didn't match:\ninput:\n%s\n\nexpected:\n%s\ngot:\n%s\n", + in, expected, got) + } +} + func TestMergeMatchingProperties(t *testing.T) { tests := []struct { name string @@ -207,47 +250,95 @@ func TestMergeMatchingProperties(t *testing.T) { 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) - } + runPass(t, test.in, test.out, func(fixer *Fixer) error { + return fixer.runPatchListMod(mergeMatchingModuleProperties) + }) + }) + } +} - out, err := parser.Print(fixer.tree) - if err != nil { - t.Fatal(err) +func TestReorderCommonProperties(t *testing.T) { + var tests = []struct { + name string + in string + out string + }{ + { + name: "empty", + in: `cc_library {}`, + out: `cc_library {}`, + }, + { + name: "only priority", + in: ` + cc_library { + name: "foo", } + `, + out: ` + cc_library { + name: "foo", + } + `, + }, + { + name: "already in order", + in: ` + cc_library { + name: "foo", + defaults: ["bar"], + } + `, + out: ` + cc_library { + name: "foo", + defaults: ["bar"], + } + `, + }, + { + name: "reorder only priority", + in: ` + cc_library { + defaults: ["bar"], + name: "foo", + } + `, + out: ` + cc_library { + name: "foo", + defaults: ["bar"], + } + `, + }, + { + name: "reorder", + in: ` + cc_library { + name: "foo", + srcs: ["a.c"], + host_supported: true, + defaults: ["bar"], + shared_libs: ["baz"], + } + `, + out: ` + cc_library { + name: "foo", + defaults: ["bar"], + host_supported: true, + srcs: ["a.c"], + shared_libs: ["baz"], + } + `, + }, + } - 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) - } - + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runPass(t, test.in, test.out, func(fixer *Fixer) error { + return fixer.runPatchListMod(reorderCommonProperties) + }) }) } } -- cgit v1.2.3