More flexible LValue syntax

--
MOS_MIGRATED_REVID=131056178

6 files changed, 69 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
index 729dd0c160..ef5bdb78de 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
@@ -84,39 +84,39 @@ public class LValue implements Serializable {
     }
 
     // Support syntax for setting an element in an array, e.g. a[5] = 2
-    // We currently do not allow slices (e.g. a[2:6] = [3]).
+    // TODO: We currently do not allow slices (e.g. a[2:6] = [3]).
     if (lvalue instanceof FuncallExpression) {
       FuncallExpression func = (FuncallExpression) lvalue;
       List<Argument.Passed> args = func.getArguments();
       if (func.getFunction().getName().equals("$index")
-          && func.getObject() instanceof Identifier
           && args.size() == 1) {
         Object key = args.get(0).getValue().eval(env);
-        assignItem(env, loc, (Identifier) func.getObject(), key, result);
+        Object evaluatedObject = func.getObject().eval(env);
+        assignItem(env, loc, evaluatedObject, key, result);
         return;
       }
     }
-
     throw new EvalException(loc,
         "can only assign to variables and tuples, not to '" + lvalue + "'");
   }
 
-  // Since dict is still immutable, the expression 'a[x] = b' creates a new dictionary and
-  // assigns it to 'a'.
   @SuppressWarnings("unchecked")
   private static void assignItem(
-      Environment env, Location loc, Identifier ident, Object key, Object value)
+      Environment env, Location loc, Object o, Object key, Object value)
       throws EvalException, InterruptedException {
-    Object o = ident.eval(env);
-    if (!(o instanceof SkylarkDict)) {
+    if (o instanceof SkylarkDict) {
+      SkylarkDict<Object, Object> dict = (SkylarkDict<Object, Object>) o;
+      dict.put(key, value, loc, env);
+    } else if (o instanceof  SkylarkList) {
+      SkylarkList<Object> list = (SkylarkList<Object>) o;
+      list.set(key, value, loc, env);
+    } else {
       throw new EvalException(
           loc,
-          "can only assign an element in a dictionary, not in a '"
+          "can only assign an element in a dictionary or a list, not in a '"
               + EvalUtils.getDataTypeName(o)
               + "'");
     }
-    SkylarkDict<Object, Object> dict = (SkylarkDict<Object, Object>) o;
-    dict.put(key, value, loc, env);
   }
 
   /**
@@ -158,7 +158,7 @@ public class LValue implements Serializable {
     }
     if (expr instanceof FuncallExpression) {
       FuncallExpression func = (FuncallExpression) expr;
-      if (func.getFunction().getName().equals("$index") && func.getObject() instanceof Identifier) {
+      if (func.getFunction().getName().equals("$index")) {
         return;
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 4b39a7d1c6..0df0aa4336 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -82,7 +82,7 @@ public class MethodLibrary {
     return str.substring(start, stop);
   }
 
-  private static int getListIndex(int index, int listSize, Location loc)
+  public static int getListIndex(int index, int listSize, Location loc)
       throws ConversionException, EvalException {
     // Get the nth element in the list
     if (index < 0) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
index 7098a1d790..05cf33b176 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
@@ -121,6 +121,25 @@ public abstract class SkylarkList<E> extends MutableCollection<E> implements Lis
     throw new UnsupportedOperationException();
   }
 
+  /**
+   * Put an entry into a SkylarkList.
+   * @param key the index
+   * @param value the associated value
+   * @param loc a {@link Location} in case of error
+   * @param env an {@link Environment}, to check Mutability
+   * @throws EvalException if the key is invalid
+   */
+  public void set(Object key, E value, Location loc, Environment env) throws EvalException {
+    checkMutable(loc, env);
+    if (!(key instanceof Integer)) {
+      throw new EvalException(loc, "list indices must be integers, not '" + key + '"');
+    }
+    int index = ((Integer) key).intValue();
+    List list = getContentsUnsafe();
+    index = MethodLibrary.getListIndex(index, list.size(), loc);
+    list.set(index, value);
+  }
+
   // Other methods
   @Override
   public void write(Appendable buffer, char quotationMark) {
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 f8823b9907..3e677be5b9 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
@@ -908,11 +908,12 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
   }
 
   @Test
-  public void testStructDictMembersAreImmutable() throws Exception {
-    checkErrorContains(
-        "can only assign to variables and tuples, not to 's.x['b']'",
+  public void testStructDictMembersAreMutable() throws Exception {
+    eval(
         "s = struct(x = {'a' : 1})",
         "s.x['b'] = 2\n");
+    assertThat(((SkylarkClassObject) lookup("s")).getValue("x"))
+        .isEqualTo(ImmutableMap.of("a", 1, "b", 2));
   }
 
   @Test
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 8f0a6f0c2f..a0a09e8709 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
@@ -868,6 +868,36 @@ public class SkylarkEvaluationTest extends EvaluationTest {
   }
 
   @Test
+  public void testNestedDictAssignmentAsLValue() throws Exception {
+    new SkylarkTest().setUp("def func():",
+        "  d = {'a' : 1}",
+        "  e = {'d': d}",
+        "  e['d']['b'] = 2",
+        "  return e",
+        "e = func()").testLookup("e", ImmutableMap.of("d", ImmutableMap.of("a", 1, "b", 2)));
+  }
+
+  @Test
+  public void testListAssignmentAsLValue() throws Exception {
+    new SkylarkTest().setUp("def func():",
+        "  a = [1, 2]",
+        "  a[1] = 3",
+        "  a[-2] = 4",
+        "  return a",
+        "a = str(func())").testLookup("a", "[4, 3]");
+  }
+
+  @Test
+  public void testNestedListAssignmentAsLValue() throws Exception {
+    new SkylarkTest().setUp("def func():",
+        "  d = [1, 2]",
+        "  e = [3, d]",
+        "  e[1][1] = 4",
+        "  return e",
+        "e = str(func())").testLookup("e", "[3, [1, 4]]");
+  }
+  
+  @Test
   public void testDictTupleAssignmentAsLValue() throws Exception {
     new SkylarkTest().setUp("def func():",
         "  d = {'a' : 1}",
@@ -902,20 +932,6 @@ public class SkylarkEvaluationTest extends EvaluationTest {
   }
 
   @Test
-  public void testListIndexAsLValueAsLValue() throws Exception {
-    new SkylarkTest()
-        .testIfErrorContains(
-            "can only assign an element in a dictionary, not in a 'list'",
-            "def id(l):",
-            "  return l",
-            "def func():",
-            "  l = id([1])",
-            "  l[0] = 2",
-            "  return l",
-            "l = func()");
-  }
-
-  @Test
   public void testTopLevelDict() throws Exception {
     new SkylarkTest().setUp("if 1:",
       "  v = 'a'",
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
index b7bd6f6306..661b8636a2 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTest.java
@@ -139,13 +139,11 @@ public class ValidationTest extends EvaluationTestCase {
 
   @Test
   public void testFuncReturningDictAssignmentAsLValue() throws Exception {
-    checkError(
-        "can only assign to variables and tuples, not to 'my_dict()['b']'",
+    parse(
         "def my_dict():",
         "  return {'a': 1}",
         "def func():",
-        "  my_dict()['b'] = 2",
-        "  return d\n");
+        "  my_dict()['b'] = 2");
   }
 
   @Test