Add more helpers to Parser This simplifies parsing a lone statement or expression in unit tests. RELNOTES: None PiperOrigin-RevId: 160276410
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 0101117..d28912f 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,9 @@ // limitations under the License. package com.google.devtools.build.lib.syntax; +import static com.google.devtools.build.lib.syntax.Parser.Dialect.BUILD; +import static com.google.devtools.build.lib.syntax.Parser.Dialect.SKYLARK; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; @@ -260,12 +263,12 @@ public static BuildFileAST parseBuildFile(ParserInputSource input, List<Statement> preludeStatements, EventHandler eventHandler) { - Parser.ParseResult result = Parser.parseFile(input, eventHandler); + Parser.ParseResult result = Parser.parseFile(input, eventHandler, BUILD); return create(preludeStatements, result, /*contentHashCode=*/ null, eventHandler); } public static BuildFileAST parseBuildFile(ParserInputSource input, EventHandler eventHandler) { - Parser.ParseResult result = Parser.parseFile(input, eventHandler); + Parser.ParseResult result = Parser.parseFile(input, eventHandler, BUILD); return create(ImmutableList.<Statement>of(), result, /*contentHashCode=*/ null, eventHandler); } @@ -283,7 +286,7 @@ public static BuildFileAST parseSkylarkFile(Path file, long fileSize, EventHandler eventHandler) throws IOException { ParserInputSource input = ParserInputSource.create(file, fileSize); - Parser.ParseResult result = Parser.parseFileForSkylark(input, eventHandler); + Parser.ParseResult result = Parser.parseFile(input, eventHandler, SKYLARK); return create( ImmutableList.<Statement>of(), result, HashCode.fromBytes(file.getDigest()).toString(), eventHandler); @@ -298,7 +301,7 @@ */ public static BuildFileAST parseSkylarkFileWithoutImports( ParserInputSource input, EventHandler eventHandler) { - ParseResult result = Parser.parseFileForSkylark(input, eventHandler); + ParseResult result = Parser.parseFile(input, eventHandler, SKYLARK); return new BuildFileAST( ImmutableList.<Statement>builder() .addAll(ImmutableList.<Statement>of()) @@ -324,19 +327,20 @@ return new BuildFileAST(stmts, true, contentHashCode, getLocation(), comments, imports); } - public static BuildFileAST parseBuildString(EventHandler eventHandler, String... content) { + private static BuildFileAST parseString( + Parser.Dialect dialect, EventHandler eventHandler, String... content) { String str = Joiner.on("\n").join(content); ParserInputSource input = ParserInputSource.create(str, PathFragment.EMPTY_FRAGMENT); - Parser.ParseResult result = Parser.parseFile(input, eventHandler); + Parser.ParseResult result = Parser.parseFile(input, eventHandler, dialect); return create(ImmutableList.<Statement>of(), result, null, eventHandler); } - // TODO(laurentlb): Merge parseSkylarkString and parseBuildString. + public static BuildFileAST parseBuildString(EventHandler eventHandler, String... content) { + return parseString(BUILD, eventHandler, content); + } + public static BuildFileAST parseSkylarkString(EventHandler eventHandler, String... content) { - String str = Joiner.on("\n").join(content); - ParserInputSource input = ParserInputSource.create(str, PathFragment.EMPTY_FRAGMENT); - Parser.ParseResult result = Parser.parseFileForSkylark(input, eventHandler); - return create(ImmutableList.<Statement>of(), result, null, eventHandler); + return parseString(SKYLARK, eventHandler, content); } /** @@ -345,7 +349,7 @@ * @return true if the input file is syntactically valid */ public static boolean checkSyntax(ParserInputSource input, EventHandler eventHandler) { - Parser.ParseResult result = Parser.parseFile(input, eventHandler); + Parser.ParseResult result = Parser.parseFile(input, eventHandler, BUILD); return !result.containsErrors; }
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 1416d8f..df37954 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
@@ -14,8 +14,7 @@ package com.google.devtools.build.lib.syntax; -import static com.google.devtools.build.lib.syntax.Parser.ParsingMode.BUILD; -import static com.google.devtools.build.lib.syntax.Parser.ParsingMode.SKYLARK; +import static com.google.devtools.build.lib.syntax.Parser.Dialect.SKYLARK; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Supplier; @@ -73,16 +72,23 @@ } } - /** - * ParsingMode is used to select which features the parser should accept. - */ - public enum ParsingMode { - /** Used for parsing BUILD files */ + /** Used to select whether the parser rejects features that are prohibited for BUILD files. */ + // TODO(brandjon): Instead of using an enum to control what features are allowed, factor these + // restrictions into a separate visitor that can be outside the core Skylark parser. This will + // reduce parser complexity and help keep Bazel-specific knowledge out of the interpreter. + public enum Dialect { + /** Used for BUILD files. */ BUILD, - /** Used for parsing .bzl files */ + /** Used for .bzl and other Skylark files. This allows all language features. */ SKYLARK, } + /** Used to select what constructs are allowed based on whether we're at the top level. */ + public enum ParsingLevel { + TOP_LEVEL, + LOCAL_LEVEL + } + private static final EnumSet<TokenKind> STATEMENT_TERMINATOR_SET = EnumSet.of(TokenKind.EOF, TokenKind.NEWLINE, TokenKind.SEMI); @@ -141,7 +147,7 @@ private final Lexer lexer; private final EventHandler eventHandler; private final List<Comment> comments; - private final ParsingMode parsingMode; + private final Dialect dialect; private static final Map<TokenKind, Operator> binaryOperators = new ImmutableMap.Builder<TokenKind, Operator>() @@ -192,10 +198,10 @@ private int errorsCount; private boolean recoveryMode; // stop reporting errors until next statement - private Parser(Lexer lexer, EventHandler eventHandler, ParsingMode parsingMode) { + private Parser(Lexer lexer, EventHandler eventHandler, Dialect dialect) { this.lexer = lexer; this.eventHandler = eventHandler; - this.parsingMode = parsingMode; + this.dialect = dialect; this.tokens = lexer.getTokens().iterator(); this.comments = new ArrayList<>(); nextToken(); @@ -212,17 +218,18 @@ } /** - * Entry-point for parsing a file with comments. + * Main entry point for parsing a file. * * @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 + * @param dialect may restrict the parser to Build-language features + * @see BuildFileAST#parseBuildString + * @see BuildFileAST#parseSkylarkString */ public static ParseResult parseFile( - ParserInputSource input, EventHandler eventHandler, ParsingMode parsingMode) { + ParserInputSource input, EventHandler eventHandler, Dialect dialect) { Lexer lexer = new Lexer(input, eventHandler); - Parser parser = new Parser(lexer, eventHandler, parsingMode); + Parser parser = new Parser(lexer, eventHandler, dialect); List<Statement> statements = parser.parseFileInput(); return new ParseResult( statements, @@ -231,31 +238,19 @@ 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 for parsing an expression. The expression may be followed by newline tokens. + * Parses a sequence of statements, possibly 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 + * <p>{@code load()} statements are not permitted. Use {@code parsingLevel} to control whether + * function definitions, for statements, etc., are allowed. */ - @VisibleForTesting - public static Expression parseExpression( - ParserInputSource input, EventHandler eventHandler, ParsingMode parsingMode) { + public static List<Statement> parseStatements( + ParserInputSource input, EventHandler eventHandler, + ParsingLevel parsingLevel, Dialect dialect) { Lexer lexer = new Lexer(input, eventHandler); - Parser parser = new Parser(lexer, eventHandler, parsingMode); - Expression result = parser.parseExpression(); + Parser parser = new Parser(lexer, eventHandler, dialect); + List<Statement> result = new ArrayList<>(); + parser.parseStatement(result, parsingLevel); while (parser.token.kind == TokenKind.NEWLINE) { parser.nextToken(); } @@ -263,17 +258,30 @@ 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 wrapper for {@link #parseStatements} where exactly one statement is expected. + * + * @throws IllegalArgumentException if the number of parsed statements was not exactly one + */ + public static Statement parseStatement( + ParserInputSource input, EventHandler eventHandler, + ParsingLevel parsingLevel, Dialect dialect) { + List<Statement> stmts = parseStatements( + input, eventHandler, parsingLevel, dialect); + return Iterables.getOnlyElement(stmts); } - /** Convenience method for {@code parseExpression} with Skylark. */ - @VisibleForTesting - public static Expression parseExpressionForSkylark( - ParserInputSource input, EventHandler eventHandler) { - return parseExpression(input, eventHandler, SKYLARK); + /** Parses an expression, possibly followed by newline tokens. */ + public static Expression parseExpression( + ParserInputSource input, EventHandler eventHandler, Dialect dialect) { + Lexer lexer = new Lexer(input, eventHandler); + Parser parser = new Parser(lexer, eventHandler, dialect); + Expression result = parser.parseExpression(); + while (parser.token.kind == TokenKind.NEWLINE) { + parser.nextToken(); + } + parser.expect(TokenKind.EOF); + return result; } private void reportError(Location location, String message) { @@ -452,7 +460,7 @@ final int start = token.left; // parse **expr if (token.kind == TokenKind.STAR_STAR) { - if (parsingMode != SKYLARK) { + if (dialect != SKYLARK) { reportError( lexer.createLocation(token.left, token.right), "**kwargs arguments are not allowed in BUILD files"); @@ -463,7 +471,7 @@ } // parse *expr if (token.kind == TokenKind.STAR) { - if (parsingMode != SKYLARK) { + if (dialect != SKYLARK) { reportError( lexer.createLocation(token.left, token.right), "*args arguments are not allowed in BUILD files"); @@ -1176,7 +1184,7 @@ } pushToken(identToken); // push the ident back to parse it as a statement } - parseStatement(list, true); + parseStatement(list, ParsingLevel.TOP_LEVEL); } // small_stmt | 'pass' @@ -1388,7 +1396,7 @@ } expect(TokenKind.INDENT); while (token.kind != TokenKind.OUTDENT && token.kind != TokenKind.EOF) { - parseStatement(list, false); + parseStatement(list, ParsingLevel.LOCAL_LEVEL); } expectAndRecover(TokenKind.OUTDENT); } else { @@ -1434,17 +1442,17 @@ // stmt ::= simple_stmt // | compound_stmt - private void parseStatement(List<Statement> list, boolean isTopLevel) { - if (token.kind == TokenKind.DEF && parsingMode == SKYLARK) { - if (!isTopLevel) { + private void parseStatement(List<Statement> list, ParsingLevel parsingLevel) { + if (token.kind == TokenKind.DEF && dialect == SKYLARK) { + if (parsingLevel == ParsingLevel.LOCAL_LEVEL) { reportError(lexer.createLocation(token.left, token.right), "nested functions are not allowed. Move the function to top-level"); } parseFunctionDefStatement(list); - } else if (token.kind == TokenKind.IF && parsingMode == SKYLARK) { + } else if (token.kind == TokenKind.IF && dialect == SKYLARK) { list.add(parseIfStatement()); - } else if (token.kind == TokenKind.FOR && parsingMode == SKYLARK) { - if (isTopLevel) { + } else if (token.kind == TokenKind.FOR && dialect == SKYLARK) { + if (parsingLevel == ParsingLevel.TOP_LEVEL) { reportError( lexer.createLocation(token.left, token.right), "for loops are not allowed on top-level. Put it into a function");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java b/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java index 69bede7..b2ae351 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/LValueBoundNamesTest.java
@@ -48,9 +48,9 @@ assertBoundNames("[[x], y], [z, w[1]] = 1", "x", "y", "z"); } - private static void assertBoundNames(String assignement, String... boundNames) { + private static void assertBoundNames(String assignment, String... boundNames) { BuildFileAST buildFileAST = BuildFileAST - .parseSkylarkString(Environment.FAIL_FAST_HANDLER, assignement); + .parseSkylarkString(Environment.FAIL_FAST_HANDLER, assignment); LValue lValue = ((AssignmentStatement) buildFileAST.getStatements().get(0)).getLValue(); Truth.assertThat(lValue.boundNames()).containsExactlyElementsIn(Arrays.asList(boundNames)); }
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 4120a89..19665c6 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
@@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.truth.Ordered; import com.google.devtools.build.lib.events.Event; @@ -152,18 +151,37 @@ return parseBuildFileAST(input).getStatements(); } - /** Parses an Expression from string without a supporting file */ - @VisibleForTesting - public Expression parseExpression(String... input) { - return Parser.parseExpression( - ParserInputSource.create(Joiner.on("\n").join(input), null), getEventHandler()); + /** Construct a ParserInputSource by concatenating multiple strings with newlines. */ + private ParserInputSource makeParserInputSource(String... input) { + return ParserInputSource.create(Joiner.on("\n").join(input), null); } - /** 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()); + /** Parses a statement, possibly followed by newlines. */ + protected Statement parseStatement(Parser.ParsingLevel parsingLevel, String... input) { + return Parser.parseStatement( + makeParserInputSource(input), getEventHandler(), + parsingLevel, Parser.Dialect.SKYLARK); + } + + /** Parses a top-level statement, possibly followed by newlines. */ + protected Statement parseTopLevelStatement(String... input) { + return Parser.parseStatement( + makeParserInputSource(input), getEventHandler(), + Parser.ParsingLevel.TOP_LEVEL, Parser.Dialect.SKYLARK); + } + + /** Parses a local statement, possibly followed by newlines. */ + protected Statement parseLocalLevelStatement(String... input) { + return Parser.parseStatement( + makeParserInputSource(input), getEventHandler(), + Parser.ParsingLevel.LOCAL_LEVEL, Parser.Dialect.SKYLARK); + } + + /** Parses an expression, possibly followed by newlines. */ + protected Expression parseExpression(String... input) { + return Parser.parseExpression( + makeParserInputSource(input), getEventHandler(), + Parser.Dialect.SKYLARK); } public EvaluationTestCase update(String varname, Object value) throws Exception {