Preliminary cleanup for removing Blaze-specific code from the environment

The goal is to remove parse and eval functions from Environment, as well as
isSkylark boolean.

--
MOS_MIGRATED_REVID=129202204

4 files changed, 69 insertions, 34 deletions

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 3d324b21bf..dffd6352a8 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
@@ -13,6 +13,7 @@
 // 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.hash.HashCode;
 import com.google.devtools.build.lib.events.Event;
@@ -238,6 +239,21 @@ public class BuildFileAST extends ASTNode {
         HashCode.fromBytes(file.getMD5Digest()).toString());
   }
 
+  public static BuildFileAST parseBuildString(EventHandler eventHandler, String... content) {
+    String str = Joiner.on("\n").join(content);
+    ParserInputSource input = ParserInputSource.create(str, null);
+    Parser.ParseResult result = Parser.parseFile(input, eventHandler, false);
+    return new BuildFileAST(ImmutableList.<Statement>of(), result, null);
+  }
+
+  // TODO(laurentlb): Merge parseSkylarkString and parseBuildString.
+  public static BuildFileAST parseSkylarkString(EventHandler eventHandler, String... content) {
+    String str = Joiner.on("\n").join(content);
+    ParserInputSource input = ParserInputSource.create(str, null);
+    Parser.ParseResult result = Parser.parseFileForSkylark(input, eventHandler, null);
+    return new BuildFileAST(ImmutableList.<Statement>of(), result, null);
+  }
+
   /**
    * Parse the specified build file, without building the AST.
    *
@@ -249,6 +265,23 @@ public class BuildFileAST extends ASTNode {
   }
 
   /**
+   * Evaluates the code and return the value of the last statement if it's an
+   * Expression or else null.
+   */
+  @Nullable public Object eval(Environment env) throws EvalException, InterruptedException {
+    Object last = null;
+    for (Statement statement : stmts) {
+      if (statement instanceof ExpressionStatement) {
+        last = ((ExpressionStatement) statement).getExpression().eval(env);
+      } else {
+        statement.exec(env);
+        last = null;
+      }
+    }
+    return last;
+  }
+
+  /**
    * Returns a hash code calculated from the string content of the source file of this AST.
    */
   @Nullable public String getContentHashCode() {
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 a2db357ef6..4418167d33 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
@@ -321,12 +321,14 @@ public final class Environment implements Freezable {
 
   /**
    * Is this Environment being executed in Skylark context?
+   * TODO(laurentlb): Remove from Environment
    */
   private boolean isSkylark;
 
   /**
    * Is this Environment being executed during the loading phase?
    * Many builtin functions are only enabled during the loading phase, and check this flag.
+   * TODO(laurentlb): Remove from Environment
    */
   private Phase phase;
 
@@ -355,6 +357,7 @@ public final class Environment implements Freezable {
 
   /**
    * The path to the tools repository.
+   * TODO(laurentlb): Remove from Environment
    */
   private final String toolsRepository;
 
@@ -935,42 +938,35 @@ public final class Environment implements Freezable {
     };
 
   /**
-   * Parses some String input without a supporting file, returning statements and comments.
+   * Parses some String inputLines without a supporting file, returning statements only.
+   * TODO(laurentlb): Remove from Environment
    * @param inputLines a list of lines of code
    */
   @VisibleForTesting
-  Parser.ParseResult parseFileWithComments(String... inputLines) {
+  public List<Statement> parseFile(String... inputLines) {
     ParserInputSource input = ParserInputSource.create(Joiner.on("\n").join(inputLines), null);
-    return isSkylark
+    Parser.ParseResult result = isSkylark
         ? Parser.parseFileForSkylark(input, eventHandler, new ValidationEnvironment(this))
         : Parser.parseFile(input, eventHandler, /*parsePython=*/false);
-  }
-
-  /**
-   * Parses some String input without a supporting file, returning statements only.
-   * @param input a list of lines of code
-   */
-  @VisibleForTesting
-  public List<Statement> parseFile(String... input) {
-    return parseFileWithComments(input).statements;
+    return result.statements;
   }
 
   /**
    * Evaluates code some String input without a supporting file.
+   * TODO(laurentlb): Remove from Environment
    * @param input a list of lines of code to evaluate
    * @return the value of the last statement if it's an Expression or else null
    */
   @Nullable public Object eval(String... input) throws EvalException, InterruptedException {
-    Object last = null;
-    for (Statement statement : parseFile(input)) {
-      if (statement instanceof ExpressionStatement) {
-        last = ((ExpressionStatement) statement).getExpression().eval(this);
-      } else {
-        statement.exec(this);
-        last = null;
-      }
-    }
-    return last;
+    BuildFileAST ast;
+    if (isSkylark) {
+      ast = BuildFileAST.parseSkylarkString(eventHandler, input);
+      ValidationEnvironment valid = new ValidationEnvironment(this);
+      valid.validateAst(ast.getStatements(), eventHandler);
+    } else {
+      ast = BuildFileAST.parseBuildString(eventHandler, input);
+    }
+    return ast.eval(this);
   }
 
   public String getToolsRepository() {
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 dbc16cdfa5..641f7d9bfd 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
@@ -240,18 +240,10 @@ public class Parser {
     Lexer lexer = new Lexer(input, eventHandler, false);
     Parser parser = new Parser(lexer, eventHandler, SKYLARK);
     List<Statement> statements = parser.parseFileInput();
-    boolean hasSemanticalErrors = false;
-    try {
-      if (validationEnvironment != null) {
-        validationEnvironment.validateAst(statements);
-      }
-    } catch (EvalException e) {
-      // Do not report errors caused by a previous parsing error, as it has already been reported.
-      if (!e.isDueToIncompleteAST()) {
-        eventHandler.handle(Event.error(e.getLocation(), e.getMessage()));
-      }
-      hasSemanticalErrors = true;
-    }
+    // TODO(laurentlb): Remove validation from parser
+    boolean hasSemanticalErrors = validationEnvironment == null
+        ? false
+        : !validationEnvironment.validateAst(statements, eventHandler);
     return new ParseResult(statements, parser.comments, locationFromStatements(lexer, statements),
         parser.errorsCount > 0 || lexer.containsErrors() || hasSemanticalErrors);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 8c625d8138..991b106d91 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -14,6 +14,8 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -157,6 +159,18 @@ public final class ValidationEnvironment {
     }
   }
 
+  public boolean validateAst(List<Statement> statements, EventHandler eventHandler) {
+    try {
+      validateAst(statements);
+      return true;
+    } catch (EvalException e) {
+      if (!e.isDueToIncompleteAST()) {
+        eventHandler.handle(Event.error(e.getLocation(), e.getMessage()));
+      }
+      return false;
+    }
+  }
+
   /**
    * Returns whether the current statement is inside a for loop (either in this environment or one
    * of its parents)