Add a pretty printer for Skylark ASTs

This can be used to canonically compare ASTs for equality, e.g. in tests.

RELNOTES: None
PiperOrigin-RevId: 160283160

35 files changed, 549 insertions, 129 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java b/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
index d2e22d9dd7..18258a2d41 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ASTNode.java
@@ -16,8 +16,9 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.devtools.build.lib.events.Location;
-
+import java.io.IOException;
 import java.io.Serializable;
+import java.util.List;
 
 /**
  * Root class for nodes in the Abstract Syntax Tree of the Build language.
@@ -73,12 +74,83 @@ public abstract class ASTNode implements Serializable {
     return node;
   }
 
+  /** Number of spaces that each indentation level expands to when pretty-printing. */
+  public static final int INDENT_WIDTH = 2;
+
+  /** Writes out the indentation prefix for a line. */
+  protected void printIndent(Appendable buffer, int indentLevel) throws IOException {
+    for (int i = 0; i < indentLevel * INDENT_WIDTH; i++) {
+      buffer.append(' ');
+    }
+  }
+
   /**
-   * Print the syntax node in a form useful for debugging.  The output is not
-   * precisely specified, and should not be used by pretty-printing routines.
+   * Writes out a suite of statements. The statements are indented one more level than given, i.e.,
+   * the {@code indentLevel} parameter should be the same as the parent node's.
+   *
+   * <p>This also prints out a {@code pass} line if the suite is empty.
+   */
+  protected void printSuite(Appendable buffer, List<Statement> statements, int parentIndentLevel)
+      throws IOException {
+    if (statements.isEmpty()) {
+      printIndent(buffer, parentIndentLevel + 1);
+      buffer.append("pass\n");
+    } else {
+      for (Statement stmt : statements) {
+        stmt.prettyPrint(buffer, parentIndentLevel + 1);
+      }
+    }
+  }
+
+  /**
+   * Writes a pretty-printed representation of this node to a buffer, assuming the given starting
+   * indentation level.
+   *
+   * <p>For expressions, the indentation level is ignored. For statements, the indentation is
+   * written, then the statement contents (which may include multiple lines with their own
+   * indentation), then a newline character.
+   *
+   * <p>Indentation expands to {@code INDENT_WIDTH} many spaces per indent.
+   *
+   * <p>Pretty printing returns the canonical source code corresponding to an AST. Generally, the
+   * output can be round-tripped: Pretty printing an AST and then parsing the result should give you
+   * back an equivalent AST.
+   *
+   * <p>Pretty printing can also be used as a proxy for comparing for equality between two ASTs.
+   * This can be very useful in tests. However, it is still possible for two different trees to have
+   * the same pretty printing. In particular, {@link BuildFileAST} includes import metadata and
+   * comment information that is not reflected in the string.
+   */
+  public abstract void prettyPrint(Appendable buffer, int indentLevel) throws IOException;
+
+  /** Same as {@link #prettyPrint(Appendable, int)}, except with no indent. */
+  public void prettyPrint(Appendable buffer) throws IOException {
+    prettyPrint(buffer, 0);
+  }
+
+  /** Returns a pretty-printed representation of this node. */
+  public String prettyPrint() {
+    StringBuilder builder = new StringBuilder();
+    try {
+      prettyPrint(builder);
+    } catch (IOException e) {
+      // Not possible for StringBuilder.
+      throw new AssertionError(e);
+    }
+    return builder.toString();
+  }
+
+  /**
+   * Print the syntax node in a form useful for debugging.
+   *
+   * <p>The output is not precisely specified; use {@link #prettyPrint()} if you need more stable
+   * and complete information. For instance, this function may omit child statements of compound
+   * statements, or parentheses around some expressions. It may also abbreviate large list literals.
    */
   @Override
