Make SkylarkList a List.

SkylarkList now implements the List interfaces, except that its mutating methods
throw an UnsupportedOperationException, just like ImmutableList does.
To actually mutate a SkylarkList, you need to pass a Location and a suitable
Environment object with a matching Mutability while it is still active.

Introduce SkylarkMutable and SkylarkMutable.MutableCollection to better handle
mutable data structures.

Remove some functions in EvalUtils made obsolete by this and previous changes regarding Skylark lists.

--
MOS_MIGRATED_REVID=112768457

4 files changed, 47 insertions, 60 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 553fc5eb2e..709557ab8f 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -235,7 +235,8 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
                 ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions());
     assertArtifactFilenames(action.getInputs(), "a.txt", "b.img");
     assertArtifactFilenames(action.getOutputs(), "a.txt", "b.img");
-    MoreAsserts.assertContainsSublist(action.getArguments(), "-c", "dummy_command", "", "--a", "--b");
+    MoreAsserts.assertContainsSublist(action.getArguments(),
+        "-c", "dummy_command", "", "--a", "--b");
     assertEquals("DummyMnemonic", action.getMnemonic());
     assertEquals("dummy_message", action.getProgressMessage());
     assertEquals(targetConfig.getLocalShellEnvironment(), action.getEnvironment());
@@ -502,7 +503,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "  command='I got the $(HELLO) on a $(DAVE)', ",
         "  make_variables={'HELLO': 'World', 'DAVE': type('')})");
     @SuppressWarnings("unchecked")
-    List<String> argv = (List<String>) (List<?>) ((MutableList) lookup("argv")).getList();
+    List<String> argv = (List<String>) (List<?>) (MutableList) lookup("argv");
     assertThat(argv).hasSize(3);
     assertMatches("argv[0]", "^.*/bash$", argv.get(0));
     assertThat(argv.get(1)).isEqualTo("-c");
@@ -516,7 +517,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "inputs, argv, manifests = ruleContext.resolve_command(",
         "   tools=ruleContext.attr.tools)");
     @SuppressWarnings("unchecked")
-    List<Artifact> inputs = (List<Artifact>) (List<?>) ((MutableList) lookup("inputs")).getList();
+    List<Artifact> inputs = (List<Artifact>) (List<?>) (MutableList) lookup("inputs");
     assertArtifactFilenames(inputs, "mytool.sh", "mytool", "foo_Smytool-runfiles", "t.exe");
     Map<?, ?> manifests = (Map<?, ?>) lookup("manifests");
     assertThat(manifests).hasSize(1);
@@ -537,7 +538,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "    attribute='cmd', expand_locations=True, label_dict=label_dict)",
         "inputs, argv, manifests = foo()");
     @SuppressWarnings("unchecked")
-    List<String> argv = (List<String>) (List<?>) ((MutableList) lookup("argv")).getList();
+    List<String> argv = (List<String>) (List<?>) (MutableList) lookup("argv");
     assertThat(argv).hasSize(3);
     assertMatches("argv[0]", "^.*/bash$", argv.get(0));
     assertThat(argv.get(1)).isEqualTo("-c");
@@ -555,7 +556,7 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase {
         "    command=s)",
         "argv = foo()[1]");
     @SuppressWarnings("unchecked")
-    List<String> argv = (List<String>) (List<?>) ((MutableList) lookup("argv")).getList();
+    List<String> argv = (List<String>) (List<?>) (MutableList) lookup("argv");
     assertThat(argv).hasSize(2);
     assertMatches("argv[0]", "^.*/bash$", argv.get(0));
     assertMatches("argv[1]", "^.*/resolve_me[.]script[.]sh$", argv.get(1));
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 f5cb497f67..4989cbd900 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,13 +19,14 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+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;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -36,14 +37,6 @@ import java.util.TreeMap;
 @RunWith(JUnit4.class)
 public class EvalUtilsTest {
 
-  private static List<?> makeList(Object... args) {
-    return EvalUtils.makeSequence(Arrays.<Object>asList(args), false);
-  }
-
-  private static List<?> makeTuple(Object... args) {
-    return EvalUtils.makeSequence(Arrays.<Object>asList(args), true);
-  }
-
   private static Map<Object, Object> makeDict() {
     return new LinkedHashMap<>();
   }
@@ -62,8 +55,8 @@ public class EvalUtilsTest {
   public void testDataTypeNames() throws Exception {
     assertEquals("string", EvalUtils.getDataTypeName("foo"));
     assertEquals("int", EvalUtils.getDataTypeName(3));
-    assertEquals("Tuple", EvalUtils.getDataTypeName(makeTuple(1, 2, 3)));
-    assertEquals("List",  EvalUtils.getDataTypeName(makeList(1, 2, 3)));
+    assertEquals("tuple", EvalUtils.getDataTypeName(Tuple.of(1, 2, 3)));
+    assertEquals("list",  EvalUtils.getDataTypeName(MutableList.of(null, 1, 2, 3)));
     assertEquals("dict",  EvalUtils.getDataTypeName(makeDict()));
     assertEquals("NoneType", EvalUtils.getDataTypeName(Runtime.NONE));
   }
@@ -72,8 +65,8 @@ public class EvalUtilsTest {
   public void testDatatypeMutability() throws Exception {
     assertTrue(EvalUtils.isImmutable("foo"));
     assertTrue(EvalUtils.isImmutable(3));
-    assertTrue(EvalUtils.isImmutable(makeTuple(1, 2, 3)));
-    assertFalse(EvalUtils.isImmutable(makeList(1, 2, 3)));
+    assertTrue(EvalUtils.isImmutable(Tuple.of(1, 2, 3)));
+    assertFalse(EvalUtils.isImmutable(MutableList.of(null, 1, 2, 3)));
     assertFalse(EvalUtils.isImmutable(makeDict()));
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
index 19bd520b6f..2807b1b26e 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -22,6 +22,8 @@ import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
+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;
@@ -41,14 +43,6 @@ import java.util.Map;
 @RunWith(JUnit4.class)
 public class PrinterTest {
 
-  private static List<?> makeList(Object... args) {
-    return EvalUtils.makeSequence(Arrays.<Object>asList(args), false);
-  }
-
-  private static List<?> makeTuple(Object... args) {
-    return EvalUtils.makeSequence(Arrays.<Object>asList(args), true);
-  }
-
   @Test
   public void testPrinter() throws Exception {
     // Note that prettyPrintValue and printValue only differ on behaviour of
@@ -68,22 +62,22 @@ public class PrinterTest {
     assertEquals("\"//x:x\"", Printer.repr(
         Label.parseAbsolute("//x")));
 
-    List<?> list = makeList("foo", "bar");
-    List<?> tuple = makeTuple("foo", "bar");
+    List<?> list = MutableList.of(null, "foo", "bar");
+    List<?> tuple = Tuple.of("foo", "bar");
 
     assertEquals("(1, [\"foo\", \"bar\"], 3)",
-                 Printer.str(makeTuple(1, list, 3)));
+                 Printer.str(Tuple.of(1, list, 3)));
     assertEquals("(1, [\"foo\", \"bar\"], 3)",
-                 Printer.repr(makeTuple(1, list, 3)));
+                 Printer.repr(Tuple.of(1, list, 3)));
     assertEquals("[1, (\"foo\", \"bar\"), 3]",
-                 Printer.str(makeList(1, tuple, 3)));
+                 Printer.str(MutableList.of(null, 1, tuple, 3)));
     assertEquals("[1, (\"foo\", \"bar\"), 3]",
-                 Printer.repr(makeList(1, tuple, 3)));
+                 Printer.repr(MutableList.of(null, 1, tuple, 3)));
 
     Map<Object, Object> dict = ImmutableMap.<Object, Object>of(
         1, tuple,
         2, list,
-        "foo", makeList());
+        "foo", MutableList.of(null));
     assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
                 Printer.str(dict));
     assertEquals("{1: (\"foo\", \"bar\"), 2: [\"foo\", \"bar\"], \"foo\": []}",
@@ -112,12 +106,12 @@ public class PrinterTest {
 
   @Test
   public void testFormatPositional() throws Exception {
-    assertEquals("foo 3", Printer.formatToString("%s %d", makeTuple("foo", 3)));
+    assertEquals("foo 3", Printer.formatToString("%s %d", Tuple.of("foo", 3)));
     assertEquals("foo 3", Printer.format("%s %d", "foo", 3));
 
     // Note: formatToString doesn't perform scalar x -> (x) conversion;
     // The %-operator is responsible for that.
-    assertThat(Printer.formatToString("", makeTuple())).isEmpty();
+    assertThat(Printer.formatToString("", Tuple.of())).isEmpty();
     assertEquals("foo", Printer.format("%s", "foo"));
     assertEquals("3.14159", Printer.format("%s", 3.14159));
     checkFormatPositionalFails("not all arguments converted during string formatting",
@@ -127,10 +121,10 @@ public class PrinterTest {
         "%%s", "foo");
     checkFormatPositionalFails("unsupported format character \" \" at index 1 in \"% %s\"",
         "% %s", "foo");
-    assertEquals("[1, 2, 3]", Printer.format("%s", makeList(1, 2, 3)));
-    assertEquals("(1, 2, 3)", Printer.format("%s", makeTuple(1, 2, 3)));
-    assertEquals("[]", Printer.format("%s", makeList()));
-    assertEquals("()", Printer.format("%s", makeTuple()));
+    assertEquals("[1, 2, 3]", Printer.format("%s", MutableList.of(null, 1, 2, 3)));
+    assertEquals("(1, 2, 3)", Printer.format("%s", Tuple.of(1, 2, 3)));
+    assertEquals("[]", Printer.format("%s", MutableList.of(null)));
+    assertEquals("()", Printer.format("%s", Tuple.of()));
     assertEquals("% 1 \"2\" 3", Printer.format("%% %d %r %s", 1, "2", "3"));
 
     checkFormatPositionalFails(
@@ -153,16 +147,18 @@ public class PrinterTest {
     assertEquals("\"", Printer.str("\"", '\''));
     assertEquals("'\"'", Printer.repr("\"", '\''));
 
-    List<?> list = makeList("foo", "bar");
-    List<?> tuple = makeTuple("foo", "bar");
+    List<?> list = MutableList.of(null, "foo", "bar");
+    List<?> tuple = Tuple.of("foo", "bar");
 
-    assertThat(Printer.str(makeTuple(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
-    assertThat(Printer.repr(makeTuple(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
-    assertThat(Printer.str(makeList(1, tuple, 3), '\'')).isEqualTo("[1, ('foo', 'bar'), 3]");
-    assertThat(Printer.repr(makeList(1, tuple, 3), '\'')).isEqualTo("[1, ('foo', 'bar'), 3]");
+    assertThat(Printer.str(Tuple.of(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
+    assertThat(Printer.repr(Tuple.of(1, list, 3), '\'')).isEqualTo("(1, ['foo', 'bar'], 3)");
+    assertThat(Printer.str(MutableList.of(null, 1, tuple, 3), '\''))
+        .isEqualTo("[1, ('foo', 'bar'), 3]");
+    assertThat(Printer.repr(MutableList.of(null, 1, tuple, 3), '\''))
+        .isEqualTo("[1, ('foo', 'bar'), 3]");
 
     Map<Object, Object> dict =
-        ImmutableMap.<Object, Object>of(1, tuple, 2, list, "foo", makeList());
+        ImmutableMap.<Object, Object>of(1, tuple, 2, list, "foo", MutableList.of(null));
 
     assertThat(Printer.str(dict, '\''))
         .isEqualTo("{1: ('foo', 'bar'), 2: ['foo', 'bar'], 'foo': []}");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java
index 48c27e5e90..b5f2bd8e67 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java
@@ -27,6 +27,8 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.License;
 import com.google.devtools.build.lib.packages.TriState;
+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.Type.ConversionException;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
 
@@ -292,14 +294,13 @@ public class TypeTest {
 
   @Test
   public void testStringDictBadElements() throws Exception {
-    Object input = ImmutableMap.of("foo", Arrays.asList("bar", "baz"),
-        "wiz", "bang");
+    Object input = ImmutableMap.of("foo", MutableList.of(null, "bar", "baz"), "wiz", "bang");
     try {
       Type.STRING_DICT.convert(input, null);
       fail();
     } catch (Type.ConversionException e) {
       assertThat(e).hasMessage("expected value of type 'string' for dict value element, "
-          + "but got [\"bar\", \"baz\"] (List)");
+          + "but got [\"bar\", \"baz\"] (list)");
     }
   }
 
@@ -489,14 +490,13 @@ public class TypeTest {
 
   @Test
   public void testStringListDictBadElements1() throws Exception {
-    Object input = ImmutableMap.of(Arrays.asList("foo"), Arrays.asList("bang"),
-                                   "wiz", Arrays.asList("bang"));
+    Object input = ImmutableMap.of(Tuple.of("foo"), Tuple.of("bang"), "wiz", Tuple.of("bang"));
     try {
       Type.STRING_LIST_DICT.convert(input, null);
       fail();
     } catch (Type.ConversionException e) {
       assertThat(e).hasMessage("expected value of type 'string' for dict key element, but got "
-          + "[\"foo\"] (List)");
+          + "(\"foo\",) (tuple)");
     }
   }
 
@@ -529,28 +529,25 @@ public class TypeTest {
 
   @Test
   public void testStringDictUnaryBadSecondElement() throws Exception {
-    Object input = ImmutableMap.of("foo", "bar",
-                                   "wiz", Arrays.asList("bang"));
+    Object input = ImmutableMap.of("foo", "bar", "wiz", MutableList.of(null, "bang"));
     try {
       Type.STRING_DICT_UNARY.convert(input, null, currentRule);
       fail();
     } catch (Type.ConversionException e) {
       assertThat(e).hasMessage("expected value of type 'string' for dict value element, but got "
-          + "[\"bang\"] (List)");
+          + "[\"bang\"] (list)");
     }
   }
 
   @Test
   public void testStringDictUnaryBadElements1() throws Exception {
-    Object input = ImmutableMap.of("foo", "bar",
-                                   Arrays.asList("foo", "bar"),
-                                   Arrays.<Object>asList("wiz", "bang"));
+    Object input = ImmutableMap.of("foo", "bar", Tuple.of("foo", "bar"), Tuple.of("wiz", "bang"));
     try {
       Type.STRING_DICT_UNARY.convert(input, null);
       fail();
     } catch (Type.ConversionException e) {
       assertThat(e).hasMessage("expected value of type 'string' for dict key element, but got "
-          + "[\"foo\", \"bar\"] (List)");
+          + "(\"foo\", \"bar\") (tuple)");
     }
   }