From 86dfa3326a72db042b6ca3c6e3521070b3dd0b75 Mon Sep 17 00:00:00 2001 From: Tyler Sellmayer Date: Fri, 31 May 2024 19:48:48 +0000 Subject: [PATCH 1/2] Add dict_replace_if_equal command to buildozer. --- buildozer/README.md | 4 ++ edit/buildozer.go | 88 ++++++++++++++++++++----------- edit/buildozer_test.go | 116 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+), 29 deletions(-) diff --git a/buildozer/README.md b/buildozer/README.md index c4d20f973..48bd8c4b3 100644 --- a/buildozer/README.md +++ b/buildozer/README.md @@ -147,6 +147,10 @@ Buildozer supports the following commands(`'command args'`): * `dict_set <(key:value)(s)>`: Sets the value of a key for the dict attribute `attr`. If the key was already present, its old value is replaced. * `dict_remove `: Deletes the key for the dict attribute `attr`. + * `dict_replace_if_equal `: Replaces + `old_value` with `new_value` for key `key` in dictionary attribute `attr`. + If the key is not present in the dictionary, or does not have value + `old_value`, it will _not_ be updated. * `dict_list_add `: Adds value(s) to the list in the dict attribute `attr`. * `format`: Force formatting of all files, even if they were not changed by diff --git a/edit/buildozer.go b/edit/buildozer.go index 6bc0a829c..3ad344ffd 100644 --- a/edit/buildozer.go +++ b/edit/buildozer.go @@ -745,6 +745,35 @@ func cmdDictRemove(opts *Options, env CmdEnvironment) (*build.File, error) { return env.File, nil } +// cmdDictReplaceIfEqual updates a value in a dict if it is equal to a given value. +func cmdDictReplaceIfEqual(opts *Options, env CmdEnvironment) (*build.File, error) { + attr := env.Args[0] + key := env.Args[1] + oldV := getStringValue(env.Args[2]) + newV := getStringValue(env.Args[3]) + + thing := env.Rule.Attr(attr) + dictAttr, ok := thing.(*build.DictExpr) + if !ok { + return env.File, nil + } + + prev := DictionaryGet(dictAttr, key) + if prev == nil { + return nil, fmt.Errorf("key '%s' not found in dict", key) + } + if e, ok := prev.(*build.StringExpr); ok { + if labels.Equal(e.Value, oldV, env.Pkg) || e.Value == oldV { + DictionarySet(dictAttr, key, getStringExpr(newV, env.Pkg)) + } + } else if e, ok := prev.(*build.Ident); ok { + if e.Name == oldV { + DictionarySet(dictAttr, key, getStringExpr(newV, env.Pkg)) + } + } + return env.File, nil +} + // cmdDictListAdd adds an item to a list in a dict. func cmdDictListAdd(opts *Options, env CmdEnvironment) (*build.File, error) { attr := env.Args[0] @@ -880,35 +909,36 @@ type CommandInfo struct { // AllCommands associates the command names with their function and number // of arguments. var AllCommands = map[string]CommandInfo{ - "add": {cmdAdd, true, 2, -1, " "}, - "new_load": {cmdNewLoad, false, 1, -1, " <[to=]from(s)>"}, - "replace_load": {cmdReplaceLoad, false, 1, -1, " <[to=]symbol(s)>"}, - "substitute_load": {cmdSubstituteLoad, false, 2, 2, " "}, - "comment": {cmdComment, true, 1, 3, "? ? "}, - "print_comment": {cmdPrintComment, true, 0, 2, "? ?"}, - "delete": {cmdDelete, true, 0, 0, ""}, - "fix": {cmdFix, true, 0, -1, "?"}, - "move": {cmdMove, true, 3, -1, " "}, - "new": {cmdNew, false, 2, 4, " [(before|after) ]"}, - "print": {cmdPrint, true, 0, -1, ""}, - "remove": {cmdRemove, true, 1, -1, " "}, - "remove_comment": {cmdRemoveComment, true, 0, 2, "? ?"}, - "remove_if_equal": {cmdRemoveIfEqual, true, 2, 2, " "}, - "rename": {cmdRename, true, 2, 2, " "}, - "replace": {cmdReplace, true, 3, 3, " "}, - "substitute": {cmdSubstitute, true, 3, 3, " "}, - "set": {cmdSet, true, 1, -1, " "}, - "set_if_absent": {cmdSetIfAbsent, true, 1, -1, " "}, - "set_select": {cmdSetSelect, true, 1, -1, " "}, - "copy": {cmdCopy, true, 2, 2, " "}, - "copy_no_overwrite": {cmdCopyNoOverwrite, true, 2, 2, " "}, - "dict_add": {cmdDictAdd, true, 2, -1, " <(key:value)(s)>"}, - "dict_set": {cmdDictSet, true, 2, -1, " <(key:value)(s)>"}, - "dict_remove": {cmdDictRemove, true, 2, -1, " "}, - "dict_list_add": {cmdDictListAdd, true, 3, -1, " "}, - "use_repo_add": {cmdUseRepoAdd, false, 2, -1, "([dev] |) "}, - "use_repo_remove": {cmdUseRepoRemove, false, 2, -1, "([dev] |) "}, - "format": {cmdFormat, false, 0, 0, ""}, + "add": {cmdAdd, true, 2, -1, " "}, + "new_load": {cmdNewLoad, false, 1, -1, " <[to=]from(s)>"}, + "replace_load": {cmdReplaceLoad, false, 1, -1, " <[to=]symbol(s)>"}, + "substitute_load": {cmdSubstituteLoad, false, 2, 2, " "}, + "comment": {cmdComment, true, 1, 3, "? ? "}, + "print_comment": {cmdPrintComment, true, 0, 2, "? ?"}, + "delete": {cmdDelete, true, 0, 0, ""}, + "fix": {cmdFix, true, 0, -1, "?"}, + "move": {cmdMove, true, 3, -1, " "}, + "new": {cmdNew, false, 2, 4, " [(before|after) ]"}, + "print": {cmdPrint, true, 0, -1, ""}, + "remove": {cmdRemove, true, 1, -1, " "}, + "remove_comment": {cmdRemoveComment, true, 0, 2, "? ?"}, + "remove_if_equal": {cmdRemoveIfEqual, true, 2, 2, " "}, + "rename": {cmdRename, true, 2, 2, " "}, + "replace": {cmdReplace, true, 3, 3, " "}, + "substitute": {cmdSubstitute, true, 3, 3, " "}, + "set": {cmdSet, true, 1, -1, " "}, + "set_if_absent": {cmdSetIfAbsent, true, 1, -1, " "}, + "set_select": {cmdSetSelect, true, 1, -1, " "}, + "copy": {cmdCopy, true, 2, 2, " "}, + "copy_no_overwrite": {cmdCopyNoOverwrite, true, 2, 2, " "}, + "dict_add": {cmdDictAdd, true, 2, -1, " <(key:value)(s)>"}, + "dict_set": {cmdDictSet, true, 2, -1, " <(key:value)(s)>"}, + "dict_remove": {cmdDictRemove, true, 2, -1, " "}, + "dict_replace_if_equal": {cmdDictReplaceIfEqual, true, 4, 4, " "}, + "dict_list_add": {cmdDictListAdd, true, 3, -1, " "}, + "use_repo_add": {cmdUseRepoAdd, false, 2, -1, "([dev] |) "}, + "use_repo_remove": {cmdUseRepoRemove, false, 2, -1, "([dev] |) "}, + "format": {cmdFormat, false, 0, 0, ""}, } var readonlyCommands = map[string]bool{ diff --git a/edit/buildozer_test.go b/edit/buildozer_test.go index 5e9d37995..19aab18bc 100644 --- a/edit/buildozer_test.go +++ b/edit/buildozer_test.go @@ -351,6 +351,122 @@ func TestCmdDictListAdd(t *testing.T) { } } +var dictReplaceIfEqualTests = []struct { + args []string + buildFile string + expected string +}{ + {[]string{ + "attr", "key1", "value1", "value2", + }, + `foo( + name = "foo", + attr = {"key1": "value1"}, + )`, + `foo( + name = "foo", + attr = {"key1": "value2"}, +)`, + }, + {[]string{ + "attr", "key1", "value1", "value2", + }, + `foo( + name = "foo", + attr = {"key1": "x"}, + )`, + `foo( + name = "foo", + attr = {"key1": "x"}, +)`, + }, + {[]string{ + "attr", "key1", "value1", "value2", + }, + `foo( + name = "foo", + attr = { + "key1": ["value1"], + "key2": ["value2"], + }, + )`, + `foo( + name = "foo", + attr = { + "key1": ["value1"], + "key2": ["value2"], + }, +)`, + }, + {[]string{ + "attr", "key1", "value1", "value2", + }, + `foo( + name = "foo", + attr = { + "key1": "value1", + "key2": "value2", + }, + )`, + `foo( + name = "foo", + attr = { + "key1": "value2", + "key2": "value2", +}, +)`, + }, + {[]string{ + "attr", "key1", "value1", "value2", + }, + `foo( + name = "foo", + attr = { + "key1": "value1", + "key2": "x", + }, + )`, + `foo( + name = "foo", + attr = { + "key1": "value2", + "key2": "x", + }, +)`, + }, +} + +func TestCmdDictReplaceIfEqual(t *testing.T) { + for i, tt := range dictReplaceIfEqualTests { + bld, err := build.Parse("BUILD", []byte(tt.buildFile)) + if err != nil { + t.Error(err) + continue + } + expectedBld, err := build.Parse("BUILD", []byte(tt.expected)) + if err != nil { + t.Error(err) + continue + } + rl := bld.Rules("foo")[0] + env := CmdEnvironment{ + File: bld, + Rule: rl, + Args: tt.args, + } + bld, err = cmdDictReplaceIfEqual(NewOpts(), env) + if err != nil { + t.Errorf("cmdDictReplaceIfEqual(%d):\ngot error:\n%s", i, err) + } + got := strings.TrimSpace(string(build.Format(bld))) + expected := strings.TrimSpace(string(build.Format(expectedBld))) + if got != expected { + t.Errorf("cmdDictReplaceIfEqual(%d):\ngot:\n%s\nexpected:\n%s", i, got, tt.expected) + } + } +} + + var substituteLoadsTests = []struct { args []string buildFile string From 89c0576c0845f26a82f259b0b7a932eb17c0bb6d Mon Sep 17 00:00:00 2001 From: Tyler Sellmayer Date: Thu, 6 Jun 2024 18:33:16 +0000 Subject: [PATCH 2/2] Run gofmt -w and remove unnecessary equality check. --- edit/buildozer.go | 2 +- edit/buildozer_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/edit/buildozer.go b/edit/buildozer.go index 3ad344ffd..aa7557569 100644 --- a/edit/buildozer.go +++ b/edit/buildozer.go @@ -763,7 +763,7 @@ func cmdDictReplaceIfEqual(opts *Options, env CmdEnvironment) (*build.File, erro return nil, fmt.Errorf("key '%s' not found in dict", key) } if e, ok := prev.(*build.StringExpr); ok { - if labels.Equal(e.Value, oldV, env.Pkg) || e.Value == oldV { + if labels.Equal(e.Value, oldV, env.Pkg) { DictionarySet(dictAttr, key, getStringExpr(newV, env.Pkg)) } } else if e, ok := prev.(*build.Ident); ok { diff --git a/edit/buildozer_test.go b/edit/buildozer_test.go index 19aab18bc..f883d2a8c 100644 --- a/edit/buildozer_test.go +++ b/edit/buildozer_test.go @@ -466,7 +466,6 @@ func TestCmdDictReplaceIfEqual(t *testing.T) { } } - var substituteLoadsTests = []struct { args []string buildFile string