diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index d4384b274..adb0a7bb9 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -130,7 +130,11 @@ var fixSteps = []FixStep{ }, { Name: "removePdkProperty", - Fix: runPatchListMod(removePdkProperty), + Fix: runPatchListMod(removeObsoleteProperty("product_variables.pdk")), + }, + { + Name: "removeScudoProperty", + Fix: runPatchListMod(removeObsoleteProperty("sanitize.scudo")), }, } @@ -863,7 +867,9 @@ func convertToSingleSource(mod *parser.Module, srcPropertyName string) { } } -func runPatchListMod(modFunc func(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error) func(*Fixer) error { +type patchListModFunction func(*parser.Module, []byte, *parser.PatchList) error + +func runPatchListMod(modFunc patchListModFunction) func(*Fixer) error { return func(f *Fixer) error { // Make sure all the offsets are accurate buf, err := f.reparse() @@ -1033,23 +1039,63 @@ func removeTags(mod *parser.Module, buf []byte, patchlist *parser.PatchList) err return patchlist.Add(prop.Pos().Offset, prop.End().Offset+2, replaceStr) } -func removePdkProperty(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error { - prop, ok := mod.GetProperty("product_variables") - if !ok { - return nil +type propertyProvider interface { + GetProperty(string) (*parser.Property, bool) + RemoveProperty(string) bool +} + +func removeNestedProperty(mod *parser.Module, patchList *parser.PatchList, propName string) error { + propNames := strings.Split(propName, ".") + + var propProvider, toRemoveFrom propertyProvider + propProvider = mod + + var propToRemove *parser.Property + for i, name := range propNames { + p, ok := propProvider.GetProperty(name) + if !ok { + return nil + } + // if this is the inner most element, it's time to delete + if i == len(propNames)-1 { + if propToRemove == nil { + // if we cannot remove the properties that the current property is nested in, + // remove only the current property + propToRemove = p + toRemoveFrom = propProvider + } + + // remove the property from the list, in case we remove other properties in this list + toRemoveFrom.RemoveProperty(propToRemove.Name) + // only removing the property would leave blank line(s), remove with a patch + if err := patchList.Add(propToRemove.Pos().Offset, propToRemove.End().Offset+2, ""); err != nil { + return err + } + } else { + propMap, ok := p.Value.(*parser.Map) + if !ok { + return nil + } + if len(propMap.Properties) > 1 { + // if there are other properties in this struct, we need to keep this struct + toRemoveFrom = nil + propToRemove = nil + } else if propToRemove == nil { + // otherwise, we can remove the empty struct entirely + toRemoveFrom = propProvider + propToRemove = p + } + propProvider = propMap + } } - propMap, ok := prop.Value.(*parser.Map) - if !ok { - return nil + + return nil +} + +func removeObsoleteProperty(propName string) patchListModFunction { + return func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error { + return removeNestedProperty(mod, patchList, propName) } - pdkProp, ok := propMap.GetProperty("pdk") - if !ok { - return nil - } - if len(propMap.Properties) > 1 { - return patchlist.Add(pdkProp.Pos().Offset, pdkProp.End().Offset+2, "") - } - return patchlist.Add(prop.Pos().Offset, prop.End().Offset+2, "") } func mergeMatchingModuleProperties(mod *parser.Module, buf []byte, patchlist *parser.PatchList) error { diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index f502229e2..b994e2547 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -999,7 +999,171 @@ func TestRemoveSoongConfigBoolVariable(t *testing.T) { } } -func TestRemovePdkProperty(t *testing.T) { +func TestRemoveNestedProperty(t *testing.T) { + tests := []struct { + name string + in string + out string + propertyName string + }{ + { + name: "remove no nesting", + in: ` +cc_library { + name: "foo", + foo: true, +}`, + out: ` +cc_library { + name: "foo", +} +`, + propertyName: "foo", + }, + { + name: "remove one nest", + in: ` +cc_library { + name: "foo", + foo: { + bar: true, + }, +}`, + out: ` +cc_library { + name: "foo", +} +`, + propertyName: "foo.bar", + }, + { + name: "remove one nest, multiple props", + in: ` +cc_library { + name: "foo", + foo: { + bar: true, + baz: false, + }, +}`, + out: ` +cc_library { + name: "foo", + foo: { + baz: false, + }, +} +`, + propertyName: "foo.bar", + }, + { + name: "remove multiple nest", + in: ` +cc_library { + name: "foo", + foo: { + bar: { + baz: { + a: true, + } + }, + }, +}`, + out: ` +cc_library { + name: "foo", +} +`, + propertyName: "foo.bar.baz.a", + }, + { + name: "remove multiple nest, outer non-empty", + in: ` +cc_library { + name: "foo", + foo: { + bar: { + baz: { + a: true, + } + }, + other: true, + }, +}`, + out: ` +cc_library { + name: "foo", + foo: { + other: true, + }, +} +`, + propertyName: "foo.bar.baz.a", + }, + { + name: "remove multiple nest, inner non-empty", + in: ` +cc_library { + name: "foo", + foo: { + bar: { + baz: { + a: true, + }, + other: true, + }, + }, +}`, + out: ` +cc_library { + name: "foo", + foo: { + bar: { + other: true, + }, + }, +} +`, + propertyName: "foo.bar.baz.a", + }, + { + name: "remove multiple nest, inner-most non-empty", + in: ` +cc_library { + name: "foo", + foo: { + bar: { + baz: { + a: true, + other: true, + }, + }, + }, +}`, + out: ` +cc_library { + name: "foo", + foo: { + bar: { + baz: { + other: true, + }, + }, + }, +} +`, + propertyName: "foo.bar.baz.a", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + runPass(t, test.in, test.out, runPatchListMod(removeObsoleteProperty(test.propertyName))) + }) + } +} + +func TestRemoveObsoleteProperties(t *testing.T) { tests := []struct { name string in string @@ -1052,7 +1216,7 @@ func TestRemovePdkProperty(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - runPass(t, test.in, test.out, runPatchListMod(removePdkProperty)) + runPass(t, test.in, test.out, runPatchListMod(removeObsoleteProperty("product_variables.pdk"))) }) } }