Cosmetic changes moved out of []

These shouldn't affect the semantic of the program in any significant way,
but will hush the linter and other such metaprograms.

--
MOS_MIGRATED_REVID=86089271

21 files changed, 197 insertions, 136 deletions

diff --git a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java
index d2208ea85b..3903f2456d 100644
--- a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java
+++ b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationProcessor.java
@@ -98,7 +98,7 @@ public class SkylarkDocumentationProcessor {
     for (SkylarkModuleDoc builtinObject : builtinModules.values()) {
       // Check the return type for built-in functions, it can be a module previously not added.
       for (SkylarkBuiltinMethod builtinMethod : builtinObject.getBuiltinMethods().values()) {
-        Class<?> type = builtinMethod.annotation.returnType(); 
+        Class<?> type = builtinMethod.annotation.returnType();
         if (type.isAnnotationPresent(SkylarkModule.class)) {
           explorer.collect(type.getAnnotation(SkylarkModule.class), type, modules);
         }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index b82713f8e4..3e9ddc5958 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -349,7 +349,7 @@ public final class RuleConfiguredTargetBuilder {
       // SkylarkNestedSets cannot have composite items.
       Class<?> genericType = ((SkylarkNestedSet) object).getGenericType();
       if (!genericType.equals(Object.class) && !isSimpleSkylarkObjectSafe(genericType)) {
-        throw new IllegalArgumentException(EvalUtils.getDatatypeName(genericType));
+        throw new IllegalArgumentException(EvalUtils.getDataTypeName(genericType));
       }
       return;
     } else if (object instanceof Map<?, ?>) {
@@ -365,7 +365,7 @@ public final class RuleConfiguredTargetBuilder {
       }
       return;
     }
-    throw new IllegalArgumentException(EvalUtils.getDatatypeName(object));
+    throw new IllegalArgumentException(EvalUtils.getDataTypeName(object));
   }
 
   private boolean isSimpleSkylarkObjectSafe(Class<?> type) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
index 5969697337..65b4540965 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
@@ -465,7 +465,7 @@ public class MethodLibrary {
         // TODO(bazel-team): This is dead code, get rid of it.
         throw new EvalException(ast.getLocation(), String.format(
             "Unsupported datatype (%s) for indexing, only works for dict and list",
-            EvalUtils.getDatatypeName(collectionCandidate)));
+            EvalUtils.getDataTypeName(collectionCandidate)));
       }
     }
   };
@@ -552,7 +552,7 @@ public class MethodLibrary {
       int l = EvalUtils.size(arg);
       if (l == -1) {
         throw new EvalException(ast.getLocation(),
-            EvalUtils.getDatatypeName(arg) + " is not iterable");
+            EvalUtils.getDataTypeName(arg) + " is not iterable");
       }
       return l;
     }
@@ -792,7 +792,7 @@ public class MethodLibrary {
           return args[2];
         } else {
           throw new EvalException(ast.getLocation(), "Object of type '"
-              + EvalUtils.getDatatypeName(obj) + "' has no field '" + name + "'");
+              + EvalUtils.getDataTypeName(obj) + "' has no field '" + name + "'");
         }
       }
       return result;
@@ -833,7 +833,7 @@ public class MethodLibrary {
     @Override
     public Object call(Object[] args, FuncallExpression ast) throws EvalException {
       // There is no 'type' type in Skylark, so we return a string with the type name.
-      return EvalUtils.getDatatypeName(args[0]);
+      return EvalUtils.getDataTypeName(args[0]);
     }
   };
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index f495502d70..f8f442f1ff 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -127,14 +127,14 @@ public final class RuleClass {
     @Override
     public String getErrorReason(Rule param) {
       if (pathSegment == ANY_SEGMENT) {
-        return param.getRuleClass() + " rules have to be under a " +
-            StringUtil.joinEnglishList(values, "or", "'") + " directory";
+        return param.getRuleClass() + " rules have to be under a "
+            + StringUtil.joinEnglishList(values, "or", "'") + " directory";
       } else if (pathSegment == 1) {
         return param.getRuleClass() + " rules are only allowed in "
             + StringUtil.joinEnglishList(StringUtil.append(values, "//", ""), "or");
       } else {
-          return param.getRuleClass() + " rules are only allowed in packages which " +
-              StringUtil.ordinal(pathSegment) + " is " + StringUtil.joinEnglishList(values, "or");
+          return param.getRuleClass() + " rules are only allowed in packages which "
+              + StringUtil.ordinal(pathSegment) + " is " + StringUtil.joinEnglishList(values, "or");
       }
     }
 
@@ -286,8 +286,8 @@ public final class RuleClass {
             Preconditions.checkState(presentAttribute != null,
                 "Missing mandatory '%s' attribute in normal rule class.", attribute.getName());
             Preconditions.checkState(presentAttribute.getType().equals(attribute.getType()),
-                "Mandatory attribute '%s' in normal rule class has incorrect type (expcected" +
-                    " %s).", attribute.getName(), attribute.getType());
+                "Mandatory attribute '%s' in normal rule class has incorrect type (expected"
+                + " %s).", attribute.getName(), attribute.getType());
           }
         }
       },
