[Skylark] Support dictionaries in structs when serializing them using struct.to_json. Dictionaries are frequently used for generic configuration descriptions especially given that `struct` is not allowed in build files. In order to be JSON-compatible, only dictionaries with string keys are allowed. Technically Python also allows integers and booleans by automatically converting them to strings, but this is confusing and not necessarily a good thing to do. Fixes #5542 Closes #5543. PiperOrigin-RevId: 206049754
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Info.java b/src/main/java/com/google/devtools/build/lib/packages/Info.java index 775b8e0..4bc863b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Info.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Info.java
@@ -30,6 +30,7 @@ import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.syntax.Printer; import com.google.devtools.build.lib.syntax.Runtime; +import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.protobuf.TextFormat; @@ -218,8 +219,8 @@ return sb.toString(); } - private void printProtoTextMessage( - ClassObject object, StringBuilder sb, int indent, Location loc) throws EvalException { + private void printProtoTextMessage(ClassObject object, StringBuilder sb, int indent, Location loc) + throws EvalException { // For determinism sort the fields alphabetically. List<String> fields = new ArrayList<>(object.getFieldNames()); Collections.sort(fields); @@ -260,8 +261,7 @@ } private void printProtoTextMessage( - String key, Object value, StringBuilder sb, int indent, Location loc) - throws EvalException { + String key, Object value, StringBuilder sb, int indent, Location loc) throws EvalException { if (value instanceof SkylarkList) { for (Object item : ((SkylarkList) value)) { // TODO(bazel-team): There should be some constraint on the fields of the structs @@ -297,8 +297,7 @@ return sb.toString(); } - private void printJson( - Object value, StringBuilder sb, Location loc, String container, String key) + private void printJson(Object value, StringBuilder sb, Location loc, String container, String key) throws EvalException { if (value == Runtime.NONE) { sb.append("null"); @@ -315,6 +314,29 @@ printJson(((ClassObject) value).getValue(field), sb, loc, "struct field", field); } sb.append("}"); + } else if (value instanceof SkylarkDict) { + sb.append("{"); + String join = ""; + for (Map.Entry<?, ?> entry : ((SkylarkDict<?, ?>) value).entrySet()) { + sb.append(join); + join = ","; + if (!(entry.getKey() instanceof String)) { + String errorMessage = + "Keys must be a string but got a " + + EvalUtils.getDataTypeName(entry.getKey()) + + " for " + + container; + if (key != null) { + errorMessage += " '" + key + "'"; + } + throw new EvalException(loc, errorMessage); + } + sb.append("\""); + sb.append(entry.getKey()); + sb.append("\":"); + printJson(entry.getValue(), sb, loc, "dict value", String.valueOf(entry.getKey())); + } + sb.append("}"); } else if (value instanceof List) { sb.append("["); String join = "";
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/StructApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/StructApi.java index 5c09749..49d90e5 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/StructApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/StructApi.java
@@ -60,8 +60,9 @@ name = "to_json", doc = "Creates a JSON string from the struct parameter. This method only works if all " - + "struct elements (recursively) are strings, ints, booleans, other structs or a " - + "list of these types. Quotes and new lines in strings are escaped. " + + "struct elements (recursively) are strings, ints, booleans, other structs, a " + + "list of these types or a dictionary with string keys and values of these types. " + + "Quotes and new lines in strings are escaped. " + "Examples:<br><pre class=language-python>" + "struct(key=123).to_json()\n# {\"key\":123}\n\n" + "struct(key=True).to_json()\n# {\"key\":true}\n\n"
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index 5378df2..00f83d7 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -77,7 +77,7 @@ @Rule public ExpectedException thrown = ExpectedException.none(); @Before - public final void createBuildFile() throws Exception { + public final void createBuildFile() throws Exception { scratch.file( "foo/BUILD", "genrule(name = 'foo',", @@ -329,7 +329,6 @@ + " but got an element of type string.", "c = provider()", "attr.label_list(allow_files = True, providers = [['a', b], c])"); - } @Test @@ -358,7 +357,6 @@ assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass()); } - @Test public void testLabelListWithAspectsError() throws Exception { checkErrorContains( @@ -736,6 +734,7 @@ assertThat(c.hasAttr("a1", BuildType.LABEL_LIST)).isTrue(); assertThat(c.hasAttr("a2", Type.INTEGER)).isTrue(); } + @Test public void testRuleAttributeFlag() throws Exception { evalAndExport( @@ -953,6 +952,21 @@ } @Test + public void testJsonDictFields() throws Exception { + checkJson("struct(config={}).to_json()", "{\"config\":{}}"); + checkJson("struct(config={'key': 'value'}).to_json()", "{\"config\":{\"key\":\"value\"}}"); + checkErrorContains( + "Keys must be a string but got a int for struct field 'config'", + "struct(config={1:2}).to_json()"); + checkErrorContains( + "Keys must be a string but got a int for dict value 'foo'", + "struct(config={'foo':{1:2}}).to_json()"); + checkErrorContains( + "Keys must be a string but got a bool for struct field 'config'", + "struct(config={True: False}).to_json()"); + } + + @Test public void testJsonEncoding() throws Exception { checkJson("struct(name='value').to_json()", "{\"name\":\"value\"}"); checkJson("struct(name=['a', 'b']).to_json()", "{\"name\":[\"a\",\"b\"]}"); @@ -1537,8 +1551,6 @@ "aspect(impl, doc = 1)"); } - - @Test public void fancyExports() throws Exception { evalAndExport( @@ -1585,7 +1597,6 @@ SkylarkProvider p = (SkylarkProvider) lookup("p"); SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); - assertThat(p1.getProvider()).isEqualTo(p); assertThat(lookup("x")).isEqualTo(1); assertThat(lookup("y")).isEqualTo(2); @@ -1602,7 +1613,6 @@ SkylarkProvider p = (SkylarkProvider) lookup("p"); SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); - assertThat(p1.getProvider()).isEqualTo(p); assertThat(lookup("x")).isEqualTo(1); assertThat(lookup("y")).isEqualTo(2); @@ -1618,7 +1628,6 @@ SkylarkProvider p = (SkylarkProvider) lookup("p"); SkylarkInfo p1 = (SkylarkInfo) lookup("p1"); - assertThat(p1.getProvider()).isEqualTo(p); assertThat(lookup("y")).isEqualTo(2); } @@ -1657,7 +1666,6 @@ "unexpected keywords 'x', 'y', 'z' in call to p()"); } - @Test public void starTheOnlyAspectArg() throws Exception { checkEvalError("'*' must be the only string in 'attr_aspects' list",