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
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 d2e22d9..18258a2 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 @@ 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 @@ 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 deb939e..12adf1a 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 @@ 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 @@ * <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 @@ * @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 @@ } @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 @@ } @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 @@ } @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 @@ } } - /** - * 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 ce1b545..12e8a7b 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 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ } } } + + @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 e41e78b..fc2c5fd 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 @@ } @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 5b910ce..cfa2c8f 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 @@ } @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 de3bf92..ba86818 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.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 @@ } @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 d28912f..a0b7464 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 @@ } @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 27304eb..b0b69c2 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 @@ } @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 a891022..4abc12a 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 @@ * 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 be9191a..fca62c3 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 @@ } @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 c2328b1..56a62ac 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 @@ 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 @@ } @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 @@ } @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 97588c0..4672af1 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.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 @@ } @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 dcfefbd..45abea5 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 @@ /** - * 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 d5e8e8d..c92eebb 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 @@ * @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 34df3fc..b6fc4c8 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 @@ } @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 63ce6cb..a1639c9 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 @@ 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 a43d289..8ac89bd 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 @@ 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 @@ } @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 2c7a2c8..6312093 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.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 @@ 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 e6ef1ba..881368b 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 @@ } @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 ada928b..a404bfa 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 @@ } @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 b6adc60..2c0915b 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 @@ 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 @@ } } + // 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 @@ } } + /** "if" or "elif" clauses. Must be non-empty. */ private final ImmutableList<ConditionalStatements> thenBlocks; private final ImmutableList<Statement> elseBlock; @@ -89,11 +97,26 @@ } @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 5a3b813..6d46e71 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 @@ } @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 be9ce53..14827fe 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 @@ } @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 72c9ac2..31918ce 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 @@ 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 @@ } @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 f5cef33..3a457fb 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 @@ 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 @@ } @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 6f5ece7..fbf145b 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 @@ 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 @@ } @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 d6842cb..12eec77 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 @@ } @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 a7fc147..d75f21e 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 @@ } @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 1b74dcd..5e16ea3 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 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 @@ 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 @@ 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 @@ /** 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 @@ 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 @@ 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 8e04d43..fde22dc 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 @@ * <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 45889a4..3c8ac34 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 @@ } @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 b2ddcc0..9d28ee8 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 @@ /** * 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 14988f0..f3ef277 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 @@ } @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 640d007..df559fe 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 @@ } @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 5c36be4..b808182 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 @@ 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 @@ } @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; }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ASTNodeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ASTNodeTest.java index 877bbe5..651c1f1 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/ASTNodeTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/ASTNodeTest.java
@@ -15,6 +15,7 @@ import static org.junit.Assert.fail; +import java.io.IOException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,6 +33,8 @@ public final void createNode() throws Exception { node = new ASTNode() { @Override + public void prettyPrint(Appendable buffer, int indentLevel) throws IOException {} + @Override public String toString() { return null; }
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java new file mode 100644 index 0000000..7c7b02a --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
@@ -0,0 +1,432 @@ +// Copyright 2017 The Bazel Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.syntax; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.syntax.Parser.ParsingLevel.LOCAL_LEVEL; +import static com.google.devtools.build.lib.syntax.Parser.ParsingLevel.TOP_LEVEL; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.syntax.util.EvaluationTestCase; +import java.io.IOException; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests the {@code toString} and pretty printing methods for {@link ASTNode} subclasses. */ +@RunWith(JUnit4.class) +public class ASTPrettyPrintTest extends EvaluationTestCase { + + private String join(String... lines) { + return Joiner.on("\n").join(lines); + } + + /** + * Asserts that the given node's pretty print at a given indent level matches the given string. + */ + private void assertPrettyMatches(ASTNode node, int indentLevel, String expected) { + StringBuilder prettyBuilder = new StringBuilder(); + try { + node.prettyPrint(prettyBuilder, indentLevel); + } catch (IOException e) { + // Impossible for StringBuilder. + throw new AssertionError(e); + } + assertThat(prettyBuilder.toString()).isEqualTo(expected); + } + + /** Asserts that the given node's pretty print with no indent matches the given string. */ + private void assertPrettyMatches(ASTNode node, String expected) { + assertPrettyMatches(node, 0, expected); + } + + /** Asserts that the given node's pretty print with one indent matches the given string. */ + private void assertIndentedPrettyMatches(ASTNode node, String expected) { + assertPrettyMatches(node, 1, expected); + } + + /** Asserts that the given node's {@code toString} matches the given string. */ + private void assertTostringMatches(ASTNode node, String expected) { + assertThat(node.toString()).isEqualTo(expected); + } + + /** + * Parses the given string as an expression, and asserts that its pretty print matches the given + * string. + */ + private void assertExprPrettyMatches(String source, String expected) { + Expression node = parseExpression(source); + assertPrettyMatches(node, expected); + } + + /** + * Parses the given string as an expression, and asserts that its {@code toString} matches the + * given string. + */ + private void assertExprTostringMatches(String source, String expected) { + Expression node = parseExpression(source); + assertThat(node.toString()).isEqualTo(expected); + } + + /** + * Parses the given string as an expression, and asserts that both its pretty print and {@code + * toString} return the original string. + */ + private void assertExprBothRoundTrip(String source) { + assertExprPrettyMatches(source, source); + assertExprTostringMatches(source, source); + } + + /** + * Parses the given string as a statement, and asserts that its pretty print with one indent + * matches the given string. + */ + private void assertStmtIndentedPrettyMatches( + Parser.ParsingLevel parsingLevel, String source, String expected) { + Statement node = parseStatement(parsingLevel, source); + assertIndentedPrettyMatches(node, expected); + } + + /** + * Parses the given string as an statement, and asserts that its {@code toString} matches the + * given string. + */ + private void assertStmtTostringMatches( + Parser.ParsingLevel parsingLevel, String source, String expected) { + Statement node = parseStatement(parsingLevel, source); + assertThat(node.toString()).isEqualTo(expected); + } + + // Expressions. + + @Test + public void abstractComprehension() { + // Covers DictComprehension and ListComprehension. + assertExprBothRoundTrip("[z for y in x if True for z in y]"); + assertExprBothRoundTrip("{z: x for y in x if True for z in y}"); + } + + @Test + public void binaryOperatorExpression() { + assertExprPrettyMatches("1 + 2", "(1 + 2)"); + assertExprTostringMatches("1 + 2", "1 + 2"); + + assertExprPrettyMatches("1 + (2 * 3)", "(1 + (2 * 3))"); + assertExprTostringMatches("1 + (2 * 3)", "1 + 2 * 3"); + } + + @Test + public void conditionalExpression() { + assertExprBothRoundTrip("1 if True else 2"); + } + + @Test + public void dictionaryLiteral() { + assertExprBothRoundTrip("{1: \"a\", 2: \"b\"}"); + } + + @Test + public void dotExpression() { + assertExprBothRoundTrip("o.f"); + } + + @Test + public void funcallExpression() { + assertExprBothRoundTrip("f()"); + assertExprBothRoundTrip("f(a)"); + assertExprBothRoundTrip("f(a, b = B, *c, d = D, **e)"); + assertExprBothRoundTrip("o.f()"); + } + + @Test + public void identifier() { + assertExprBothRoundTrip("foo"); + } + + @Test + public void indexExpression() { + assertExprBothRoundTrip("a[i]"); + } + + @Test + public void integerLiteral() { + assertExprBothRoundTrip("5"); + } + + @Test + public void listLiteralShort() { + assertExprBothRoundTrip("[]"); + assertExprBothRoundTrip("[5]"); + assertExprBothRoundTrip("[5, 6]"); + assertExprBothRoundTrip("()"); + assertExprBothRoundTrip("(5,)"); + assertExprBothRoundTrip("(5, 6)"); + } + + @Test + public void listLiteralLong() { + // List literals with enough elements to trigger the abbreviated toString() format. + assertExprPrettyMatches("[1, 2, 3, 4, 5, 6]", "[1, 2, 3, 4, 5, 6]"); + assertExprTostringMatches("[1, 2, 3, 4, 5, 6]", "[1, 2, 3, 4, <2 more arguments>]"); + + assertExprPrettyMatches("(1, 2, 3, 4, 5, 6)", "(1, 2, 3, 4, 5, 6)"); + assertExprTostringMatches("(1, 2, 3, 4, 5, 6)", "(1, 2, 3, 4, <2 more arguments>)"); + } + + @Test + public void listLiteralNested() { + // Make sure that the inner list doesn't get abbreviated when the outer list is printed using + // prettyPrint(). + assertExprPrettyMatches( + "[1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]", + "[1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]"); + // It doesn't matter as much what toString does. This case demonstrates an apparent bug in how + // Printer#printList abbreviates the nested contents. We can keep this test around to help + // monitor changes in the buggy behavior or eventually fix it. + assertExprTostringMatches( + "[1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]", + "[1, 2, 3, [10, 20, 30, 40, <2 more argu...<2 more arguments>], <3 more arguments>]"); + } + + @Test + public void sliceExpression() { + assertExprBothRoundTrip("a[b:c:d]"); + assertExprBothRoundTrip("a[b:c]"); + assertExprBothRoundTrip("a[b:]"); + assertExprBothRoundTrip("a[:c:d]"); + assertExprBothRoundTrip("a[:c]"); + assertExprBothRoundTrip("a[::d]"); + assertExprBothRoundTrip("a[:]"); + } + + @Test + public void stringLiteral() { + assertExprBothRoundTrip("\"foo\""); + assertExprBothRoundTrip("\"quo\\\"ted\""); + } + + @Test + public void unaryOperatorExpression() { + assertExprPrettyMatches("not True", "not (True)"); + assertExprTostringMatches("not True", "not True"); + assertExprPrettyMatches("-5", "-(5)"); + assertExprTostringMatches("-5", "-5"); + } + + // Statements. + + @Test + public void assignmentStatement() { + assertStmtIndentedPrettyMatches(LOCAL_LEVEL, "x = y", " x = y\n"); + assertStmtTostringMatches(LOCAL_LEVEL, "x = y", "x = y\n"); + } + + @Test + public void augmentedAssignmentStatement() { + assertStmtIndentedPrettyMatches(LOCAL_LEVEL, "x += y", " x += y\n"); + assertStmtTostringMatches(LOCAL_LEVEL, "x += y", "x += y\n"); + } + + @Test + public void expressionStatement() { + assertStmtIndentedPrettyMatches(LOCAL_LEVEL, "5", " 5\n"); + assertStmtTostringMatches(LOCAL_LEVEL, "5", "5\n"); + } + + @Test + public void functionDefStatement() { + assertStmtIndentedPrettyMatches( + TOP_LEVEL, + join("def f(x):", + " print(x)"), + join(" def f(x):", + " print(x)", + "")); + assertStmtTostringMatches( + TOP_LEVEL, + join("def f(x):", + " print(x)"), + "def f(x): ...\n"); + + assertStmtIndentedPrettyMatches( + TOP_LEVEL, + join("def f(a, b=B, *c, d=D, **e):", + " print(x)"), + join(" def f(a, b=B, *c, d=D, **e):", + " print(x)", + "")); + assertStmtTostringMatches( + TOP_LEVEL, + join("def f(a, b=B, *c, d=D, **e):", + " print(x)"), + "def f(a, b = B, *c, d = D, **e): ...\n"); + + assertStmtIndentedPrettyMatches( + TOP_LEVEL, + join("def f():", + " pass"), + join(" def f():", + " pass", + "")); + assertStmtTostringMatches( + TOP_LEVEL, + join("def f():", + " pass"), + "def f(): ...\n"); + } + + @Test + public void flowStatement() { + // The parser would complain if we tried to construct them from source. + ASTNode breakNode = new FlowStatement(FlowStatement.Kind.BREAK); + assertIndentedPrettyMatches(breakNode, " break\n"); + assertTostringMatches(breakNode, "break\n"); + + ASTNode continueNode = new FlowStatement(FlowStatement.Kind.CONTINUE); + assertIndentedPrettyMatches(continueNode, " continue\n"); + assertTostringMatches(continueNode, "continue\n"); + } + + @Test + public void forStatement() { + assertStmtIndentedPrettyMatches( + LOCAL_LEVEL, + join("for x in y:", + " print(x)"), + join(" for x in y:", + " print(x)", + "")); + assertStmtTostringMatches( + LOCAL_LEVEL, + join("for x in y:", + " print(x)"), + "for x in y: ...\n"); + + assertStmtIndentedPrettyMatches( + LOCAL_LEVEL, + join("for x in y:", + " pass"), + join(" for x in y:", + " pass", + "")); + assertStmtTostringMatches( + LOCAL_LEVEL, + join("for x in y:", + " pass"), + "for x in y: ...\n"); + } + + @Test + public void ifStatement() { + assertStmtIndentedPrettyMatches( + LOCAL_LEVEL, + join("if True:", + " print(x)"), + join(" if True:", + " print(x)", + "")); + assertStmtTostringMatches( + LOCAL_LEVEL, + join("if True:", + " print(x)"), + "if True: ...\n"); + + assertStmtIndentedPrettyMatches( + LOCAL_LEVEL, + join("if True:", + " print(x)", + "elif False:", + " print(y)", + "else:", + " print(z)"), + join(" if True:", + " print(x)", + " elif False:", + " print(y)", + " else:", + " print(z)", + "")); + assertStmtTostringMatches( + LOCAL_LEVEL, + join("if True:", + " print(x)", + "elif False:", + " print(y)", + "else:", + " print(z)"), + "if True: ...\n"); + } + + @Test + public void loadStatement() { + // load("foo.bzl", a="A", "B") + ASTNode loadStatement = new LoadStatement( + new StringLiteral("foo.bzl"), + ImmutableMap.of(new Identifier("a"), "A", new Identifier("B"), "B")); + assertIndentedPrettyMatches( + loadStatement, + " load(\"foo.bzl\", a=\"A\", \"B\")\n"); + assertTostringMatches( + loadStatement, + "load(\"foo.bzl\", a=\"A\", \"B\")\n"); + } + + @Test + public void returnStatement() { + assertIndentedPrettyMatches( + new ReturnStatement(new StringLiteral("foo")), + " return \"foo\"\n"); + assertTostringMatches( + new ReturnStatement(new StringLiteral("foo")), + "return \"foo\"\n"); + + assertIndentedPrettyMatches( + new ReturnStatement(new Identifier("None")), + " return\n"); + assertTostringMatches( + new ReturnStatement(new Identifier("None")), + "return\n"); + } + + // Miscellaneous. + + @Test + public void buildFileAST() { + ASTNode node = parseBuildFileASTWithoutValidation("print(x)\nprint(y)"); + assertIndentedPrettyMatches( + node, + join(" print(x)", + " print(y)", + "")); + assertTostringMatches( + node, + "<BuildFileAST with 2 statements>"); + } + + @Test + public void comment() { + Comment node = new Comment("foo"); + assertIndentedPrettyMatches(node, " # foo"); + assertTostringMatches(node, "foo"); + } + + /* Not tested explicitly because they're covered implicitly by tests for other nodes: + * - LValue + * - DictionaryEntryLiteral + * - passed arguments / formal parameters + * - ConditionalStatements + */ +}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java index 606c4ce..afaf9aa 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -426,14 +426,6 @@ } @Test - public void testDictComprehensions_ToString() throws Exception { - assertThat(parseExpression("{x : x for x in [1, 2]}").toString()) - .isEqualTo("{x: x for x in [1, 2]}"); - assertThat(parseExpression("{x + 'a' : x for x in [1, 2]}").toString()) - .isEqualTo("{x + \"a\": x for x in [1, 2]}"); - } - - @Test public void testListConcatenation() throws Exception { newTest() .testStatement("[1, 2] + [3, 4]", MutableList.of(env, 1, 2, 3, 4))