@@ -384,8 +384,8 @@ public final class RuleClass {
 
       @Override
       public boolean equals(Object o) {
-        return (o instanceof RuleClassNamePredicate) &&
-            ruleClasses.equals(((RuleClassNamePredicate) o).ruleClasses);
+        return (o instanceof RuleClassNamePredicate)
+            && ruleClasses.equals(((RuleClassNamePredicate) o).ruleClasses);
       }
 
       @Override
@@ -1024,8 +1024,8 @@ public final class RuleClass {
    * which does not have the configured targets available.
    *
    * <p>This should in theory only contain subclasses of
-   * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}, but our current dependency
-   * structure does not allow a reference to that class here.
+   * {@link com.google.devtools.build.lib.analysis.TransitiveInfoProvider}, but
+   * our current dependency structure does not allow a reference to that class here.
    */
   public ImmutableSet<Class<?>> getAdvertisedProviders() {
     return advertisedProviders;
@@ -1401,8 +1401,8 @@ public final class RuleClass {
 
     Integer attrIndex = getAttributeIndex(attrName);
     if (attrIndex == null) {
-      rule.reportError(rule.getLabel() + ": no such attribute '" + attrName +
-                       "' in '" + name + "' rule", eventHandler);
+      rule.reportError(rule.getLabel() + ": no such attribute '" + attrName
+          + "' in '" + name + "' rule", eventHandler);
       return null;
     }
 
@@ -1431,8 +1431,8 @@ public final class RuleClass {
 
     if (attrName.equals("visibility")) {
       List<Label> attrList = (List<Label>) converted;
-      if (!attrList.isEmpty() &&
-        ConstantRuleVisibility.LEGACY_PUBLIC_LABEL.equals(attrList.get(0))) {
+      if (!attrList.isEmpty()
+          && ConstantRuleVisibility.LEGACY_PUBLIC_LABEL.equals(attrList.get(0))) {
         rule.reportError(rule.getLabel() + ": //visibility:legacy_public only allowed in package "
             + "declaration", eventHandler);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java
index 9172a1f5dc..48ba948090 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java
@@ -319,7 +319,7 @@ public abstract class Type<T> {
       }
       builder.append(", but got '");
       EvalUtils.printValue(value, builder);
-      builder.append("' (").append(EvalUtils.getDatatypeName(value)).append(")");
+      builder.append("' (").append(EvalUtils.getDataTypeName(value)).append(")");
       return builder.toString();
     }
 
@@ -609,7 +609,7 @@ public abstract class Type<T> {
             STRING.convert(x, what, currentRule));
       } catch (Label.SyntaxException e) {
         throw new ConversionException("invalid label '" + x + "' in "
-            + what + ": "+ e.getMessage());
+            + what + ": " + e.getMessage());
       }
     }
   }
@@ -813,6 +813,7 @@ public abstract class Type<T> {
     }
   }
 
+  /** A type for lists of a given element type */
   public static class ListType<ELEM> extends Type<List<ELEM>> {
 
     private final Type<ELEM> elemType;
@@ -905,6 +906,7 @@ public abstract class Type<T> {
     }
   }
 
+  /** Type for lists of arbitrary objects */
   public static class ObjectListType extends ListType<Object> {
 
     private static final Type<Object> elemType = new ObjectType();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 39b8836f64..4fe9cf0863 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -397,7 +397,7 @@ public class SkylarkRuleClassFunctions {
       } else {
         throw new EvalException(loc,
             "Invalid text format, expected a struct, a string, a bool, or an int but got a "
-            + EvalUtils.getDatatypeName(value) + " for " + container + " '" + key + "'");
+            + EvalUtils.getDataTypeName(value) + " for " + container + " '" + key + "'");
       }
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
index da96f8a599..c6ca475b7c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
@@ -123,7 +123,7 @@ public class SkylarkRuleImplementationFunctions {
           builder.setExecutable((PathFragment) exe);
         } else {
           throw new EvalException(loc, "expected file or PathFragment for "
-              + "executable but got " + EvalUtils.getDatatypeName(exe) + " instead");
+              + "executable but got " + EvalUtils.getDataTypeName(exe) + " instead");
         }
       }
       if (params.containsKey("command") == params.containsKey("executable")) {
@@ -141,7 +141,7 @@ public class SkylarkRuleImplementationFunctions {
           builder.setShellCommand(castList(commandList, String.class, "command"));
         } else {
           throw new EvalException(loc, "expected string or list of strings for "
-              + "command instead of " + EvalUtils.getDatatypeName(command));
+              + "command instead of " + EvalUtils.getDataTypeName(command));
         }
       }
       if (params.containsKey("command_line")) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
