Remove syntactic sugar when assigning to a dict item.

e.g.  a['key'] = value
  is handled through a proper lvalue, instead of using syntactic sugar.

Benefits include:
 - better error messages (reference to the '+' operator was cryptic)
 - more robust, e.g. it is compatible with the += operator
 - can be used in a tuple, e.g. a[1], a[2] = 3, 4
 - it is a step towards mutable dict

--
MOS_MIGRATED_REVID=111597545

2 files changed, 44 insertions, 15 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 7f486fc6f9..1f031d9cc2 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
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax;
 
 import static com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils.append;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils;
 import com.google.devtools.build.lib.syntax.compiler.DebugInfo.AstAccessors;
@@ -31,7 +32,9 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Class representing an LValue.
@@ -83,10 +86,45 @@ public class LValue implements Serializable {
       return;
     }
 
+    // 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]).
+    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);
+        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'.
+  // TODO(bazel-team): make dict mutable - this function should be O(1) instead of O(n).
+  private static void assignItem(
+      Environment env, Location loc, Identifier ident, Object key, Object value)
+      throws EvalException, InterruptedException {
+    Object o = ident.eval(env);
+    if (!(o instanceof Map)) {
+      throw new EvalException(
+          loc,
+          "can only assign an element in a dictionary, not in a '"
+              + EvalUtils.getDataTypeName(o)
+              + "'");
+    }
+    Map<?, ?> dict = (Map<?, ?>) o;
+    Map<Object, Object> result = new LinkedHashMap<>(dict.size() + 1);
+    result.putAll(dict);
+    result.put(key, value);
+    env.update(ident.getName(), ImmutableMap.copyOf(result));
+  }
+
   /**
    * Assign value to a single variable.
    */
@@ -124,6 +162,12 @@ public class LValue implements Serializable {
       }
       return;
     }
+    if (expr instanceof FuncallExpression) {
+      FuncallExpression func = (FuncallExpression) expr;
+      if (func.getFunction().getName().equals("$index") && func.getObject() instanceof Identifier) {
+        return;
+      }
+    }
     throw new EvalException(loc,
         "can only assign to variables and tuples, not to '" + expr + "'");
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index aa37796de6..b6fe82aa45 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -1197,21 +1197,6 @@ public class Parser {
     if (token.kind == TokenKind.EQUALS) {
       nextToken();
       Expression rvalue = parseExpression();
-      if (expression instanceof FuncallExpression) {
-        FuncallExpression func = (FuncallExpression) expression;
-        if (func.getFunction().getName().equals("$index")
-            && func.getObject() instanceof Identifier) {
-          // Special casing to translate 'ident[key] = value' to 'ident = ident + {key: value}'
-          // Note that the locations of these extra expressions are fake.
-          Preconditions.checkArgument(func.getArguments().size() == 1);
-          DictionaryLiteral dictRValue = setLocation(new DictionaryLiteral(ImmutableList.of(
-              setLocation(new DictionaryEntryLiteral(func.getArguments().get(0).getValue(), rvalue),
-                  start, token.right))), start, token.right);
-          BinaryOperatorExpression binExp = setLocation(new BinaryOperatorExpression(
-              Operator.PLUS, func.getObject(), dictRValue), start, token.right);
-          return setLocation(new AssignmentStatement(func.getObject(), binExp), start, token.right);
-        }
-      }
       return setLocation(new AssignmentStatement(expression, rvalue), start, rvalue);
     } else if (augmentedAssignmentMethods.containsKey(token.kind)) {
       Operator operator = augmentedAssignmentMethods.get(token.kind);