From 8e965b8a449f802ac0adda10a8ca8d713796f996 Mon Sep 17 00:00:00 2001 From: Laurent Le Brun Date: Wed, 3 Aug 2016 11:50:24 +0000 Subject: 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 --- .../devtools/build/lib/syntax/BuildFileAST.java | 33 ++++++++++++++++++ .../devtools/build/lib/syntax/Environment.java | 40 ++++++++++------------ .../google/devtools/build/lib/syntax/Parser.java | 16 +++------ .../build/lib/syntax/ValidationEnvironment.java | 14 ++++++++ 4 files changed, 69 insertions(+), 34 deletions(-) (limited to 'src/main') 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.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.of(), result, null); + } + /** * Parse the specified build file, without building the AST. * @@ -248,6 +264,23 @@ public class BuildFileAST extends ASTNode { return !parseBuildFile(input, eventHandler, parsePython).containsErrors(); } + /** + * 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. */ 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 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 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 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 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) -- cgit v1.2.3