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
This commit is contained in:
parent
eeb8a6474c
commit
36c69ee42b
|
@ -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 {
|
||||
|
|
|
@ -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("<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 := 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("<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)
|
||||
}
|
||||
|
||||
runPass(t, test.in, test.out, func(fixer *Fixer) error {
|
||||
return fixer.runPatchListMod(mergeMatchingModuleProperties)
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
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"],
|
||||
}
|
||||
`,
|
||||
},
|
||||
}
|
||||
|
||||
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)
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue