Allow callers to explicitly provide types for set and add commands (#1448)
diff --git a/buildozer/README.md b/buildozer/README.md
index d907ff9..82a9394 100644
--- a/buildozer/README.md
+++ b/buildozer/README.md
@@ -86,8 +86,10 @@
Buildozer supports the following commands(`'command args'`):
- * `add <attr> <value(s)>`: Adds value(s) to a list attribute of a rule. If a
- value is already present in the list, it is not added.
+ * `add <attr>[:<type>] <value(s)>`: Adds value(s) to a list attribute of a rule.
+ If a value is already present in the list, it is not added. `type` specifies
+ the type of values being added. See [supported types](#supported-types) for
+ the available types.
* `new_load <path> <[to=]from(s)>`: Add a load statement for the given path,
importing the symbols. Afterwards, consider running `buildozer 'fix unusedLoads'`.
* `replace_load <path> <[to=]from(s)>`: Similar to `new_load`, but removes
@@ -130,10 +132,14 @@
be a simple replacement string, but it may also expand numbered or named
groups using `$0` or `$x`. Lists without strings that match `old_regexp`
are not modified.
- * `set <attr> <value(s)>`: Sets the value of an attribute. If the attribute
- was already present, its old value is replaced.
- * `set_if_absent <attr> <value(s)>`: Sets the value of an attribute. If the
- attribute was already present, no action is taken.
+ * `set <attr>[:<type>] <value(s)>`: Sets the value of an attribute. If the
+ attribute was already present, its old value is replaced. `type` specifies
+ the type of values being added. See
+ [supported types](#supported-types) for the available types.
+ * `set_if_absent <attr>[:<type>] <value(s)>`: Sets the value of an attribute.
+ If the attribute was already present, no action is taken. `type` specifies
+ the type of values being added. See [supported types](#supported-types) for
+ * the available types.
* `set kind <value>`: Set the target type to value.
* `set_select <attr> <key_1> <value_1> <key_n> <value_n>`
* `copy <attr> <from_rule>`: Copies the value of `attr` between rules. If it
@@ -188,6 +194,15 @@
usages with `dev_dependency = True` will be considered instead. Extension
usages with `isolated = True` are ignored.
+#### Supported types
+
+For the commands that support explicitly providing argument types, the following
+types are available:
+
+ * `expr`: arbitrary expression, won't be quoted as a string unless already
+ quoted. Buildozer doesn't check that the expression is syntactically
+ correct, however the subsequent formatting step will fail if it's not.
+
#### Examples
```bash
@@ -247,6 +262,9 @@
# Add an attribute new_attr with value "def_val" to all cc_binary rules
# Note that special characters will automatically be escaped in the string
buildozer 'add new_attr def_val' //:%cc_binary
+
+# Specifying an argument type to prevent automatic quoting
+buildozer 'add tags:expr FOO' //pkg:rule # adds tags = [FOO]
```
### Print commands
diff --git a/buildozer/buildozer_test.sh b/buildozer/buildozer_test.sh
index 955c582..8ed014b 100755
--- a/buildozer/buildozer_test.sh
+++ b/buildozer/buildozer_test.sh
@@ -224,6 +224,34 @@
)'
}
+function test_add_override_expr() {
+ in='go_library(
+ name = "edit",
+ deps = ["build"],
+)'
+ run "$in" 'add deps:expr build' '//pkg:edit'
+ assert_equals 'go_library(
+ name = "edit",
+ deps = [
+ build,
+ "build",
+ ],
+)'
+}
+
+function test_add_override_unknown_type() {
+ in='go_library(
+ name = "edit",
+ deps = ["build"],
+)'
+ ERROR=2 run "$in" 'add deps:foo build' '//pkg:edit'
+ assert_equals 'go_library(
+ name = "edit",
+ deps = ["build"],
+)'
+}
+
+
function test_remove_last_dep() {
run "$one_dep" 'remove deps //buildifier:build' '//pkg:edit'
assert_equals 'go_library(name = "edit")'
@@ -1085,6 +1113,16 @@
)'
}
+function test_set_if_absent_override_expr() {
+ in='soy_js(name = "a")'
+
+ run "$in" 'set_if_absent copts:expr foo()' '//pkg:a'
+ assert_equals 'soy_js(
+ name = "a",
+ copts = foo(),
+)'
+}
+
function test_set_custom_code() {
in='cc_test(name = "a")'
@@ -1098,6 +1136,16 @@
)'
}
+function test_set_override_expr() {
+ in='cc_library(name = "a")'
+
+ run "$in" 'set copts:expr foo()' '//pkg:a'
+ assert_equals 'cc_library(
+ name = "a",
+ copts = foo(),
+)'
+}
+
function assert_output() {
echo "$1" > "expected"
diff -u "expected" "$log" || fail "Output didn't match"
diff --git a/edit/buildozer.go b/edit/buildozer.go
index d246bfa..e896df4 100644
--- a/edit/buildozer.go
+++ b/edit/buildozer.go
@@ -83,18 +83,52 @@
output *apipb.Output_Record // output proto, stores whatever a command wants to print
}
+// AttrType is an enum representing a type of the attribute, e.g. expr or not
+// provided type
+type AttrType int
+
+const (
+ // notProvidedTypeAttr represents an attr type that's not provided explicitly and
+ // needs to be inferred by buildozer from bazel's build language proto
+ notProvidedTypeAttr = iota
+ // rawAttr is a type for idents, ints or other types that buildozer should
+ // take as is, without quoting or wrapping
+ rawAttr
+)
+
+// parseAttr parses the attr name and optional type, e.g. "foo" or "bar:expr"
+// The only supported type now is `expr`
+func parseAttr(attrName string) (attr string, attrType AttrType, err error) {
+ chunks := strings.SplitN(attrName, ":", 2)
+ if len(chunks) == 1 {
+ return attrName, notProvidedTypeAttr, nil
+ }
+ attr, typeName := chunks[0], chunks[1]
+ switch typeName {
+ case "expr":
+ return attr, rawAttr, nil
+ default:
+ return attr, notProvidedTypeAttr, fmt.Errorf("unknown attr type: %q", typeName)
+ }
+}
+
// The cmdXXX functions implement the various commands.
func cmdAdd(opts *Options, env CmdEnvironment) (*build.File, error) {
- attr := env.Args[0]
+ attr, attrType, err := parseAttr(env.Args[0])
+ if err != nil {
+ return nil, err
+ }
for _, val := range env.Args[1:] {
- if IsIntList(attr) {
+ if attrType == rawAttr || (attrType == notProvidedTypeAttr && IsIntList(attr)) {
AddValueToListAttribute(env.Rule, attr, env.Pkg, &build.LiteralExpr{Token: val}, &env.Vars)
continue
}
- strVal := getStringExpr(val, env.Pkg)
+ var strVal build.Expr
+ strVal = getLabelStringExpr(val, env.Pkg)
AddValueToListAttribute(env.Rule, attr, env.Pkg, strVal, &env.Vars)
}
+
ResolveAttr(env.Rule, attr, env.Pkg)
return env.File, nil
}
@@ -528,7 +562,7 @@
attr := env.Rule.Attr(key)
if e, ok := attr.(*build.StringExpr); ok {
if labels.Equal(e.Value, oldV, env.Pkg) {
- env.Rule.SetAttr(key, getAttrValueExpr(key, []string{newV}, env))
+ env.Rule.SetAttr(key, getAttrValueExpr(key, notProvidedTypeAttr, []string{newV}, env))
}
} else {
ListReplace(attr, oldV, newV, env.Pkg)
@@ -551,56 +585,71 @@
continue
}
if newValue, ok := stringSubstitute(e.Value, oldRegexp, newTemplate); ok {
- env.Rule.SetAttr(key, getStringExpr(newValue, env.Pkg))
+ env.Rule.SetAttr(key, getLabelStringExpr(newValue, env.Pkg))
}
}
return env.File, nil
}
func cmdSet(opts *Options, env CmdEnvironment) (*build.File, error) {
- attr := env.Args[0]
+ attr, attrType, err := parseAttr(env.Args[0])
+ if err != nil {
+ return nil, err
+ }
args := env.Args[1:]
if attr == "kind" {
env.Rule.SetKind(args[0])
} else {
- env.Rule.SetAttr(attr, getAttrValueExpr(attr, args, env))
+ env.Rule.SetAttr(attr, getAttrValueExpr(attr, attrType, args, env))
}
return env.File, nil
}
func cmdSetIfAbsent(opts *Options, env CmdEnvironment) (*build.File, error) {
- attr := env.Args[0]
+ attr, attrType, err := parseAttr(env.Args[0])
+ if err != nil {
+ return nil, err
+ }
args := env.Args[1:]
if attr == "kind" {
return nil, fmt.Errorf("setting 'kind' is not allowed for set_if_absent. Got %s", env.Args)
}
if env.Rule.Attr(attr) == nil {
- env.Rule.SetAttr(attr, getAttrValueExpr(attr, args, env))
+ env.Rule.SetAttr(attr, getAttrValueExpr(attr, attrType, args, env))
}
return env.File, nil
}
-func getAttrValueExpr(attr string, args []string, env CmdEnvironment) build.Expr {
+func getAttrValueExpr(attr string, attrType AttrType, args []string, env CmdEnvironment) build.Expr {
switch {
case attr == "kind":
return nil
+ case attrType == rawAttr:
+ // treat as an ident (it could be an arbitrary expression though, e.g. an
+ // already quoted string or a function call, will be re-parsed and
+ // formatted properly by the subsequent call of buildifier on the
+ //resulting file)
+ return &build.Ident{Name: args[0]}
case IsIntList(attr):
+ // list of raw objects (e.g. ints or idents)
var list []build.Expr
for _, i := range args {
list = append(list, &build.LiteralExpr{Token: i})
}
return &build.ListExpr{List: list}
case IsList(attr) && !(len(args) == 1 && strings.HasPrefix(args[0], "glob(")):
+ // list of labels
var list []build.Expr
for _, arg := range args {
- list = append(list, getStringExpr(arg, env.Pkg))
+ list = append(list, getLabelStringExpr(arg, env.Pkg))
}
return &build.ListExpr{List: list}
case len(args) == 0:
// Expected a non-list argument, nothing provided
return &build.Ident{Name: "None"}
case IsString(attr):
- return getStringExpr(args[0], env.Pkg)
+ // single label
+ return getLabelStringExpr(args[0], env.Pkg)
default:
return &build.Ident{Name: args[0]}
}
@@ -614,15 +663,24 @@
return value
}
-// getStringExpr creates a StringExpr from an input argument, which can be either quoted or not,
+// getStringExprLabel creates a StringExpr from an input argument, which can be either quoted or not,
// and shortens the label value if possible.
-func getStringExpr(value, pkg string) build.Expr {
+func getLabelStringExpr(value, pkg string) build.Expr {
if unquoted, triple, err := build.Unquote(value); err == nil {
return &build.StringExpr{Value: ShortenLabel(unquoted, pkg), TripleQuote: triple}
}
return &build.StringExpr{Value: ShortenLabel(value, pkg)}
}
+// getListExpr creates a ListExpr from a list of input arguments
+func getListExpr(exprs ...string) *build.ListExpr {
+ listVal := &build.ListExpr{}
+ for _, expr := range exprs {
+ listVal.List = append(listVal.List, &build.Ident{Name: expr})
+ }
+ return listVal
+}
+
func cmdCopy(opts *Options, env CmdEnvironment) (*build.File, error) {
attrName := env.Args[0]
from := env.Args[1]
@@ -658,7 +716,7 @@
return nil, fmt.Errorf("no colon in dict_add argument %q found", x)
}
kv = removeEscapes(kv, ':')
- expr := getStringExpr(kv[1], env.Pkg)
+ expr := getLabelStringExpr(kv[1], env.Pkg)
prev := DictionaryGet(dict, kv[0])
if prev == nil {
@@ -688,10 +746,10 @@
if cur := DictionaryGet(dict, key); cur != nil {
list = cur.(*build.ListExpr)
}
- AddValueToList(list, env.Pkg, getStringExpr(value, env.Pkg), !attributeMustNotBeSorted(env.Rule.Name(), attr))
+ AddValueToList(list, env.Pkg, getLabelStringExpr(value, env.Pkg), !attributeMustNotBeSorted(env.Rule.Name(), attr))
expr = list
} else {
- expr = getStringExpr(value, env.Pkg)
+ expr = getLabelStringExpr(value, env.Pkg)
}
// Set overwrites previous values.
DictionarySet(dict, key, expr)
@@ -719,7 +777,7 @@
return nil, fmt.Errorf("no colon in dict_set argument %q found", x)
}
kv = removeEscapes(kv, ':')
- expr := getStringExpr(kv[1], env.Pkg)
+ expr := getLabelStringExpr(kv[1], env.Pkg)
// Set overwrites previous values.
DictionarySet(dict, kv[0], expr)
}
@@ -777,11 +835,11 @@
// Key-Value Pair matches key.
if val, ok := kv.Value.(*build.StringExpr); ok {
if labels.Equal(val.Value, oldV, env.Pkg) {
- kv.Value = getStringExpr(newV, env.Pkg)
+ kv.Value = getLabelStringExpr(newV, env.Pkg)
}
} else if val, ok := kv.Value.(*build.Ident); ok {
if val.Name == oldV {
- kv.Value = getStringExpr(newV, env.Pkg)
+ kv.Value = getLabelStringExpr(newV, env.Pkg)
}
}
}
@@ -807,7 +865,7 @@
}
for _, val := range args {
- expr := getStringExpr(val, env.Pkg)
+ expr := getLabelStringExpr(val, env.Pkg)
prev = AddValueToList(prev, env.Pkg, expr, true)
}
