[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

1 files changed, 17 insertions, 9 deletions

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 5378df232e..00f83d741e 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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
   @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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
             + " but got an element of type string.",
         "c = provider()",
         "attr.label_list(allow_files = True,  providers = [['a', b], c])");
-
   }
 
   @Test
@@ -358,7 +357,6 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass());
   }
 
-
   @Test
   public void testLabelListWithAspectsError() throws Exception {
     checkErrorContains(
@@ -736,6 +734,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
   }
 
   @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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "aspect(impl, doc = 1)");
   }
 
-
-
   @Test
   public void fancyExports() throws Exception {
     evalAndExport(
@@ -1585,7 +1597,6 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     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 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     SkylarkProvider p = (SkylarkProvider) lookup("p");
     SkylarkInfo p1 = (SkylarkInfo) lookup("p1");
 
-
     assertThat(p1.getProvider()).isEqualTo(p);
     assertThat(lookup("y")).isEqualTo(2);
   }
@@ -1657,7 +1666,6 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "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",