Refactor SkylarkList to allow MutableList

Make SkylarkList no longer read-only to match Python and the BUILD language.
Instead, subject it to a Mutability object inherited from the Environment.

--
MOS_MIGRATED_REVID=103332973

6 files changed, 70 insertions, 75 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 8513d8339b..496e30bb57 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -329,7 +329,7 @@ public class EvaluationTest extends EvaluationTestCase {
     assertEquals(Arrays.asList(1, 2, 3, 4), x);
     assertTrue(EvalUtils.isImmutable(x));
 
-    checkEvalError("can only concatenate List (not \"Tuple\") to List",
+    checkEvalError("can only concatenate Tuple (not \"List\") to Tuple",
         "(1,2) + [3,4]"); // list + tuple
   }
 
@@ -497,8 +497,8 @@ public class EvaluationTest extends EvaluationTestCase {
     newTest()
         .testStatement("[1, 2] + [3, 4]", Arrays.asList(1, 2, 3, 4))
         .testStatement("(1, 2) + (3, 4)", ImmutableList.of(1, 2, 3, 4))
-        .testIfExactError("can only concatenate Tuple (not \"List\") to Tuple", "[1, 2] + (3, 4)")
-        .testIfExactError("can only concatenate List (not \"Tuple\") to List", "(1, 2) + [3, 4]");
+        .testIfExactError("can only concatenate List (not \"Tuple\") to List", "[1, 2] + (3, 4)")
+        .testIfExactError("can only concatenate Tuple (not \"List\") to Tuple", "(1, 2) + [3, 4]");
   }
 
   @SuppressWarnings("unchecked")
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index c7ab9da7cc..1a11e4cc89 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -901,8 +901,9 @@ public class MethodLibraryTest extends EvaluationTestCase {
 
   @Test
   public void testEnumerateBadArg() throws Exception {
+    // TODO(bazel-team): unify BUILD List and Skylark list, and get rid of this ugly message.
     new BothModesTest().testIfErrorContains(
-        "expected List or list for 'list' while calling enumerate but got string instead: a",
+        "expected List or sequence for 'list' while calling enumerate but got string instead: a",
         "enumerate('a')");
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 2ebd7df157..1cc4812fe8 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -915,7 +915,8 @@ public class SkylarkEvaluationTest extends EvaluationTest {
 
   @Test
   public void testListAnTupleConcatenationDoesNotWorkInSkylark() throws Exception {
-    new SkylarkTest().testIfExactError("cannot concatenate lists and tuples", "[1, 2] + (3, 4)");
+    new SkylarkTest().testIfExactError("can only concatenate list (not \"tuple\") to list",
+        "[1, 2] + (3, 4)");
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index 7558b6c4f3..00bc347436 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -16,67 +16,49 @@ package com.google.devtools.build.lib.syntax;
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertEquals;
 
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
+import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.util.Iterator;
-
 /**
  * Tests for SkylarkList.
  */
 @RunWith(JUnit4.class)
 public class SkylarkListTest extends EvaluationTestCase {
 
-  @Immutable
-  private static final class CustomIterable implements Iterable<Object> {
-
-    @Override
-    public Iterator<Object> iterator() {
-      // Throw an exception whenever we request the iterator, to test that lazy lists
-      // are truly lazy.
-      throw new IllegalArgumentException("Iterator requested");
-    }
-  }
-
-  private static final SkylarkList list =
-      SkylarkList.lazyList(new CustomIterable(), Integer.class);
-
-  @Override
-  public Environment newEnvironment() throws Exception {
-    return newSkylarkEnvironment().update("lazy", list);
-  }
-
   @Test
-  public void testLazyListIndex() throws Exception {
-    checkEvalError("Illegal argument in call to $index: Iterator requested", "a = lazy[0]");
+  public void testListIndex() throws Exception {
+    eval("l = [1, '2', 3]");
+    assertThat(eval("l[0]")).isEqualTo(1);
+    assertThat(eval("l[1]")).isEqualTo("2");
+    assertThat(eval("l[2]")).isEqualTo(3);
   }
 
   @Test
-  public void testLazyListSize() throws Exception {
-    checkEvalError("Illegal argument in call to len: Iterator requested", "a = len(lazy)");
+  public void testListSize() throws Exception {
+    assertThat(eval("len([42, 'hello, world', []])")).isEqualTo(3);
   }
 
   @Test
-  public void testLazyListEmpty() throws Exception {
-    checkEvalError("Iterator requested", "if lazy:\n  a = 1");
+  public void testListEmpty() throws Exception {
+    assertThat(eval("8 if [1, 2, 3] else 9")).isEqualTo(8);
   }
 
   @Test
-  public void testLazyListConcat() throws Exception {
-    eval("v = [1, 2] + lazy");
-    assertThat(lookup("v")).isInstanceOf(SkylarkList.class);
+  public void testListConcat() throws Exception {
+    assertThat(eval("[1, 2] + [3, 4]")).isEqualTo(new MutableList(Tuple.of(1, 2, 3, 4)));
   }
 
   @Test
   public void testConcatListIndex() throws Exception {
     eval("l = [1, 2] + [3, 4]",
-         "e0 = l[0]",
-         "e1 = l[1]",
-         "e2 = l[2]",
-         "e3 = l[3]");
+        "e0 = l[0]",
+        "e1 = l[1]",
+        "e2 = l[2]",
+        "e3 = l[3]");
     assertEquals(1, lookup("e0"));
     assertEquals(2, lookup("e1"));
     assertEquals(3, lookup("e2"));
@@ -100,9 +82,21 @@ public class SkylarkListTest extends EvaluationTestCase {
 
   @Test
   public void testConcatListSize() throws Exception {
-    eval("l = [1, 2] + [3, 4]",
-         "s = len(l)");
-    assertEquals(4, lookup("s"));
+    assertEquals(4, eval("len([1, 2] + [3, 4])"));
+  }
+
+  @Test
+  public void testAppend() throws Exception {
+    eval("l = [1, 2]");
+    assertEquals(eval("l.append([3, 4])"), Runtime.NONE);
+    assertEquals(lookup("l"), eval("[1, 2, [3, 4]]"));
+  }
+
+  @Test
+  public void testExtend() throws Exception {
+    eval("l = [1, 2]");
+    assertEquals(eval("l.extend([3, 4])"), Runtime.NONE);
+    assertEquals(lookup("l"), eval("[1, 2, 3, 4]"));
   }
 
   @Test
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 77ffe41e8a..fbceff9ae9 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.SkylarkList.Tuple;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -195,12 +196,12 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
       Order innerOrder = orders[1 - i];
 
       SkylarkNestedSet inner1 =
-          new SkylarkNestedSet(innerOrder, SkylarkList.tuple("1", "11"), null);
+          new SkylarkNestedSet(innerOrder, Tuple.of("1", "11"), null);
       SkylarkNestedSet inner2 =
-          new SkylarkNestedSet(innerOrder, SkylarkList.tuple("2", "22"), null);
+          new SkylarkNestedSet(innerOrder, Tuple.of("2", "22"), null);
       SkylarkNestedSet innerUnion = new SkylarkNestedSet(inner1, inner2, null);
       SkylarkNestedSet result =
-          new SkylarkNestedSet(outerOrder, SkylarkList.tuple("4", "44"), null);
+          new SkylarkNestedSet(outerOrder, Tuple.of("4", "44"), null);
       result = new SkylarkNestedSet(result, innerUnion, null);
 
       assertThat(result.toString()).isEqualTo(expected[i]);
@@ -214,10 +215,10 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
     //  (b) at least one order is "stable"
 
     for (Order first : Order.values()) {
-      SkylarkNestedSet s1 = new SkylarkNestedSet(first, SkylarkList.tuple("1", "11"), null);
+      SkylarkNestedSet s1 = new SkylarkNestedSet(first, Tuple.of("1", "11"), null);
 
       for (Order second : Order.values()) {
-        SkylarkNestedSet s2 = new SkylarkNestedSet(second, SkylarkList.tuple("2", "22"), null);
+        SkylarkNestedSet s2 = new SkylarkNestedSet(second, Tuple.of("2", "22"), null);
 
         boolean compatible = true;
 
@@ -326,9 +327,9 @@ public class SkylarkNestedSetTest extends EvaluationTestCase {
 
   private SkylarkNestedSet[] makeFourSets(Order order) throws Exception {
     return new SkylarkNestedSet[] {
-        new SkylarkNestedSet(order, SkylarkList.tuple("1", "11"), null),
-        new SkylarkNestedSet(order, SkylarkList.tuple("2", "22"), null),
-        new SkylarkNestedSet(order, SkylarkList.tuple("3", "33"), null),
-        new SkylarkNestedSet(order, SkylarkList.tuple("4", "44"), null)};
+        new SkylarkNestedSet(order, Tuple.of("1", "11"), null),
+        new SkylarkNestedSet(order, Tuple.of("2", "22"), null),
+        new SkylarkNestedSet(order, Tuple.of("3", "33"), null),
+        new SkylarkNestedSet(order, Tuple.of("4", "44"), null)};
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
index 3ebdf8dde4..fd3002e1eb 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
@@ -18,13 +18,13 @@ import static com.google.common.truth.Truth.assertThat;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
 import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
+import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-import java.util.Arrays;
-
 /**
  * Tests for the validation process of Skylark files.
  */
@@ -350,18 +350,18 @@ public class ValidationTests extends EvaluationTestCase {
 
   @Test
   public void testParentWithSkylarkModule() throws Exception {
-    Class<?> emptyListClass = SkylarkList.EMPTY_LIST.getClass();
-    Class<?> simpleListClass = SkylarkList.list(Arrays.<Integer>asList(1, 2, 3), SkylarkType.INT)
-        .getClass();
-    Class<?> tupleClass = SkylarkList.tuple(Arrays.<Object>asList(1, "a", "b")).getClass();
-
-    assertThat(SkylarkList.class.isAnnotationPresent(SkylarkModule.class)).isTrue();
-    assertThat(EvalUtils.getParentWithSkylarkModule(SkylarkList.class))
-        .isEqualTo(SkylarkList.class);
-    assertThat(EvalUtils.getParentWithSkylarkModule(emptyListClass)).isEqualTo(SkylarkList.class);
-    assertThat(EvalUtils.getParentWithSkylarkModule(simpleListClass)).isEqualTo(SkylarkList.class);
+    Class<?> emptyTupleClass = Tuple.EMPTY.getClass();
+    Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
+    Class<?> mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass();
+
+    assertThat(mutableListClass).isEqualTo(MutableList.class);
+    assertThat(MutableList.class.isAnnotationPresent(SkylarkModule.class)).isTrue();
+    assertThat(EvalUtils.getParentWithSkylarkModule(MutableList.class))
+        .isEqualTo(MutableList.class);
+    assertThat(EvalUtils.getParentWithSkylarkModule(emptyTupleClass)).isEqualTo(Tuple.class);
+    assertThat(EvalUtils.getParentWithSkylarkModule(tupleClass)).isEqualTo(Tuple.class);
     // TODO(bazel-team): make a tuple not a list anymore.
-    assertThat(EvalUtils.getParentWithSkylarkModule(tupleClass)).isEqualTo(SkylarkList.class);
+    assertThat(EvalUtils.getParentWithSkylarkModule(tupleClass)).isEqualTo(Tuple.class);
 
     // TODO(bazel-team): fix that?
     assertThat(ClassObject.class.isAnnotationPresent(SkylarkModule.class))
@@ -377,16 +377,14 @@ public class ValidationTests extends EvaluationTestCase {
   @Test
   public void testSkylarkTypeEquivalence() throws Exception {
     // All subclasses of SkylarkList are made equivalent
-    Class<?> emptyListClass = SkylarkList.EMPTY_LIST.getClass();
-    Class<?> simpleListClass = SkylarkList.list(Arrays.<Integer>asList(1, 2, 3), SkylarkType.INT)
-        .getClass();
-    Class<?> tupleClass = SkylarkList.tuple(Arrays.<Object>asList(1, "a", "b")).getClass();
-
-    assertThat(SkylarkType.of(SkylarkList.class)).isEqualTo(SkylarkType.LIST);
-    assertThat(SkylarkType.of(emptyListClass)).isEqualTo(SkylarkType.LIST);
-    assertThat(SkylarkType.of(simpleListClass)).isEqualTo(SkylarkType.LIST);
-    // TODO(bazel-team): make a tuple not a list anymore.
-    assertThat(SkylarkType.of(tupleClass)).isEqualTo(SkylarkType.LIST);
+    Class<?> emptyTupleClass = Tuple.EMPTY.getClass();
+    Class<?> tupleClass = Tuple.of(1, "a", "b").getClass();
+    Class<?> mutableListClass = new MutableList(Tuple.of(1, 2, 3), env).getClass();
+
+    assertThat(SkylarkType.of(mutableListClass)).isEqualTo(SkylarkType.LIST);
+    assertThat(SkylarkType.of(emptyTupleClass)).isEqualTo(SkylarkType.TUPLE);
+    assertThat(SkylarkType.of(tupleClass)).isEqualTo(SkylarkType.TUPLE);
+    assertThat(SkylarkType.TUPLE).isNotEqualTo(SkylarkType.LIST);
 
 
     // Also for ClassObject