-  public abstract String toString();
+  public String toString() {
+    return prettyPrint();
+  }
 
   @Override
   public int hashCode() {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
index deb939ef61..12adf1a925 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.List;
@@ -58,7 +59,7 @@ public abstract class AbstractComprehension extends Expression {
      * <p>This avoids having to rely on reflection, or on checking whether {@link #getLValue} is
      * null.
      */
-    public abstract Kind getKind();
+    public Kind getKind();
 
     /**
      * The evaluation of the comprehension is based on recursion. Each clause may
@@ -72,23 +73,26 @@ public abstract class AbstractComprehension extends Expression {
      * @param collector the aggregated results of the comprehension.
      * @param step the index of the next clause to evaluate.
      */
-    abstract void eval(Environment env, OutputCollector collector, int step)
+    void eval(Environment env, OutputCollector collector, int step)
         throws EvalException, InterruptedException;
 
-    abstract void validate(ValidationEnvironment env, Location loc) throws EvalException;
+    void validate(ValidationEnvironment env, Location loc) throws EvalException;
 
     /**
      * The LValue defined in Clause, i.e. the loop variables for ForClause and null for
      * IfClause. This is needed for SyntaxTreeVisitor.
      */
     @Nullable  // for the IfClause
-    public abstract LValue getLValue();
+    public LValue getLValue();
 
     /**
      * The Expression defined in Clause, i.e. the collection for ForClause and the
      * condition for IfClause. This is needed for SyntaxTreeVisitor.
      */
-    public abstract Expression getExpression();
+    public Expression getExpression();
+
+    /** Pretty print to a buffer. */
+    public void prettyPrint(Appendable buffer) throws IOException;
   }
 
   /**
@@ -142,8 +146,23 @@ public abstract class AbstractComprehension extends Expression {
     }
 
     @Override
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append("for ");
+      variables.prettyPrint(buffer);
+      buffer.append(" in ");
+      list.prettyPrint(buffer);
+    }
+
+    @Override
     public String toString() {
-      return Printer.format("for %s in %r", variables.toString(), list);
+      StringBuilder builder = new StringBuilder();
+      try {
+        prettyPrint(builder);
+      } catch (IOException e) {
+        // Not possible for StringBuilder.
+        throw new AssertionError(e);
+      }
+      return builder.toString();
     }
   }
 
@@ -186,8 +205,21 @@ public abstract class AbstractComprehension extends Expression {
     }
 
     @Override
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append("if ");
+      condition.prettyPrint(buffer);
+    }
+
+    @Override
     public String toString() {
-      return String.format("if %s", condition);
+      StringBuilder builder = new StringBuilder();
+      try {
+        prettyPrint(builder);
+      } catch (IOException e) {
+        // Not possible for StringBuilder.
+        throw new AssertionError(e);
+      }
+      return builder.toString();
     }
   }
 
@@ -213,14 +245,14 @@ public abstract class AbstractComprehension extends Expression {
   }
 
   @Override
-  public String toString() {
-    StringBuilder sb = new StringBuilder();
-    sb.append(openingBracket()).append(printExpressions());
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append(openingBracket());
+    printExpressions(buffer);
     for (Clause clause : clauses) {
-      sb.append(' ').append(clause);
+      buffer.append(' ');
+      clause.prettyPrint(buffer);
     }
-    sb.append(closingBracket());
-    return sb.toString();
+    buffer.append(closingBracket());
   }
 
   /** Base class for comprehension builders. */
@@ -312,10 +344,8 @@ public abstract class AbstractComprehension extends Expression {
     }
   }
 
-  /**
-   * Returns a {@link String} representation of the output expression(s).
-   */
-  abstract String printExpressions();
+  /** Pretty-prints the output expression(s). */
+  protected abstract void printExpressions(Appendable buffer) throws IOException;
 
   abstract OutputCollector createCollector(Environment env);
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
index ce1b5452a2..12e8a7b5b1 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Argument.java
@@ -14,9 +14,8 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.util.Preconditions;
-
+import java.io.IOException;
 import java.util.List;
-
 import javax.annotation.Nullable;
 
 /**
@@ -54,15 +53,20 @@ public abstract class Argument extends ASTNode {
     public boolean isPositional() {
       return false;
     }
+
     public boolean isKeyword() {
       return false;
     }
-    @Nullable public String getName() { // only for keyword arguments
+
+    @Nullable
+    public String getName() { // only for keyword arguments
       return null;
     }
+
     public Expression getValue() {
       return value;
     }
+
     @Override
     public void accept(SyntaxTreeVisitor visitor) {
       visitor.visit(this);
@@ -76,12 +80,14 @@ public abstract class Argument extends ASTNode {
       super(value);
     }
 
-    @Override public boolean isPositional() {
+    @Override
+    public boolean isPositional() {
       return true;
     }
+
     @Override
-    public String toString() {
-      return String.valueOf(value);
+    public void prettyPrint(Appendable buffer) throws IOException {
+      value.prettyPrint(buffer);
     }
   }
 
@@ -95,15 +101,21 @@ public abstract class Argument extends ASTNode {
       this.name = name;
     }
 
-    @Override public String getName() {
+    @Override
+    public String getName() {
       return name;
     }
-    @Override public boolean isKeyword() {
+
+    @Override
+    public boolean isKeyword() {
       return true;
     }
+
     @Override
-    public String toString() {
-      return name + " = " + value;
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append(name);
+      buffer.append(" = ");
+      value.prettyPrint(buffer);
     }
   }
 
@@ -114,12 +126,15 @@ public abstract class Argument extends ASTNode {
       super(value);
     }
 
-    @Override public boolean isStar() {
+    @Override
+    public boolean isStar() {
       return true;
     }
+
     @Override
-    public String toString() {
-      return "*" + value;
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append('*');
+      value.prettyPrint(buffer);
     }
   }
 
@@ -130,12 +145,15 @@ public abstract class Argument extends ASTNode {
       super(value);
     }
 
-    @Override public boolean isStarStar() {
+    @Override
+    public boolean isStarStar() {
       return true;
     }
+
     @Override
-    public String toString() {
-      return "**" + value;
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append("**");
+      value.prettyPrint(buffer);
     }
   }
 
@@ -179,4 +197,12 @@ public abstract class Argument extends ASTNode {
       }
     }
   }
+
+  @Override
+  public final void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    prettyPrint(buffer);
+  }
+
+  @Override
+  public abstract void prettyPrint(Appendable buffer) throws IOException;
 }
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 e41e78b6d0..fc2c5fd64a 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
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
 
 /**
  * Syntax node for an assignment statement.
@@ -47,8 +48,12 @@ public final class AssignmentStatement extends Statement {
   }
 
   @Override
-  public String toString() {
-    return lvalue + " = " + expression + '\n';
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    lvalue.prettyPrint(buffer, indentLevel);
+    buffer.append(" = ");
+    expression.prettyPrint(buffer, indentLevel);
+    buffer.append('\n');
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
index 5b910ce777..cfa2c8fce2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
+
 /** Syntax node for an augmented assignment statement. */
 public final class AugmentedAssignmentStatement extends Statement {
 
@@ -46,8 +48,14 @@ public final class AugmentedAssignmentStatement extends Statement {
   }
 
   @Override
-  public String toString() {
-    return String.format("%s %s= %s\n", lvalue, operator, expression);
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    lvalue.prettyPrint(buffer);
+    buffer.append(' ');
+    buffer.append(operator.toString());
+    buffer.append("= ");
+    expression.prettyPrint(buffer);
+    buffer.append('\n');
   }
 
   @Override
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 de3bf92c2b..ba868188dd 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
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.Concatable.Concatter;
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
+import java.io.IOException;
 import java.util.Collections;
 import java.util.IllegalFormatException;
 
@@ -55,7 +56,22 @@ public final class BinaryOperatorExpression extends Expression {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer) throws IOException {
+    // TODO(bazel-team): Possibly omit parentheses when they are not needed according to operator
+    // precedence rules. This requires passing down more contextual information.
+    buffer.append('(');
+    lhs.prettyPrint(buffer);
+    buffer.append(' ');
+    buffer.append(operator.toString());
+    buffer.append(' ');
+    rhs.prettyPrint(buffer);
+    buffer.append(')');
+  }
+
+  @Override
   public String toString() {
+    // This omits the parentheses for brevity, but is not correct in general due to operator
+    // precedence rules.
     return lhs + " " + operator + " " + rhs;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index d28912f1a7..a0b7464d31 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -247,8 +247,16 @@ public class BuildFileAST extends ASTNode {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    // Only statements are printed, not comments and processed import data.
+    for (Statement stmt : stmts) {
+      stmt.prettyPrint(buffer, indentLevel);
+    }
+  }
+
+  @Override
   public String toString() {
-    return "BuildFileAST" + getStatements();
+    return "<BuildFileAST with " + stmts.size() + " statements>";
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Comment.java b/src/main/java/com/google/devtools/build/lib/syntax/Comment.java
index 27304eba87..b0b69c213a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Comment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Comment.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
+
 /**
  * Syntax node for comments.
  */
@@ -34,6 +36,16 @@ public final class Comment extends ASTNode {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    // We can't really print comments in the right place anyway, due to how their relative order
+    // is lost in the representation of BuildFileAST. So don't bother word-wrapping and just print
+    // it on a single line.
+    printIndent(buffer, indentLevel);
+    buffer.append("# ");
+    buffer.append(value);
+  }
+
+  @Override
   public String toString() {
     return value;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
index a891022238..4abc12ade7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ConditionalExpression.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
 
 /**
  * Syntax node for an if/else expression.
@@ -43,8 +44,12 @@ public final class ConditionalExpression extends Expression {
    * Constructs a string representation of the if expression
    */
   @Override
-  public String toString() {
-    return thenCase + " if " + condition + " else " + elseCase;
+  public void prettyPrint(Appendable buffer) throws IOException {
+    thenCase.prettyPrint(buffer);
+    buffer.append(" if ");
+    condition.prettyPrint(buffer);
+    buffer.append(" else ");
+    elseCase.prettyPrint(buffer);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
index be9191a2fe..fca62c3193 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
@@ -13,9 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
-
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 
@@ -52,8 +52,10 @@ public final class DictComprehension extends AbstractComprehension {
   }
 
   @Override
-  String printExpressions() {
-    return String.format("%s: %s", keyExpression, valueExpression);
+  protected void printExpressions(Appendable buffer) throws IOException {
+    keyExpression.prettyPrint(buffer);
+    buffer.append(": ");
+    valueExpression.prettyPrint(buffer);
   }
 
   /** Builder for {@link DictComprehension}. */
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 c2328b190c..56a62ac2de 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
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 import java.util.List;
 
 /**
@@ -42,12 +43,10 @@ public final class DictionaryLiteral extends Expression {
     }
 
     @Override
-    public String toString() {
-      StringBuilder sb = new StringBuilder();
-      sb.append(key);
-      sb.append(": ");
-      sb.append(value);
-      return sb.toString();
+    public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+      key.prettyPrint(buffer);
+      buffer.append(": ");
+      value.prettyPrint(buffer);
     }
 
     @Override
@@ -84,17 +83,15 @@ public final class DictionaryLiteral extends Expression {
   }
 
   @Override
-  public String toString() {
-    StringBuilder sb = new StringBuilder();
-    sb.append("{");
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append("{");
     String sep = "";
     for (DictionaryEntryLiteral e : entries) {
-      sb.append(sep);
-      sb.append(e);
+      buffer.append(sep);
+      e.prettyPrint(buffer);
       sep = ", ";
     }
-    sb.append("}");
-    return sb.toString();
+    buffer.append("}");
   }
 
   @Override
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 97588c0c9a..4672af11c0 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
@@ -18,6 +18,7 @@ import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.FuncallExpression.MethodDescriptor;
 import com.google.devtools.build.lib.util.SpellChecker;
+import java.io.IOException;
 
 /** Syntax node for a dot expression. e.g. obj.field, but not obj.method() */
 public final class DotExpression extends Expression {
@@ -40,8 +41,10 @@ public final class DotExpression extends Expression {
   }
 
   @Override
-  public String toString() {
-    return obj + "." + field;
+  public void prettyPrint(Appendable buffer) throws IOException {
+    obj.prettyPrint(buffer);
+    buffer.append('.');
+    field.prettyPrint(buffer);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index dcfefbd103..45abea50d2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -942,7 +942,8 @@ public final class Environment implements Freezable {
 
 
   /**
-   * The fail fast handler, which throws a AssertionError whenever an error or warning occurs.
+   * The fail fast handler, which throws an {@link IllegalArgumentException} whenever an error or
+   * warning occurs.
    */
   public static final EventHandler FAIL_FAST_HANDLER = new EventHandler() {
       @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
index d5e8e8d16e..c92eebbd0f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
+
 /**
  * Base class for all expression nodes in the AST.
  */
@@ -66,4 +68,16 @@ public abstract class Expression extends ASTNode {
    * @see Statement
    */
   abstract void validate(ValidationEnvironment env) throws EvalException;
+
+  @Override
+  public final void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    prettyPrint(buffer);
+  }
+
+  /**
+   * Expressions should implement this method instead of {@link #prettyPrint(Appendable, int)},
+   * since the {@code indentLevel} argument is not needed.
+   */
+  @Override
+  public abstract void prettyPrint(Appendable buffer) throws IOException;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
index 34df3fc9b3..b6fc4c822b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
 
 /**
  * Syntax node for a function call statement. Used for build rules.
@@ -31,8 +32,10 @@ public final class ExpressionStatement extends Statement {
   }
 
   @Override
-  public String toString() {
-    return expr.toString() + '\n';
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    expr.prettyPrint(buffer);
+    buffer.append('\n');
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
index 63ce6cb818..a1639c95eb 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
 
 /**
  * A class for flow statements (e.g. break and continue)
@@ -54,8 +55,15 @@ public final class FlowStatement extends Statement {
   void validate(ValidationEnvironment env) throws EvalException {}
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append(kind.name);
+    buffer.append('\n');
+  }
+
+  @Override
   public String toString() {
-    return kind.name;
+    return kind.name + "\n";
   }
 
   @Override
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 a43d2894dd..8ac89bdd38 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
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.syntax.FlowStatement.FlowException;
 import com.google.devtools.build.lib.util.Preconditions;
+import java.io.IOException;
 import java.util.List;
 
 /**
@@ -52,9 +53,18 @@ public final class ForStatement extends Statement {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("for ");
+    variable.prettyPrint(buffer);
+    buffer.append(" in ");
+    collection.prettyPrint(buffer);
+    buffer.append(":\n");
+    printSuite(buffer, block, indentLevel);
+  }
+
+  @Override
   public String toString() {
-    // TODO(bazel-team): if we want to print the complete statement, the function
-    // needs an extra argument to specify indentation level.
     return "for " + variable + " in " + collection + ": ...\n";
   }
 
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 2c7a2c888f..63120932a0 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
@@ -32,6 +32,7 @@ import com.google.devtools.build.lib.syntax.Runtime.NoneType;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.StringUtilities;
+import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
@@ -249,6 +250,23 @@ public final class FuncallExpression extends Expression {
     return numPositionalArgs;
   }
 
+   @Override
+   public void prettyPrint(Appendable buffer) throws IOException {
+     if (obj != null) {
+       obj.prettyPrint(buffer);
+       buffer.append('.');
+     }
+     func.prettyPrint(buffer);
+     buffer.append('(');
+     String sep = "";
+     for (Argument.Passed arg : args) {
+       buffer.append(sep);
+       arg.prettyPrint(buffer);
+       sep = ", ";
+     }
+     buffer.append(')');
+   }
+
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
index e6ef1bab6d..881368bf15 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -69,8 +70,24 @@ public final class FunctionDefStatement extends Statement {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("def ");
+    ident.prettyPrint(buffer);
+    buffer.append('(');
+    String sep = "";
+    for (Parameter<?, ?> param : parameters) {
+      buffer.append(sep);
+      param.prettyPrint(buffer);
+      sep = ", ";
+    }
+    buffer.append("):\n");
+    printSuite(buffer, statements, indentLevel);
+  }
+
+  @Override
   public String toString() {
-    return "def " + ident + "(" + signature + "):\n";
+    return "def " + ident + "(" + signature + "): ...\n";
   }
 
   public Identifier getIdent() {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index ada928ba48..a404bfa79f 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.util.SpellChecker;
+import java.io.IOException;
 import java.util.Set;
 import javax.annotation.Nullable;
 
@@ -52,8 +53,8 @@ public final class Identifier extends Expression {
   }
 
   @Override
-  public String toString() {
-    return name;
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append(name);
   }
 
   @Override
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 b6adc6067b..2c0915bffd 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
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.util.Preconditions;
+import java.io.IOException;
 import java.util.List;
 
 /**
@@ -42,6 +43,12 @@ public final class IfStatement extends Statement {
       }
     }
 
+    // No prettyPrint function; handled directly by IfStatement#prettyPrint.
+    @Override
+    public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+      throw new UnsupportedOperationException("Cannot pretty print ConditionalStatements node");
+    }
+
     @Override
     public String toString() {
       return "[el]if " + condition + ": " + stmts + "\n";
@@ -67,6 +74,7 @@ public final class IfStatement extends Statement {
     }
   }
 
+  /** "if" or "elif" clauses. Must be non-empty. */
   private final ImmutableList<ConditionalStatements> thenBlocks;
   private final ImmutableList<Statement> elseBlock;
 
@@ -89,11 +97,26 @@ public final class IfStatement extends Statement {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    String clauseWord = "if ";
+    for (ConditionalStatements condStmt : thenBlocks) {
+      printIndent(buffer, indentLevel);
+      buffer.append(clauseWord);
+      condStmt.getCondition().prettyPrint(buffer);
+      buffer.append(":\n");
+      printSuite(buffer, condStmt.getStmts(), indentLevel);
+      clauseWord = "elif ";
+    }
+    if (!elseBlock.isEmpty()) {
+      printIndent(buffer, indentLevel);
+      buffer.append("else:\n");
+      printSuite(buffer, elseBlock, indentLevel);
+    }
+  }
+
+  @Override
   public String toString() {
-    // TODO(bazel-team): if we want to print the complete statement, the function
-    // needs an extra argument to specify indentation level.
-    // As guaranteed by the constructor, there must be at least one element in thenBlocks.
-    return String.format("if %s:\n", thenBlocks.get(0).getCondition());
+    return String.format("if %s: ...\n", thenBlocks.get(0).getCondition());
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
index 5a3b81378e..6d46e710d8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 
 /** Syntax node for an index expression. e.g. obj[field], but not obj[from:to] */
 public final class IndexExpression extends Expression {
@@ -36,8 +37,11 @@ public final class IndexExpression extends Expression {
   }
 
   @Override
-  public String toString() {
-    return String.format("%s[%s]", obj, key);
+  public void prettyPrint(Appendable buffer) throws IOException {
+    obj.prettyPrint(buffer);
+    buffer.append('[');
+    key.prettyPrint(buffer);
+    buffer.append(']');
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
index be9ce536e3..14827fe2a7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/IntegerLiteral.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
+
 /**
  * Syntax node for an integer literal.
  */
@@ -23,6 +25,11 @@ public final class IntegerLiteral extends Literal<Integer> {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append(value.toString());
+  }
+
+  @Override
   public void accept(SyntaxTreeVisitor visitor) {
     visitor.visit(this);
   }
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 72c9ac2b7b..31918ce792 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
@@ -17,17 +17,30 @@ package com.google.devtools.build.lib.syntax;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.util.Preconditions;
+import java.io.IOException;
 import java.util.Collection;
 
 /**
- * Class representing an LValue.
- * It appears in assignment, for loop and comprehensions, e.g.
- *    lvalue = 2
- *    [for lvalue in exp]
- *    for lvalue in exp: pass
- * An LValue can be a simple variable or something more complex like a tuple.
+ * A term that can appear on the left-hand side of an assignment statement, for loop, comprehension
+ * clause, etc. E.g.,
+ * <ul>
+ *   <li>{@code lvalue = 2}
+ *   <li>{@code [for lvalue in exp]}
+ *   <li>{@code for lvalue in exp: pass}
+ * </ul>
+ *
+ * <p>An {@code LValue}'s expression must have one of the following forms:
+ * <ul>
+ *   <li>(Variable assignment) an {@link Identifier};
+ *   <li>(Sequence assignment) a {@link ListLiteral} (either list or tuple) of expressions that can
+ *       themselves appear in an {@code LValue}; or
+ *   <li>(List or dictionary item assignment) an {@link IndexExpression}.
+ * </ul>
+ * In particular and unlike Python, slice expressions, dot expressions, and starred expressions
+ * cannot appear in LValues.
  */
-public class LValue extends ASTNode {
+public final class LValue extends ASTNode {
+
   private final Expression expr;
 
   public LValue(Expression expr) {
@@ -211,7 +224,7 @@ public class LValue extends ASTNode {
   }
 
   @Override
-  public String toString() {
-    return expr.toString();
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    expr.prettyPrint(buffer);
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
index f5cef330ad..3a457fb760 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
@@ -16,6 +16,7 @@ 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.syntax.SkylarkList.MutableList;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -45,8 +46,8 @@ public final class ListComprehension extends AbstractComprehension {
   }
 
   @Override
-  String printExpressions() {
-    return outputExpression.toString();
+  protected void printExpressions(Appendable buffer) throws IOException {
+    outputExpression.prettyPrint(buffer);
   }
 
   /** Builder for {@link ListComprehension}. */
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 6f5ece7715..fbf145bd42 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
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
 import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -71,6 +72,21 @@ public final class ListLiteral extends Expression {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append(isTuple() ? '(' : '[');
+    String sep = "";
+    for (Expression e : exprs) {
+      buffer.append(sep);
+      e.prettyPrint(buffer);
+      sep = ", ";
+    }
+    if (isTuple() && exprs.size() == 1) {
+      buffer.append(',');
+    }
+    buffer.append(isTuple() ? ')' : ']');
+  }
+
+  @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
     Printer.printList(sb, exprs, isTuple(), '"', Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Literal.java b/src/main/java/com/google/devtools/build/lib/syntax/Literal.java
index d6842cbadf..12eec77e35 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Literal.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Literal.java
@@ -33,11 +33,6 @@ public abstract class Literal<T> extends Expression {
   }
 
   @Override
-  public String toString() {
-    return value.toString();
-  }
-
-  @Override
   Object doEval(Environment env) {
     return value;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
index a7fc147319..d75f21e2cc 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java
@@ -13,9 +13,9 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
-import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import java.io.IOException;
 import java.util.Map;
 
 /**
@@ -53,9 +53,25 @@ public final class LoadStatement extends Statement {
   }
 
   @Override
-  public String toString() {
-    return String.format(
-        "load(\"%s\", %s)", imp.getValue(), Joiner.on(", ").join(cachedSymbols));
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("load(");
+    imp.prettyPrint(buffer);
+    for (Identifier symbol : cachedSymbols) {
+      buffer.append(", ");
+      String origName = symbolMap.get(symbol);
+      if (origName.equals(symbol.getName())) {
+        buffer.append('"');
+        symbol.prettyPrint(buffer);
+        buffer.append('"');
+      } else {
+        symbol.prettyPrint(buffer);
+        buffer.append("=\"");
+        buffer.append(origName);
+        buffer.append('"');
+      }
+    }
+    buffer.append(")\n");
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
index 1b74dcde5a..5e16ea3f4e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parameter.java
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
 import javax.annotation.Nullable;
 
 /**
@@ -44,27 +45,37 @@ public abstract class Parameter<V, T> extends Argument {
   public boolean isMandatory() {
     return false;
   }
+
   public boolean isOptional() {
     return false;
   }
+
   @Override
   public boolean isStar() {
     return false;
   }
+
   @Override
   public boolean isStarStar() {
     return false;
   }
-  @Nullable public String getName() {
+
+  @Nullable
+  public String getName() {
     return name;
   }
+
   public boolean hasName() {
     return true;
   }
-  @Nullable public T getType() {
+
+  @Nullable
+  public T getType() {
     return type;
   }
-  @Nullable public V getDefaultValue() {
+
+  @Nullable
+  public V getDefaultValue() {
     return null;
   }
 
@@ -79,18 +90,20 @@ public abstract class Parameter<V, T> extends Argument {
       super(name, type);
     }
 
-    @Override public boolean isMandatory() {
+    @Override
+    public boolean isMandatory() {
       return true;
     }
 
     @Override
-    public String toString() {
-      return name;
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append(name);
     }
   }
 
   /** optional parameter (positional or key-only depending on position): Ident = Value */
   public static final class Optional<V, T> extends Parameter<V, T> {
+
     public final V defaultValue;
 
     public Optional(String name, @Nullable V defaultValue) {
@@ -103,15 +116,29 @@ public abstract class Parameter<V, T> extends Argument {
       this.defaultValue = defaultValue;
     }
 
-    @Override @Nullable public V getDefaultValue() {
+    @Override
+    @Nullable
+    public V getDefaultValue() {
       return defaultValue;
     }
 
-    @Override public boolean isOptional() {
+    @Override
+    public boolean isOptional() {
       return true;
     }
 
     @Override
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append(name);
+      buffer.append('=');
+      // This should only ever be used on a parameter representing static information, i.e. with V
+      // and T instantiated as Expression.
+      ((Expression) defaultValue).prettyPrint(buffer);
+    }
+
+    // Keep this as a separate method so that it can be used regardless of what V and T are
+    // parameterized with.
+    @Override
     public String toString() {
       return name + "=" + defaultValue;
     }
@@ -119,6 +146,7 @@ public abstract class Parameter<V, T> extends Argument {
 
   /** extra positionals parameter (star): *identifier */
   public static final class Star<V, T> extends Parameter<V, T> {
+
     public Star(@Nullable String name, @Nullable T type) {
       super(name, type);
     }
@@ -132,21 +160,23 @@ public abstract class Parameter<V, T> extends Argument {
       return name != null;
     }
 
-    @Override public boolean isStar() {
+    @Override
+    public boolean isStar() {
       return true;
     }
 
-    @Override public String toString() {
-      if (name == null) {
-        return "*";
-      } else {
-        return "*" + name;
+    @Override
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append('*');
+      if (name != null) {
+        buffer.append(name);
       }
     }
   }
 
   /** extra keywords parameter (star_star): **identifier */
   public static final class StarStar<V, T> extends Parameter<V, T> {
+
     public StarStar(String name, @Nullable T type) {
       super(name, type);
     }
@@ -155,13 +185,15 @@ public abstract class Parameter<V, T> extends Argument {
       super(name);
     }
 
-    @Override public boolean isStarStar() {
+    @Override
+    public boolean isStarStar() {
       return true;
     }
 
     @Override
-    public String toString() {
-      return "**" + name;
+    public void prettyPrint(Appendable buffer) throws IOException {
+      buffer.append("**");
+      buffer.append(name);
     }
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index 8e04d43494..fde22dc946 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -604,6 +604,13 @@ public final class Printer {
      * <p>If the input string was already shortened and contains "<x more arguments>", this part
      * will also be appended.
      */
+    // TODO(bazel-team): Given an input list
+    //
+    //     [1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]
+    //
+    // the inner list gets doubly mangled as
+    //
+    //     [1, 2, 3, [10, 20, 30, 40, <2 more argu...<2 more arguments>], <3 more arguments>]
     private void appendTrailingSpecialChars(CharSequence csq, int limit) throws IOException {
       int length = csq.length();
       Matcher matcher = ARGS_PATTERN.matcher(csq);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
index 45889a4287..3c8ac34d9b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 
 /**
  * A wrapper Statement class for return expressions.
@@ -61,8 +62,16 @@ public final class ReturnStatement extends Statement {
   }
 
   @Override
-  public String toString() {
-    return "return " + returnExpression;
+  public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {
+    printIndent(buffer, indentLevel);
+    buffer.append("return");
+    // "return" with no arg is represented internally as returning the None identifier.
+    if (!(returnExpression instanceof Identifier
+          && ((Identifier) returnExpression).getName().equals("None"))) {
+      buffer.append(' ');
+      returnExpression.prettyPrint(buffer, indentLevel);
+    }
+    buffer.append('\n');
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
index b2ddcc0ff3..9d28ee8b19 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkImports.java
@@ -24,7 +24,11 @@ import java.util.Objects;
 /**
  * Factory class for creating appropriate instances of {@link SkylarkImports}.
  */
-public abstract class SkylarkImports {
+public class SkylarkImports {
+
+  private SkylarkImports() {
+    throw new IllegalStateException("This class should not be instantiated");
+  }
 
   // Default implementation class for SkylarkImport.
   private abstract static class SkylarkImportImpl implements SkylarkImport {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
index 14988f0f7a..f3ef2779b3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SliceExpression.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
+import java.io.IOException;
 import java.util.List;
 
 /** Syntax node for an index expression. e.g. obj[field], but not obj[from:to] */
@@ -48,16 +49,34 @@ public final class SliceExpression extends Expression {
   }
 
   @Override
-  public String toString() {
-    return String.format("%s[%s:%s%s]",
-        obj,
-        start,
-        // Omit `end` if it's a literal `None` (default value)
-        ((end instanceof Identifier) && (((Identifier) end).getName().equals("None"))) ? "" : end,
-        // Omit `step` if it's an integer literal `1` (default value)
-        ((step instanceof IntegerLiteral) && (((IntegerLiteral) step).value.equals(1)))
-            ? "" : ":" + step
-    );
+  public void prettyPrint(Appendable buffer) throws IOException {
+    boolean startIsDefault =
+        (start instanceof Identifier) && ((Identifier) start).getName().equals("None");
+    boolean endIsDefault =
+        (end instanceof Identifier) && ((Identifier) end).getName().equals("None");
+    boolean stepIsDefault =
+        (step instanceof IntegerLiteral) && ((IntegerLiteral) step).getValue().equals(1);
+
+    obj.prettyPrint(buffer);
+    buffer.append('[');
+    // Start and end are omitted if they are the literal identifier None, which is the default value
+    // inserted by the parser if no bound is given. Likewise, step is omitted if it is the literal
+    // integer 1.
+    //
+    // The first separator colon is unconditional. The second separator appears only if step is
+    // printed.
+    if (!startIsDefault) {
+      start.prettyPrint(buffer);
+    }
+    buffer.append(':');
+    if (!endIsDefault) {
+      end.prettyPrint(buffer);
+    }
+    if (!stepIsDefault) {
+      buffer.append(':');
+      step.prettyPrint(buffer);
+    }
+    buffer.append(']');
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
index 640d007454..df559fe7fa 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StringLiteral.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.syntax;
 
+import java.io.IOException;
+
 /**
  * Syntax node for a string literal.
  */
@@ -23,8 +25,8 @@ public final class StringLiteral extends Literal<String> {
   }
 
   @Override
-  public String toString() {
-    return Printer.repr(value);
+  public void prettyPrint(Appendable buffer) throws IOException {
+    buffer.append(Printer.repr(value));
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
index 5c36be48f9..b80818294d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
@@ -15,6 +15,8 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.devtools.build.lib.events.Location;
 
+import java.io.IOException;
+
 /** Syntax node for a unary operator expression. */
 public final class UnaryOperatorExpression extends Expression {
 
@@ -36,9 +38,23 @@ public final class UnaryOperatorExpression extends Expression {
   }
 
   @Override
+  public void prettyPrint(Appendable buffer) throws IOException {
+    // TODO(bazel-team): Possibly omit parentheses when they are not needed according to operator
+    // precedence rules. This requires passing down more contextual information.
+    buffer.append(operator.toString());
+    buffer.append('(');
+    operand.prettyPrint(buffer);
+    buffer.append(')');
+  }
+
+  @Override
   public String toString() {
     // All current and planned unary operators happen to be prefix operators.
     // Non-symbolic operators have trailing whitespace built into their name.
+    //
+    // Note that this omits the parentheses for brevity, but is not correct in general due to
+    // operator precedence rules. For example, "(not False) in mylist" prints as
+    // "not False in mylist", which evaluates to opposite results in the case that mylist is empty.
     return operator.toString() + operand;
   }