index 619e841864..556fe2e15b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
@@ -80,7 +80,7 @@ public final class AssignmentStatement extends Statement {
         throw new EvalException(getLocation(), String.format("Incompatible variable types, "
             + "trying to assign %s (type of %s) to variable %s which is already %s",
             EvalUtils.prettyPrintValue(result),
-            EvalUtils.getDatatypeName(result),
+            EvalUtils.getDataTypeName(result),
             ident.getName(),
             EvalUtils.getDataTypeNameFromClass(variableType)));
       }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index 2b9145021f..d54e8e8856 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -71,8 +71,8 @@ public final class BinaryOperatorExpression extends Expression {
     try {
       return ((Comparable<Object>) lval).compareTo(rval);
     } catch (ClassCastException e) {
-      throw new EvalException(getLocation(), "Cannot compare " + EvalUtils.getDatatypeName(lval)
-          + " with " + EvalUtils.getDatatypeName(rval));
+      throw new EvalException(getLocation(), "Cannot compare " + EvalUtils.getDataTypeName(lval)
+          + " with " + EvalUtils.getDataTypeName(rval));
     }
   }
 
@@ -117,9 +117,9 @@ public final class BinaryOperatorExpression extends Expression {
           List<?> rlist = (List<?>) rval;
           if (EvalUtils.isImmutable(llist) != EvalUtils.isImmutable(rlist)) {
             throw new EvalException(getLocation(), "can only concatenate "
-                + EvalUtils.getDatatypeName(rlist) + " (not \""
-                + EvalUtils.getDatatypeName(llist) + "\") to "
-                + EvalUtils.getDatatypeName(rlist));
+                + EvalUtils.getDataTypeName(rlist) + " (not \""
+                + EvalUtils.getDataTypeName(llist) + "\") to "
+                + EvalUtils.getDataTypeName(rlist));
           }
           if (llist instanceof GlobList<?> || rlist instanceof GlobList<?>) {
             return GlobList.concat(llist, rlist);
@@ -270,8 +270,8 @@ public final class BinaryOperatorExpression extends Expression {
 
     throw new EvalException(getLocation(),
         "unsupported operand types for '" + operator + "': '"
-        + EvalUtils.getDatatypeName(lval) + "' and '"
-        + EvalUtils.getDatatypeName(rval) + "'");
+        + EvalUtils.getDataTypeName(lval) + "' and '"
+        + EvalUtils.getDataTypeName(rval) + "'");
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
index 8f79739d02..953840b3f1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DictionaryLiteral.java
@@ -20,7 +20,7 @@ import java.util.List;
 import java.util.Map;
 
 /**
- * Syntax node for dictionary literals. 
+ * Syntax node for dictionary literals.
  */
 public class DictionaryLiteral extends Expression {
 
@@ -63,6 +63,11 @@ public class DictionaryLiteral extends Expression {
     this.entries = ImmutableList.copyOf(exprs);
   }
 
+  /** A new literal for an empty dictionary, onto which a new location can be specified */
+  public static DictionaryLiteral emptyDict() {
+    return new DictionaryLiteral(ImmutableList.<DictionaryEntryLiteral>of());
+  }
+
   @Override
   Object eval(Environment env) throws EvalException, InterruptedException {
     // We need LinkedHashMap to maintain the order during iteration (e.g. for loops)
@@ -72,7 +77,6 @@ public class DictionaryLiteral extends Expression {
         throw new EvalException(getLocation(), "null expression in " + this);
       }
       map.put(entry.key.eval(env), entry.value.eval(env));
-      
     }
     return map;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
index b0ae5a975d..6c064e44b8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java
@@ -62,7 +62,7 @@ public final class DotExpression extends Expression {
         }
       }
       throw new EvalException(getLocation(), "Object of type '"
-          + EvalUtils.getDatatypeName(objValue) + "' has no field '" + name + "'");
+          + EvalUtils.getDataTypeName(objValue) + "' has no field '" + name + "'");
     }
     return result;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
