diff options
author | Dan Willemsen <dwillemsen@google.com> | 2018-05-08 15:53:42 -0700 |
---|---|---|
committer | Dan Willemsen <dwillemsen@google.com> | 2018-05-14 15:32:21 -0700 |
commit | 2b21a77ad9afa003689037f47c93706ee5865be9 (patch) | |
tree | 387885065cd3052bb8dca0ebb73b9d6c6f03a642 | |
parent | 36c69ee42bb0f27435db992f973f40322e3a31ad (diff) | |
download | build_soong-2b21a77ad9afa003689037f47c93706ee5865be9.tar.gz build_soong-2b21a77ad9afa003689037f47c93706ee5865be9.tar.bz2 build_soong-2b21a77ad9afa003689037f47c93706ee5865be9.zip |
bpfix: Convert local_include_dirs removal to PatchList
This way we can remove the line the property was on, not just the
property itself.
Test: `m blueprint_tools` to run the unit tests
Test: diff bpfix results on all of AOSP before/after this change
Change-Id: I61fdd945e6ee711c620b79683dfee7b7c751b3c4
-rw-r--r-- | bpfix/bpfix/bpfix.go | 50 | ||||
-rw-r--r-- | bpfix/bpfix/bpfix_test.go | 157 |
2 files changed, 185 insertions, 22 deletions
diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index 9a9c78b7..55de9936 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -147,7 +147,7 @@ func parse(name string, r io.Reader) (*parser.File, error) { func (f *Fixer) fixTreeOnce(config FixRequest) error { if config.simplifyKnownRedundantVariables { - err := f.simplifyKnownPropertiesDuplicatingEachOther() + err := f.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther) if err != nil { return err } @@ -183,9 +183,10 @@ func (f *Fixer) fixTreeOnce(config FixRequest) error { return nil } -func (f *Fixer) simplifyKnownPropertiesDuplicatingEachOther() error { +func simplifyKnownPropertiesDuplicatingEachOther(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { // remove from local_include_dirs anything in export_include_dirs - return f.removeMatchingModuleListProperties("export_include_dirs", "local_include_dirs") + return removeMatchingModuleListProperties(mod, patchList, + "export_include_dirs", "local_include_dirs") } func (f *Fixer) rewriteIncorrectAndroidmkPrebuilts() error { @@ -429,7 +430,7 @@ func mergeListProperties(a, b *parser.Property, buf []byte, patchlist *parser.Pa } // removes from <items> every item present in <removals> -func filterExpressionList(items *parser.List, removals *parser.List) { +func filterExpressionList(patchList *parser.PatchList, items *parser.List, removals *parser.List) { writeIndex := 0 for _, item := range items.Values { included := true @@ -446,32 +447,39 @@ func filterExpressionList(items *parser.List, removals *parser.List) { if included { items.Values[writeIndex] = item writeIndex++ + } else { + patchList.Add(item.Pos().Offset, item.End().Offset+2, "") } } items.Values = items.Values[:writeIndex] } // Remove each modules[i].Properties[<legacyName>][j] that matches a modules[i].Properties[<canonicalName>][k] -func (f *Fixer) removeMatchingModuleListProperties(canonicalName string, legacyName string) error { - for _, def := range f.tree.Defs { - mod, ok := def.(*parser.Module) - if !ok { - continue - } - legacyList, ok := getLiteralListProperty(mod, legacyName) - if !ok || len(legacyList.Values) == 0 { - continue - } - canonicalList, ok := getLiteralListProperty(mod, canonicalName) - if !ok { - continue - } - filterExpressionList(legacyList, canonicalList) +func removeMatchingModuleListProperties(mod *parser.Module, patchList *parser.PatchList, canonicalName string, legacyName string) error { + legacyProp, ok := mod.GetProperty(legacyName) + if !ok { + return nil + } + legacyList, ok := legacyProp.Value.(*parser.List) + if !ok || len(legacyList.Values) == 0 { + return nil + } + canonicalList, ok := getLiteralListProperty(mod, canonicalName) + if !ok { + return nil + } - if len(legacyList.Values) == 0 { - removeProperty(mod, legacyName) + localPatches := parser.PatchList{} + filterExpressionList(&localPatches, legacyList, canonicalList) + + if len(legacyList.Values) == 0 { + patchList.Add(legacyProp.Pos().Offset, legacyProp.End().Offset+2, "") + } else { + for _, p := range localPatches { + patchList.Add(p.Start, p.End, p.Replacement) } } + return nil } diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index 7aba39e6..654ccf93 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -66,7 +66,7 @@ func implFilterListTest(t *testing.T, local_include_dirs []string, export_includ fixer := NewFixer(tree) // apply simplifications - err := fixer.simplifyKnownPropertiesDuplicatingEachOther() + err := fixer.runPatchListMod(simplifyKnownPropertiesDuplicatingEachOther) if len(errs) > 0 { t.Fatal(err) } @@ -342,3 +342,158 @@ func TestReorderCommonProperties(t *testing.T) { }) } } + +func TestRemoveMatchingModuleListProperties(t *testing.T) { + var tests = []struct { + name string + in string + out string + }{ + { + name: "simple", + in: ` + cc_library { + name: "foo", + foo: ["a"], + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + bar: ["a"], + } + `, + }, + { + name: "long", + in: ` + cc_library { + name: "foo", + foo: [ + "a", + "b", + ], + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + foo: [ + "b", + ], + bar: ["a"], + } + `, + }, + { + name: "long fully removed", + in: ` + cc_library { + name: "foo", + foo: [ + "a", + ], + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + bar: ["a"], + } + `, + }, + { + name: "comment", + in: ` + cc_library { + name: "foo", + + // comment + foo: ["a"], + + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + + // comment + + bar: ["a"], + } + `, + }, + { + name: "inner comment", + in: ` + cc_library { + name: "foo", + foo: [ + // comment + "a", + ], + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + bar: ["a"], + } + `, + }, + { + name: "eol comment", + in: ` + cc_library { + name: "foo", + foo: ["a"], // comment + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + // comment + bar: ["a"], + } + `, + }, + { + name: "eol comment with blank lines", + in: ` + cc_library { + name: "foo", + + foo: ["a"], // comment + + // bar + bar: ["a"], + } + `, + out: ` + cc_library { + name: "foo", + + // comment + + // bar + bar: ["a"], + } + `, + }, + } + 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(func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { + return removeMatchingModuleListProperties(mod, patchList, "bar", "foo") + }) + }) + }) + } +} |