Check that deeply nested values in sets are immutable

--
MOS_MIGRATED_REVID=129331086

2 files changed, 50 insertions, 6 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
index 95423b6544..26763ea796 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java
@@ -19,16 +19,16 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
-
+import java.util.TreeMap;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.util.TreeMap;
-
 /**
  *  Test properties of the evaluator's datatypes and utility functions
  *  without actually creating any parse trees.
@@ -44,6 +44,21 @@ public class EvalUtilsTest extends EvaluationTestCase {
     return SkylarkDict.<Object, Object>of(env, 1, 1, 2, 2);
   }
 
+  private static SkylarkClassObject makeStruct(String field, Object value) {
+    return new SkylarkClassObject(ImmutableMap.of(field, value));
+  }
+
+  private static SkylarkClassObject makeBigStruct(Environment env) {
+    // struct(a=[struct(x={1:1}), ()], b=(), c={2:2})
+    return new SkylarkClassObject(ImmutableMap.<String, Object>of(
+        "a", MutableList.<Object>of(env,
+            new SkylarkClassObject(ImmutableMap.<String, Object>of(
+                "x", SkylarkDict.<Object, Object>of(env, 1, 1))),
+            Tuple.of()),
+        "b", Tuple.of(),
+        "c", SkylarkDict.<Object, Object>of(env, 2, 2)));
+  }
+
   @Test
   public void testEmptyStringToIterable() throws Exception {
     assertThat(EvalUtils.toIterable("", null)).isEmpty();
@@ -65,10 +80,15 @@ public class EvalUtilsTest extends EvaluationTestCase {
   }
 
   @Test
-  public void testDatatypeMutability() throws Exception {
+  public void testDatatypeMutabilityPrimitive() throws Exception {
     assertTrue(EvalUtils.isImmutable("foo"));
     assertTrue(EvalUtils.isImmutable(3));
+  }
+
+  @Test
+  public void testDatatypeMutabilityShallow() throws Exception {
     assertTrue(EvalUtils.isImmutable(Tuple.of(1, 2, 3)));
+    assertTrue(EvalUtils.isImmutable(makeStruct("a", 1)));
 
     // Mutability depends on the environment.
     assertTrue(EvalUtils.isImmutable(makeList(null)));
@@ -78,6 +98,17 @@ public class EvalUtilsTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testDatatypeMutabilityDeep() throws Exception {
+    assertTrue(EvalUtils.isImmutable(Tuple.<Object>of(makeList(null))));
+    assertTrue(EvalUtils.isImmutable(makeStruct("a", makeList(null))));
+    assertTrue(EvalUtils.isImmutable(makeBigStruct(null)));
+
+    assertFalse(EvalUtils.isImmutable(Tuple.<Object>of(makeList(env))));
+    assertFalse(EvalUtils.isImmutable(makeStruct("a", makeList(env))));
+    assertFalse(EvalUtils.isImmutable(makeBigStruct(env)));
+  }
+
+  @Test
   public void testComparatorWithDifferentTypes() throws Exception {
     TreeMap<Object, Object> map = new TreeMap<>(EvalUtils.SKYLARK_COMPARATOR);
     map.put(2, 3);
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
index dae2667d54..5c6535e8c9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java
@@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
 
@@ -26,6 +27,7 @@ import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -154,8 +156,19 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
   }
 
   @Test
-  public void testNsetBadCompositeItem() throws Exception {
-    checkEvalError("sets cannot contain items of type 'struct'", "set([struct(a='a')])");
+  public void testNsetGoodCompositeItem() throws Exception {
+    eval("def func():",
+         "  return set([struct(a='a')])",
+         "s = func()");
+    Collection<Object> result = get("s").toCollection();
+    assertThat(result).hasSize(1);
+    assertThat(result.iterator().next()).isInstanceOf(SkylarkClassObject.class);
+  }
+
+  @Test
+  public void testNsetBadMutableItem() throws Exception {
+    checkEvalError("sets cannot contain mutable items", "set([([],)])");
+    checkEvalError("sets cannot contain mutable items", "set([struct(a=[])])");
   }
 
   @Test