index 27aba0fc3c..91b7304cc7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalException.java
@@ -16,6 +16,10 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.util.LoggingUtil;
+
+import java.util.logging.Level;
+
 
 /**
  * Exceptions thrown during evaluation of BUILD ASTs or Skylark extensions.
@@ -54,18 +58,45 @@ public class EvalException extends Exception {
     this.dueToIncompleteAST = dueToIncompleteAST;
   }
 
-  private EvalException(Location location, Throwable cause) {
+  /**
+   * @param location the location where evaluation/execution failed.
+   * @param message the error message.
+   * @param cause a Throwable that caused this exception.
+   */
+  public EvalException(Location location, String message, Throwable cause) {
     super(cause);
     this.location = location;
     // This is only used from Skylark, it's useful for debugging. Note that this only happens
     // when the Precondition below kills the execution anyway.
-    if (cause.getMessage() == null) {
-      cause.printStackTrace();
+    if (message == null) {
+      message = "";
     }
-    this.message = Preconditions.checkNotNull(cause.getMessage());
+    if (cause != null) {
+      message = message + (message.isEmpty() ? "" : "\n") + cause.getMessage();
+    }
+    if (message.isEmpty()) {
+      LoggingUtil.logToRemote(Level.SEVERE, "Invalid EvalException", cause);
+      throw new IllegalArgumentException("Invalid EvalException");
+    }
+    this.message = message;
     this.dueToIncompleteAST = false;
   }
 
+  public EvalException(Location location, Throwable cause) {
+    this(location, null, cause);
+  }
+
+  /**
+   * Returns the error message with location info if exists.
+   */
+  public String print() { // TODO(bazel-team): do we also need a toString() method?
+    return this.getClass().getName()
+        + (getLocation() == null ? "" : " at " + getLocation()) + ": "
+        + (message == null ? "" : message + "\n")
+        + (dueToIncompleteAST ? "due to incomplete AST\n" : "")
+        + (getCause() != null && getCause().getMessage() != null ? getCause().getMessage() : "");
+  }
+
   /**
    * Returns the error message.
    */
@@ -81,25 +112,34 @@ public class EvalException extends Exception {
     return location;
   }
 
+  /**
+   * Returns a boolean that tells whether this exception was due to an incomplete AST
+   */
   public boolean isDueToIncompleteAST() {
     return dueToIncompleteAST;
   }
 
   /**
    * A class to support a special case of EvalException when the cause of the error is an
-   * Exception during a direct Java call.
+   * Exception during a direct Java call. Allow the throwing code to provide context in a message.
    */
   public static final class EvalExceptionWithJavaCause extends EvalException {
 
-    public EvalExceptionWithJavaCause(Location location, Throwable cause) {
-      super(location, cause);
+    /**
+     * @param location the location where evaluation/execution failed.
+     * @param message the error message.
+     * @param cause a Throwable that caused this exception.
+     */
+    public EvalExceptionWithJavaCause(Location location, String message, Throwable cause) {
+      super(location, message, cause);
     }
-  }
 
-  /**
-   * Returns the error message with location info if exists.
-   */
-  public String print() {
-    return getLocation() == null ? getMessage() : getLocation().print() + ": " + getMessage();
+    /**
+     * @param location the location where evaluation/execution failed.
+     * @param cause a Throwable that caused this exception.
+     */
+    public EvalExceptionWithJavaCause(Location location, Throwable cause) {
+      this(location, null, cause);
+    }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 4de32faaad..57aec07e56 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -24,7 +24,6 @@ import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 import java.io.IOException;
@@ -169,10 +168,11 @@ public abstract class EvalUtils {
     return c;
   }
 
+  // TODO(bazel-team): shouldn't we agree on Datatype vs DataType in the two methods below???
   /**
    * Returns a pretty name for the datatype of object 'o' in the Build language.
    */
-  public static String getDatatypeName(Object o) {
+  public static String getDataTypeName(Object o) {
     Preconditions.checkNotNull(o);
     if (o instanceof SkylarkList) {
       return ((SkylarkList) o).isTuple() ? "tuple" : "list";
@@ -195,9 +195,12 @@ public abstract class EvalUtils {
     } else if (c.equals(Void.TYPE) || c.equals(Environment.NoneType.class)) {
       return "None";
     } else if (List.class.isAssignableFrom(c)) {
+      // TODO(bazel-team): for better debugging, we should distinguish "java tuple" and "java list"
+      // from "tuple" and "list" -- or better yet, only use one set of pure data structures
+      // everywhere and eliminate all calls to .append and .extend from the code base.
       return isTuple(c) ? "tuple" : "list";
     } else if (GlobList.class.isAssignableFrom(c)) {
-      return "list";
+      return "glob list";
     } else if (Map.class.isAssignableFrom(c)) {
       return "dict";
     } else if (Function.class.isAssignableFrom(c)) {
@@ -206,10 +209,10 @@ public abstract class EvalUtils {
       return "FilesetEntry";
     } else if (NestedSet.class.isAssignableFrom(c) || SkylarkNestedSet.class.isAssignableFrom(c)) {
       return "set";
-    } else if (SkylarkClassObject.class.isAssignableFrom(c)) {
+    } else if (ClassObject.class.isAssignableFrom(c)) {
       return "struct";
     } else if (SkylarkList.class.isAssignableFrom(c)) {
-      // TODO(bazel-team): this is not entirely correct, it can also be a tuple.
+      // TODO(bazel-team): Refactor the class hierarchy so we can distinguish list and tuple types.
       return "list";
     } else if (c.isAnnotationPresent(SkylarkModule.class)) {
       SkylarkModule module = c.getAnnotation(SkylarkModule.class);
@@ -261,28 +264,21 @@ public abstract class EvalUtils {
 
     } else if (o instanceof List<?>) {
       List<?> seq = (List<?>) o;
-      boolean isTuple = isImmutable(seq);
-      String sep = "";
-      buffer.append(isTuple ? '(' : '[');
-      for (int ii = 0, len = seq.size(); ii < len; ++ii) {
-        buffer.append(sep);
-        prettyPrintValue(seq.get(ii), buffer);
-        sep = ", ";
-      }
-      buffer.append(isTuple ? ')' : ']');
+      printList(seq, isImmutable(seq), buffer);
+
+    } else if (o instanceof SkylarkList) {
+      SkylarkList list = (SkylarkList) o;
+      printList(list.toList(), list.isTuple(), buffer);
 
     } else if (o instanceof Map<?, ?>) {
       Map<?, ?> dict = (Map<?, ?>) o;
-      buffer.append('{');
-      String sep = "";
-      for (Map.Entry<?, ?> entry : dict.entrySet()) {
-        buffer.append(sep);
-        prettyPrintValue(entry.getKey(), buffer);
-        buffer.append(": ");
-        prettyPrintValue(entry.getValue(), buffer);
-        sep = ", ";
-      }
-      buffer.append('}');
+      printList(dict.entrySet(), "{", ", ", "}", buffer);
+
+    } else if (o instanceof Map.Entry<?, ?>) {
+      Map.Entry<?, ?> entry = (Map.Entry<?, ?>) o;
+      prettyPrintValue(entry.getKey(), buffer);
+      buffer.append(": ");
+      prettyPrintValue(entry.getValue(), buffer);
 
     } else if (o instanceof Function) {
       Function func = (Function) o;
@@ -310,6 +306,27 @@ public abstract class EvalUtils {
     }
   }
 
+  private static void printList(Iterable<?> list,
+      String before, String separator, String after, Appendable buffer) throws IOException {
+    String sep = "";
+    buffer.append(before);
+    for (Object o : list) {
+      buffer.append(sep);
+      prettyPrintValue(o, buffer);
+      sep = separator;
+    }
+    buffer.append(after);
+  }
+
+  private static void printList(Iterable<?> list, boolean isTuple, Appendable buffer)
+      throws IOException {
+    if (isTuple) {
+      printList(list, "(", ", ", ")", buffer);
+    } else {
+      printList(list, "[", ", ", "]", buffer);
+    }
+  }
+
   private static List<?> makeList(Collection<?> list) {
     return list == null ? Lists.newArrayList() : Lists.newArrayList(list);
   }
@@ -546,7 +563,7 @@ public abstract class EvalUtils {
       return ((SkylarkNestedSet) o).toCollection();
     } else {
       throw new EvalException(loc,
-          "type '" + EvalUtils.getDatatypeName(o) + "' is not a collection");
+          "type '" + getDataTypeName(o) + "' is not a collection");
     }
   }
 
@@ -565,7 +582,7 @@ public abstract class EvalUtils {
       return ((Map<Object, Object>) o).keySet();
     } else {
       throw new EvalException(loc,
-          "type '" + EvalUtils.getDatatypeName(o) + "' is not an iterable");
+          "type '" + getDataTypeName(o) + "' is not an iterable");
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
index 34a4eea416..76a46eeb43 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java
@@ -84,7 +84,7 @@ public final class ForStatement extends Statement {
   void validate(ValidationEnvironment env) throws EvalException {
     if (env.isTopLevel()) {
       throw new EvalException(getLocation(),
-          "'For' is not allowed as a the top level statement");
+          "'For' is not allowed as a top level statement");
     }
     // TODO(bazel-team): validate variable. Maybe make it temporarily readonly.
     SkylarkType type = collection.validate(env);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index a5190a64d9..38f4a7d82a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -316,7 +316,7 @@ public final class FuncallExpression extends Expression {
     result = SkylarkType.convertToSkylark(result, method);
     if (result != null && !EvalUtils.isSkylarkImmutable(result.getClass())) {
       throw new EvalException(loc, "Method '" + methodName
-          + "' returns a mutable object (type of " + EvalUtils.getDatatypeName(result) + ")");
+          + "' returns a mutable object (type of " + EvalUtils.getDataTypeName(result) + ")");
     }
     return result;
   }
@@ -386,7 +386,7 @@ public final class FuncallExpression extends Expression {
       if (!first) {
         sb.append(", ");
       }
-      sb.append(EvalUtils.getDatatypeName(obj));
+      sb.append(EvalUtils.getDataTypeName(obj));
       first = false;
     }
     return sb.append(")").toString();
@@ -410,12 +410,12 @@ public final class FuncallExpression extends Expression {
       throws EvalException {
     if (!(items instanceof Map<?, ?>)) {
       throw new EvalException(getLocation(),
-          "Argument after ** must be a dictionary, not " + EvalUtils.getDatatypeName(items));
+          "Argument after ** must be a dictionary, not " + EvalUtils.getDataTypeName(items));
     }
     for (Map.Entry<?, ?> entry : ((Map<?, ?>) items).entrySet()) {
       if (!(entry.getKey() instanceof String)) {
         throw new EvalException(getLocation(),
-            "Keywords must be strings, not " + EvalUtils.getDatatypeName(entry.getKey()));
+            "Keywords must be strings, not " + EvalUtils.getDataTypeName(entry.getKey()));
       }
       addKeywordArg(kwargs, (String) entry.getKey(), entry.getValue());
     }
@@ -499,14 +499,14 @@ public final class FuncallExpression extends Expression {
       } else {
         throw new EvalException(getLocation(), String.format(
             "function '%s' is not defined on '%s'", func.getName(),
-            EvalUtils.getDatatypeName(objValue)));
+            EvalUtils.getDataTypeName(objValue)));
       }
     }
 
     Object funcValue = func.eval(env);
     if (!(funcValue instanceof Function)) {
       throw new EvalException(getLocation(),
-                              "'" + EvalUtils.getDatatypeName(funcValue)
+                              "'" + EvalUtils.getDataTypeName(funcValue)
                               + "' object is not callable");
     }
     Function function = (Function) funcValue;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
index 3877a9c3ab..7607e4a4d4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java
@@ -48,7 +48,7 @@ public final class IfStatement extends Statement {
     @Override
     public String toString() {
       // TODO(bazel-team): see TODO in the outer class
-      return "[el]if " + condition + ": ...\n";
+      return "[el]if " + condition + ": " + stmts + "\n";
     }
 
     @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index f4e98153d4..3b64492f03 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -39,6 +40,14 @@ import java.util.Stack;
  */
 public final class Lexer {
 
+  private static final Map<Character, TokenKind> EQUAL_TOKENS =
+      ImmutableMap.<Character, TokenKind>of(
+          '=', TokenKind.EQUALS_EQUALS,
+          '!', TokenKind.NOT_EQUALS,
+          '>', TokenKind.GREATER_EQUALS,
+          '<', TokenKind.LESS_EQUALS,
+          '+', TokenKind.PLUS_EQUALS);
+
   private final EventHandler eventHandler;
 
   // Input buffer and position
@@ -270,7 +279,7 @@ public final class Lexer {
   /**
    * Scans a string literal delimited by 'quot', containing escape sequences.
    *
-   * ON ENTRY: 'pos' is 1 + the index of the first delimiter
+   * <p>ON ENTRY: 'pos' is 1 + the index of the first delimiter
    * ON EXIT: 'pos' is 1 + the index of the last delimiter.
    *
    * @return the string-literal token.
@@ -531,7 +540,7 @@ public final class Lexer {
   /**
    * Scans an integer literal.
    *
-   * ON ENTRY: 'pos' is 1 + the index of the first char in the literal.
+   * <p>ON ENTRY: 'pos' is 1 + the index of the first char in the literal.
    * ON EXIT: 'pos' is 1 + the index of the last char in the literal.
    *
    * @return the integer token.
@@ -573,31 +582,16 @@ public final class Lexer {
     }
     char c1 = buffer[pos];
     char c2 = buffer[pos + 1];
+    TokenKind tok = null;
     if (c2 == '=') {
-      switch (c1) {
-        case '=': {
-          addToken(new Token(TokenKind.EQUALS_EQUALS, pos, pos + 2));
-          return true;
-        }
-        case '!': {
-          addToken(new Token(TokenKind.NOT_EQUALS, pos, pos + 2));
-          return true;
-        }
-        case '>': {
-          addToken(new Token(TokenKind.GREATER_EQUALS, pos, pos + 2));
-          return true;
-        }
-        case '<': {
-          addToken(new Token(TokenKind.LESS_EQUALS, pos, pos + 2));
-          return true;
-        }
-        case '+': {
-          addToken(new Token(TokenKind.PLUS_EQUALS, pos, pos + 2));
-          return true;
-        }
-      }
+      tok = EQUAL_TOKENS.get(c1);
+    }
+    if (tok == null) {
+      return false;
+    } else {
+      addToken(new Token(tok, pos, pos + 2));
+      return true;
     }
-    return false;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index b18c21c66b..d8c7811d30 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -47,6 +48,11 @@ public final class ListLiteral extends Expression {
     return new ListLiteral(Kind.TUPLE, exprs);
   }
 
+  /** A new literal for an empty list, onto which a new location can be specified */
+  public static ListLiteral emptyList() {
+    return makeList(Collections.<Expression>emptyList());
+  }
+
   /**
    * Returns the list of expressions for each element of the tuple.
    */
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 66c3c6729f..d86946a5af 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
@@ -754,8 +754,7 @@ class Parser {
     int start = token.left;
     expect(TokenKind.LBRACKET);
     if (token.kind == TokenKind.RBRACKET) { // empty List
-      ListLiteral literal =
-          ListLiteral.makeList(Collections.<Expression>emptyList());
+      ListLiteral literal = ListLiteral.emptyList();
       setLocation(literal, start, token.right);
       nextToken();
       return literal;
@@ -825,9 +824,8 @@ class Parser {
   private Expression parseDictExpression() {
     int start = token.left;
     expect(TokenKind.LBRACE);
-    if (token.kind == TokenKind.RBRACE) { // empty List
-      DictionaryLiteral literal =
-          new DictionaryLiteral(ImmutableList.<DictionaryEntryLiteral>of());
+    if (token.kind == TokenKind.RBRACE) { // empty Dict
+      DictionaryLiteral literal = DictionaryLiteral.emptyDict();
       setLocation(literal, start, token.right);
       nextToken();
       return literal;
@@ -1075,21 +1073,22 @@ class Parser {
   }
 
   // if_stmt ::= IF expr ':' suite [ELIF expr ':' suite]* [ELSE ':' suite]?
-  private void parseIfStatement(List<Statement> list) {
+  private IfStatement parseIfStatement() {
     int start = token.left;
     List<ConditionalStatements> thenBlocks = new ArrayList<>();
     thenBlocks.add(parseConditionalStatements(TokenKind.IF));
     while (token.kind == TokenKind.ELIF) {
       thenBlocks.add(parseConditionalStatements(TokenKind.ELIF));
     }
-    List<Statement> elseBlock = new ArrayList<>();
+    List<Statement> elseBlock;
     if (token.kind == TokenKind.ELSE) {
       expect(TokenKind.ELSE);
       expect(TokenKind.COLON);
-      parseSuite(elseBlock);
+      elseBlock = parseSuite();
+    } else {
+      elseBlock = ImmutableList.of();
     }
-    Statement stmt = new IfStatement(thenBlocks, elseBlock);
-    list.add(setLocation(stmt, start, token.right));
+    return setLocation(new IfStatement(thenBlocks, elseBlock), start, token.right);
   }
 
   // cond_stmts ::= [EL]IF expr ':' suite
@@ -1098,8 +1097,7 @@ class Parser {
     expect(tokenKind);
     Expression expr = parseExpression();
     expect(TokenKind.COLON);
-    List<Statement> thenBlock = new ArrayList<>();
-    parseSuite(thenBlock);
+    List<Statement> thenBlock = parseSuite();
     ConditionalStatements stmt = new ConditionalStatements(expr, thenBlock);
     return setLocation(stmt, start, token.right);
   }
@@ -1112,8 +1110,7 @@ class Parser {
     expect(TokenKind.IN);
     Expression collection = parseExpression();
     expect(TokenKind.COLON);
-    List<Statement> block = new ArrayList<>();
-    parseSuite(block);
+    List<Statement> block = parseSuite();
     Statement stmt = new ForStatement(ident, collection, block);
     list.add(setLocation(stmt, start, token.right));
   }
@@ -1129,8 +1126,7 @@ class Parser {
     List<Argument> args = parseFunctionDefArguments();
     expect(TokenKind.RPAREN);
     expect(TokenKind.COLON);
-    List<Statement> block = new ArrayList<>();
-    parseSuite(block);
+    List<Statement> block = parseSuite();
     FunctionDefStatement stmt = new FunctionDefStatement(ident, args, block);
     list.add(setLocation(stmt, start, token.right));
   }
@@ -1161,13 +1157,14 @@ class Parser {
 
   // suite ::= simple_stmt
   //         | NEWLINE INDENT stmt+ OUTDENT
-  private void parseSuite(List<Statement> list) {
+  private List<Statement> parseSuite() {
+    List<Statement> list = new ArrayList<>();
     if (token.kind == TokenKind.NEWLINE) {
       expect(TokenKind.NEWLINE);
       if (token.kind != TokenKind.INDENT) {
         reportError(lexer.createLocation(token.left, token.right),
                     "expected an indented block");
-        return;
+        return list;
       }
       expect(TokenKind.INDENT);
       while (token.kind != TokenKind.OUTDENT && token.kind != TokenKind.EOF) {
@@ -1179,6 +1176,7 @@ class Parser {
       list.add(stmt);
       expect(TokenKind.NEWLINE);
     }
+    return list;
   }
 
   // skipSuite does not check that the code is syntactically correct, it
@@ -1226,7 +1224,7 @@ class Parser {
       }
       parseFunctionDefStatement(list);
     } else if (token.kind == TokenKind.IF && skylarkMode) {
-      parseIfStatement(list);
+      list.add(parseIfStatement());
     } else if (token.kind == TokenKind.FOR && skylarkMode) {
       if (isTopLevel) {
         reportError(lexer.createLocation(token.left, token.right),
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
index 11f1877aad..8ab64cb3c1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkFunction.java
@@ -184,7 +184,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
       throw new EvalException(loc, String.format("expected %s for '%s' but got %s instead\n"
           + "%s.%s: %s",
           EvalUtils.getDataTypeNameFromClass(expectedType), paramName,
-          EvalUtils.getDatatypeName(realValue), functionName, paramName, paramDoc));
+          EvalUtils.getDataTypeName(realValue), functionName, paramName, paramDoc));
     }
     if (expectedType.equals(SkylarkList.class)) {
       checkGeneric(functionName, paramName, expectedType, expectedGenericType,
@@ -207,7 +207,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
           "expected %s of %ss for '%s' but got %s of %ss instead\n%s.%s: %s",
         mainType, EvalUtils.getDataTypeNameFromClass(expectedGenericType),
         paramName,
-        EvalUtils.getDatatypeName(realValue), EvalUtils.getDataTypeNameFromClass(realGenericType),
+        EvalUtils.getDataTypeName(realValue), EvalUtils.getDataTypeNameFromClass(realGenericType),
         functionName, paramName, paramDoc));
     }
   }
@@ -280,7 +280,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
               throw new IllegalArgumentException(String.format(
                   "expected %s type for '%s' but got %s instead",
                   EvalUtils.getDataTypeNameFromClass(type), what,
-                  EvalUtils.getDatatypeName(input)));
+                  EvalUtils.getDataTypeName(input)));
             }
           }
     });
@@ -303,7 +303,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
     if (!(obj instanceof Map<?, ?>)) {
       throw new IllegalArgumentException(String.format(
           "expected a dictionary for %s but got %s instead",
-          what, EvalUtils.getDatatypeName(obj)));
+          what, EvalUtils.getDataTypeName(obj)));
     }
     return Iterables.transform(((Map<?, ?>) obj).entrySet(),
         new com.google.common.base.Function<Map.Entry<?, ?>, Map.Entry<KEY_TYPE, VALUE_TYPE>>() {
@@ -320,8 +320,8 @@ public abstract class SkylarkFunction extends AbstractFunction {
             throw new IllegalArgumentException(String.format(
                 "expected <%s, %s> type for '%s' but got <%s, %s> instead",
                 keyType.getSimpleName(), valueType.getSimpleName(), what,
-                EvalUtils.getDatatypeName(input.getKey()),
-                EvalUtils.getDatatypeName(input.getValue())));
+                EvalUtils.getDataTypeName(input.getKey()),
+                EvalUtils.getDataTypeName(input.getValue())));
           }
         });
   }
@@ -334,7 +334,7 @@ public abstract class SkylarkFunction extends AbstractFunction {
       return type.cast(elem);
     } catch (ClassCastException e) {
       throw new EvalException(loc, String.format("expected %s for '%s' but got %s instead",
-          type.getSimpleName(), what, EvalUtils.getDatatypeName(elem)));
+          type.getSimpleName(), what, EvalUtils.getDataTypeName(elem)));
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
index a1b1e2675e..5efdd2eb58 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
@@ -93,7 +93,7 @@ public final class SkylarkNestedSet implements Iterable<Object> {
       }
     } else {
       throw new EvalException(loc,
-          String.format("cannot add '%s'-s to nested sets", EvalUtils.getDatatypeName(item)));
+          String.format("cannot add '%s'-s to nested sets", EvalUtils.getDataTypeName(item)));
     }
     this.genericType = Preconditions.checkNotNull(genericType, "type cannot be null");