Refactor AST APIs

Added public visibility to some constructors/accessors, and made child LValue nodes explicitly constructed by the caller rather than hidden inside constructors. This makes it easier to treat nodes as uniform dumb values. Also added some helpers.

RELNOTES: None
PiperOrigin-RevId: 158761415

16 files changed, 134 insertions, 82 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 84d859fd0c..d2e22d9dd7 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
@@ -21,6 +21,10 @@ import java.io.Serializable;
 
 /**
  * Root class for nodes in the Abstract Syntax Tree of the Build language.
+ *
+ * The standard {@link Object#equals} and {@link Object#hashCode} methods are not supported. This is
+ * because their implementation would require traversing the entire tree in the worst case, and we
+ * don't want this kind of cost to occur implicitly.
  */
 public abstract class ASTNode implements Serializable {
 
@@ -78,7 +82,7 @@ public abstract class ASTNode implements Serializable {
 
   @Override
   public int hashCode() {
-    throw new UnsupportedOperationException(); // avoid nondeterminism
+    throw new UnsupportedOperationException();
   }
 
   @Override
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 f37843bdb5..e41e78b6d0 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
@@ -27,8 +27,8 @@ public final class AssignmentStatement extends Statement {
   /**
    *  Constructs an assignment: "lvalue := value".
    */
-  AssignmentStatement(Expression lvalue, Expression expression) {
-    this.lvalue = new LValue(lvalue);
+  public AssignmentStatement(LValue lvalue, Expression expression) {
+    this.lvalue = lvalue;
     this.expression = expression;
   }
 
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 d869abae86..5b910ce777 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
@@ -24,9 +24,9 @@ public final class AugmentedAssignmentStatement extends Statement {
   private final Expression expression;
 
   /** Constructs an augmented assignment: "lvalue ::= value". */
-  AugmentedAssignmentStatement(Operator operator, Expression lhs, Expression expression) {
+  public AugmentedAssignmentStatement(Operator operator, LValue lvalue, Expression expression) {
     this.operator = operator;
-    this.lvalue = new LValue(lhs);
+    this.lvalue = lvalue;
     this.expression = expression;
   }
 
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 44f1c158d5..01011175d4 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
@@ -33,6 +33,9 @@ import javax.annotation.Nullable;
 /**
  * Abstract syntax node for an entire BUILD file.
  */
+// TODO(bazel-team): Consider breaking this up into two classes: One that extends ASTNode and does
+// not include import info; and one that wraps that object with additional import info but that
+// does not itself extend ASTNode. This would help keep the AST minimalistic.
 public class BuildFileAST extends ASTNode {
 
   private final ImmutableList<Statement> stmts;
@@ -292,22 +295,20 @@ public class BuildFileAST extends ASTNode {
    *
    * <p>This method should not be used in Bazel code, since it doesn't validate that the imports are
    * syntactically valid.
-   *
-   * @throws IOException if the file cannot not be read.
    */
   public static BuildFileAST parseSkylarkFileWithoutImports(
-      ParserInputSource input, EventHandler eventHandler) throws IOException {
+      ParserInputSource input, EventHandler eventHandler) {
     ParseResult result = Parser.parseFileForSkylark(input, eventHandler);
     return new BuildFileAST(
         ImmutableList.<Statement>builder()
             .addAll(ImmutableList.<Statement>of())
             .addAll(result.statements)
             .build(),
-        result.containsErrors, /*contentHashCode=*/
-        null,
+        result.containsErrors,
+        /*contentHashCode=*/null,
         result.location,
-        ImmutableList.copyOf(result.comments), /*imports=*/
-        null);
+        ImmutableList.copyOf(result.comments),
+        /*imports=*/null);
   }
 
   /**
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 b7c8d57f76..63ce6cb818 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
@@ -41,7 +41,7 @@ public final class FlowStatement extends Statement {
     this.ex = new FlowException(kind);
   }
 
-  Kind getKind() {
+  public Kind getKind() {
     return kind;
   }
 
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 797782ada9..34d0c30c75 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
@@ -30,8 +30,8 @@ public final class ForStatement extends Statement {
   /**
    * Constructs a for loop statement.
    */
-  ForStatement(Expression variable, Expression collection, List<Statement> block) {
-    this.variable = new LValue(Preconditions.checkNotNull(variable));
+  public ForStatement(LValue variable, Expression collection, List<Statement> block) {
+    this.variable = Preconditions.checkNotNull(variable);
     this.collection = Preconditions.checkNotNull(collection);
     this.block = ImmutableList.copyOf(block);
   }
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 3951b2b72e..e6ef1bab6d 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
@@ -32,9 +32,9 @@ public final class FunctionDefStatement extends Statement {
       FunctionSignature.WithValues<Expression, Expression> signature,
       Iterable<Statement> statements) {
     this.ident = ident;
+    this.parameters = ImmutableList.copyOf(parameters);
     this.signature = signature;
     this.statements = ImmutableList.copyOf(statements);
-    this.parameters = ImmutableList.copyOf(parameters);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
index 2903f8e6de..401b4a307b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -223,31 +223,39 @@ public abstract class FunctionSignature implements Serializable {
     /** The underlying signature with parameter shape and names */
     public abstract FunctionSignature getSignature();
 
-    /** The default values (if any) as a List of one per optional parameter.
-     * We might have preferred ImmutableList, but we care about
-     * supporting null's for some BuiltinFunction's, and we don't spit on speed.
+    /**
+     * The default values (if any) as an unmodifiable List of one per optional parameter. May
+     * contain nulls.
      */
     @Nullable public abstract List<V> getDefaultValues();
 
-    /** The parameter types (if specified) as a List of one per parameter, including * and **.
-     * We might have preferred ImmutableList, but we care about supporting null's
-     * so we can take shortcut for untyped values.
+    /**
+     * The parameter types (if specified) as an unmodifiable List of one per parameter, including *
+     * and **. May contain nulls.
      */
     @Nullable public abstract List<T> getTypes();
 
 
-    /**
-     * Create a signature with (default and type) values.
-     * If you supply mutable List's, we trust that you won't modify them afterwards.
-     */
+    /** Create a signature with (default and type) values. */
     public static <V, T> WithValues<V, T> create(FunctionSignature signature,
         @Nullable List<V> defaultValues, @Nullable List<T> types) {
       Shape shape = signature.getShape();
-      Preconditions.checkArgument(defaultValues == null
-          || defaultValues.size() == shape.getOptionals());
-      Preconditions.checkArgument(types == null
-          || types.size() == shape.getArguments());
-      return new AutoValue_FunctionSignature_WithValues<>(signature, defaultValues, types);
+      List<V> convertedDefaultValues = null;
+      if (defaultValues != null) {
+        Preconditions.checkArgument(defaultValues.size() == shape.getOptionals());
+        List<V> copiedDefaultValues = new ArrayList<>();
+        copiedDefaultValues.addAll(defaultValues);
+        convertedDefaultValues = Collections.unmodifiableList(copiedDefaultValues);
+      }
+      List<T> convertedTypes = null;
+      if (types != null) {
+        Preconditions.checkArgument(types.size() == shape.getArguments());
+        List<T> copiedTypes = new ArrayList<>();
+        copiedTypes.addAll(types);
+        convertedTypes = Collections.unmodifiableList(copiedTypes);
+      }
+      return new AutoValue_FunctionSignature_WithValues<>(
+          signature, convertedDefaultValues, convertedTypes);
     }
 
     public static <V, T> WithValues<V, T> create(FunctionSignature signature,
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 4dce2284c5..ada928ba48 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
@@ -26,7 +26,11 @@ import javax.annotation.Nullable;
 // into array reference with a constant index. Variable lookups are currently a speed bottleneck,
 // as previously measured in an experiment.
 /**
- *  Syntax node for an identifier.
+ * Syntax node for an identifier.
+ *
+ * Unlike most {@link ASTNode} subclasses, this one supports {@link Object#equals} and {@link
+ * Object#hashCode} (but note that these methods ignore location information). They are needed
+ * because {@code Identifier}s are stored in maps when constructing {@link LoadStatement}.
  */
 public final class Identifier extends Expression {
 
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 697923f264..b6adc6067b 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
@@ -25,7 +25,7 @@ public final class IfStatement extends Statement {
   /**
    * Syntax node for an [el]if statement.
    */
-  static final class ConditionalStatements extends Statement {
+  public static final class ConditionalStatements extends Statement {
 
     private final Expression condition;
     private final ImmutableList<Statement> stmts;
@@ -52,11 +52,11 @@ public final class IfStatement extends Statement {
       visitor.visit(this);
     }
 
-    Expression getCondition() {
+    public Expression getCondition() {
       return condition;
     }
 
-    ImmutableList<Statement> getStmts() {
+    public ImmutableList<Statement> getStmts() {
       return stmts;
     }
 
@@ -74,7 +74,7 @@ public final class IfStatement extends Statement {
    * Constructs a if-elif-else statement. The else part is mandatory, but the list may be empty.
    * ThenBlocks has to have at least one element.
    */
-  IfStatement(List<ConditionalStatements> thenBlocks, List<Statement> elseBlock) {
+  public IfStatement(List<ConditionalStatements> thenBlocks, List<Statement> elseBlock) {
     Preconditions.checkArgument(!thenBlocks.isEmpty());
     this.thenBlocks = ImmutableList.copyOf(thenBlocks);
     this.elseBlock = ImmutableList.copyOf(elseBlock);
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 b098eadfaf..e13074c0c5 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
@@ -23,7 +23,7 @@ import java.util.Map;
  */
 public final class LoadStatement extends Statement {
 
-  private final ImmutableMap<Identifier, String> symbols;
+  private final ImmutableMap<Identifier, String> symbolMap;
   private final ImmutableList<Identifier> cachedSymbols; // to save time
   private final StringLiteral imp;
 
@@ -34,10 +34,14 @@ public final class LoadStatement extends Statement {
    * the bzl file that should be loaded. If aliasing is used, the value differs from its key's
    * {@code symbol.getName()}. Otherwise, both values are identical.
    */
-  LoadStatement(StringLiteral imp, Map<Identifier, String> symbols) {
+  public LoadStatement(StringLiteral imp, Map<Identifier, String> symbolMap) {
     this.imp = imp;
-    this.symbols = ImmutableMap.copyOf(symbols);
-    this.cachedSymbols = ImmutableList.copyOf(symbols.keySet());
+    this.symbolMap = ImmutableMap.copyOf(symbolMap);
+    this.cachedSymbols = ImmutableList.copyOf(symbolMap.keySet());
+  }
+
+  public ImmutableMap<Identifier, String> getSymbolMap() {
+    return symbolMap;
   }
 
   public ImmutableList<Identifier> getSymbols() {
@@ -56,7 +60,7 @@ public final class LoadStatement extends Statement {
 
   @Override
   void doExec(Environment env) throws EvalException, InterruptedException {
-    for (Map.Entry<Identifier, String> entry : symbols.entrySet()) {
+    for (Map.Entry<Identifier, String> entry : symbolMap.entrySet()) {
       try {
         Identifier name = entry.getKey();
         Identifier declared = new Identifier(entry.getValue());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 1cb224d42e..763b2332b4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -212,26 +212,17 @@ public class Parser {
   }
 
   /**
-   * Entry-point to parser that parses a build file with comments. All errors encountered during
-   * parsing are reported via "reporter".
-   */
-  public static ParseResult parseFile(ParserInputSource input, EventHandler eventHandler) {
-    Lexer lexer = new Lexer(input, eventHandler);
-    Parser parser = new Parser(lexer, eventHandler, BUILD);
-    List<Statement> statements = parser.parseFileInput();
-    return new ParseResult(statements, parser.comments, locationFromStatements(lexer, statements),
-        parser.errorsCount > 0 || lexer.containsErrors());
-  }
-
-  /**
-   * Entry-point to parser that parses a build file with comments. All errors encountered during
-   * parsing are reported via "reporter". Enable Skylark extensions that are not part of the core
-   * BUILD language.
+   * Entry-point for parsing a file with comments.
+   *
+   * @param input the input to parse
+   * @param eventHandler a reporter for parsing errors
+   * @param parsingMode if set to {@link ParsingMode#BUILD}, restricts the parser to just the
+   *     features present in the Build language
    */
-  public static ParseResult parseFileForSkylark(
-      ParserInputSource input, EventHandler eventHandler) {
+  public static ParseResult parseFile(
+      ParserInputSource input, EventHandler eventHandler, ParsingMode parsingMode) {
     Lexer lexer = new Lexer(input, eventHandler);
-    Parser parser = new Parser(lexer, eventHandler, SKYLARK);
+    Parser parser = new Parser(lexer, eventHandler, parsingMode);
     List<Statement> statements = parser.parseFileInput();
     return new ParseResult(
         statements,
@@ -240,15 +231,30 @@ public class Parser {
         parser.errorsCount > 0 || lexer.containsErrors());
   }
 
+  /** Convenience method for {@code parseFile} with the Build language. */
+  public static ParseResult parseFile(ParserInputSource input, EventHandler eventHandler) {
+    return parseFile(input, eventHandler, BUILD);
+  }
+
+  /** Convenience method for {@code parseFile} with Skylark. */
+  public static ParseResult parseFileForSkylark(
+      ParserInputSource input, EventHandler eventHandler) {
+    return parseFile(input, eventHandler, SKYLARK);
+  }
+
   /**
-   * Entry-point to parser that parses an expression.  All errors encountered
-   * during parsing are reported via "reporter".  The expression may be followed
-   * by newline tokens.
+   * Entry-point for parsing an expression. The expression may be followed by newline tokens.
+   *
+   * @param input the input to parse
+   * @param eventHandler a reporter for parsing errors
+   * @param parsingMode if set to {@link ParsingMode#BUILD}, restricts the parser to just the
+   *     features present in the Build language
    */
   @VisibleForTesting
-  public static Expression parseExpression(ParserInputSource input, EventHandler eventHandler) {
+  public static Expression parseExpression(
+      ParserInputSource input, EventHandler eventHandler, ParsingMode parsingMode) {
     Lexer lexer = new Lexer(input, eventHandler);
-    Parser parser = new Parser(lexer, eventHandler, null);
+    Parser parser = new Parser(lexer, eventHandler, parsingMode);
     Expression result = parser.parseExpression();
     while (parser.token.kind == TokenKind.NEWLINE) {
       parser.nextToken();
@@ -257,6 +263,19 @@ public class Parser {
     return result;
   }
 
+  /** Convenience method for {@code parseExpression} with the Build language. */
+  @VisibleForTesting
+  public static Expression parseExpression(ParserInputSource input, EventHandler eventHandler) {
+    return parseExpression(input, eventHandler, BUILD);
+  }
+
+  /** Convenience method for {@code parseExpression} with Skylark. */
+  @VisibleForTesting
+  public static Expression parseExpressionForSkylark(
+      ParserInputSource input, EventHandler eventHandler) {
+    return parseExpression(input, eventHandler, SKYLARK);
+  }
+
   private void reportError(Location location, String message) {
     errorsCount++;
     // Limit the number of reported errors to avoid spamming output.
@@ -1200,14 +1219,16 @@ public class Parser {
       nextToken();
       Expression rvalue = parseExpression();
       return setLocation(
-          new AssignmentStatement(/*lvalue=*/ expression, /*expression=*/ rvalue), start, rvalue);
+          new AssignmentStatement(new LValue(expression), rvalue),
+          start, rvalue);
     } else if (augmentedAssignmentMethods.containsKey(token.kind)) {
       Operator operator = augmentedAssignmentMethods.get(token.kind);
       nextToken();
       Expression operand = parseExpression();
       int end = operand.getLocation().getEndOffset();
       return setLocation(
-          new AugmentedAssignmentStatement(operator, expression, operand), start, end);
+          new AugmentedAssignmentStatement(operator, new LValue(expression), operand),
+          start, end);
     } else {
       return setLocation(new ExpressionStatement(expression), start, expression);
     }
@@ -1254,7 +1275,7 @@ public class Parser {
     enterLoop();
     try {
       List<Statement> block = parseSuite();
-      Statement stmt = new ForStatement(loopVar, collection, block);
+      Statement stmt = new ForStatement(new LValue(loopVar), collection, block);
       list.add(setLocation(stmt, start, token.right));
     } finally {
       exitLoop();
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 0976982dcf..45889a4287 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
@@ -56,7 +56,7 @@ public final class ReturnStatement extends Statement {
     throw new ReturnException(returnExpression.getLocation(), returnExpression.eval(env));
   }
 
-  Expression getReturnExpression() {
+  public Expression getReturnExpression() {
     return returnExpression;
   }
 
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 9d28ee8b19..b2ddcc0ff3 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,11 +24,7 @@ import java.util.Objects;
 /**
  * Factory class for creating appropriate instances of {@link SkylarkImports}.
  */
-public class SkylarkImports {
-
-  private SkylarkImports() {
-    throw new IllegalStateException("This class should not be instantiated");
-  }
+public abstract class SkylarkImports {
 
   // Default implementation class for SkylarkImport.
   private abstract static class SkylarkImportImpl implements SkylarkImport {
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java
index fa9dfbfa51..7aa1474fb2 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkImportsTest.java
@@ -30,7 +30,7 @@ import org.junit.runners.JUnit4;
  * Tests {@link SkylarkImports}.
  */
 @RunWith(JUnit4.class)
-public class SkylarkImportTest {
+public class SkylarkImportsTest {
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
@@ -39,17 +39,17 @@ public class SkylarkImportTest {
     SkylarkImport importForLabel = SkylarkImports.create(labelString);
 
     assertThat(importForLabel.hasAbsolutePath()).named("hasAbsolutePath()").isFalse();
-    assertThat(importForLabel.getImportString()).named("getIMportString()").isEqualTo(labelString);
+    assertThat(importForLabel.getImportString()).named("getImportString()").isEqualTo(labelString);
 
     Label irrelevantContainingFile = Label.parseAbsoluteUnchecked("//another/path:BUILD");
     assertThat(importForLabel.getLabel(irrelevantContainingFile)).named("getLabel()")
         .isEqualTo(Label.parseAbsoluteUnchecked(expectedLabelString));
- 
+
     assertThat(importForLabel.asPathFragment()).named("asPathFragment()")
         .isEqualTo(PathFragment.create(expectedPathString));
 
     thrown.expect(IllegalStateException.class);
-    importForLabel.getAbsolutePath();   
+    importForLabel.getAbsolutePath();
   }
 
   @Test
@@ -147,7 +147,7 @@ public class SkylarkImportTest {
     Label containingLabel = Label.parseAbsolute(containingLabelString);
     assertThat(importForPath.getLabel(containingLabel)).named("getLabel()")
         .isEqualTo(Label.parseAbsolute(expectedLabelString));
-    
+
     assertThat(importForPath.asPathFragment()).named("asPathFragment()")
         .isEqualTo(PathFragment.create(expectedPathString));
 
@@ -174,7 +174,7 @@ public class SkylarkImportTest {
   private void invalidImportTest(String importString, String expectedMsgPrefix) throws Exception {
     thrown.expect(SkylarkImportSyntaxException.class);
     thrown.expectMessage(startsWith(expectedMsgPrefix));
-    SkylarkImports.create(importString);   
+    SkylarkImports.create(importString);
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
index ce5e91607c..9ae284d0d6 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
@@ -139,10 +139,17 @@ public class EvaluationTestCase {
     return env;
   }
 
+  protected BuildFileAST parseBuildFileASTWithoutValidation(String... input) {
+    return BuildFileAST.parseSkylarkString(getEventHandler(), input);
+  }
+
+  protected BuildFileAST parseBuildFileAST(String... input) {
+    BuildFileAST ast = parseBuildFileASTWithoutValidation(input);
+    return ast.validate(env, getEventHandler());
+  }
+
   protected List<Statement> parseFile(String... input) {
-    BuildFileAST ast = BuildFileAST.parseSkylarkString(getEventHandler(), input);
-    ast = ast.validate(env, getEventHandler());
-    return ast.getStatements();
+    return parseBuildFileAST(input).getStatements();
   }
 
   /** Parses an Expression from string without a supporting file */
@@ -152,6 +159,13 @@ public class EvaluationTestCase {
         ParserInputSource.create(Joiner.on("\n").join(input), null), getEventHandler());
   }
 
+  /** Same as {@link #parseExpression} but supports Skylark constructs. */
+  @VisibleForTesting
+  public Expression parseExpressionForSkylark(String... input) {
+    return Parser.parseExpressionForSkylark(
+        ParserInputSource.create(Joiner.on("\n").join(input), null), getEventHandler());
+  }
+
   public EvaluationTestCase update(String varname, Object value) throws Exception {
     env.update(varname, value);
     return this;