bazel syntax: break dependence on EventHandler
This change removes nearly all uses of EventHandler in the interpreter.
(The remaining uses are for events created by the Starlark 'print' statement.
A follow-up change will create a dedicated StarlarkThread.PrintHandler.)
All parse functions now report errors in one of two ways:
1) by throwing a SyntaxError exception containing a list of events.
This approach is used by "all-or-nothing" parse operations related
to expressions. Clients may catch the exception, extract the events,
and replay them on their handler should they choose. Example:
try {
Expression expr = Expression.parse(input)
...
} catch (SyntaxError ex) {
Event.replayEventsOn(handler, ex.errors());
}
2) by recording a list of scan/parse errors in StarlarkFile.errors.
The result of parsing a file is both a syntax tree (possibly incomplete),
and a list of errors. If parsing is followed by validation,
the validator appends its errors to this list. (Validation is now
capable of reporting multiple errors per file. ValidationException is gone.)
StarlarkFile file = StarlarkFile.parse(input)
if (!file.ok()) {
Event.replayEventsOn(handler, file.errors());
return;
}
Or:
StarlarkFile file = StarlarkFile.parse(input)
if (!file.ok()) {
throw new SyntaxError(file.errors());
}
Details:
- error helpers for Identifiers have moved from Eval to ValidationEnvironment
and no longer depend on EvalException.
- The EvalException.incompleteAST concept is irrelevant now that
clients do not proceed to evaluation if the parser or validation
step produced errors.
- StarlarkFile: exec + execTopLevelStatement have moved to EvalUtils, and will
change further.
- replayLexerEvents is gone from the API. The validator (which has access
to a StarlarkSemantics) can optionally copy the string escape events
into the main error bucket, or ignore them.
- StarlarkFile: eliminate parseWithoutImports
- Parser: delete dead code parseStatement{,s}
- Lexer: delete dead code stringAtLine
Numerous fixes were required in the tests, because they are such a muddle
of different phases (scan, parse, validate, evaluate)
and modes (BUILD vs .bzl vs core Starlark).
RELNOTES: Multiple Starlark validation errors are reported in a single pass.
PiperOrigin-RevId: 272205103
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
index eb6cbe5..c052eb9 100644
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java
@@ -103,7 +103,7 @@
"runfiles");
ExtendedEventHandler listener = Mockito.mock(ExtendedEventHandler.class);
ParserInput input = ParserInput.fromLines("test()");
- FuncallExpression ast = (FuncallExpression) Expression.parse(input, listener);
+ FuncallExpression ast = (FuncallExpression) Expression.parse(input);
Rule rule =
WorkspaceFactoryHelper.createAndAddRepositoryRule(
packageBuilder, buildRuleClass(attributes), null, kwargs, ast);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index 806442f..dc91321 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -1188,10 +1188,7 @@
" pattern,",
"])",
"other_variable = glob(include = ['a'], exclude = ['b'])",
- "third_variable = glob(['c'], exclude_directories = 0)"),
- event -> {
- throw new IllegalArgumentException(event.getMessage());
- }));
+ "third_variable = glob(['c'], exclude_directories = 0)")));
assertThat(globPatternExtractor.getExcludeDirectoriesPatterns())
.containsExactly("ab", "a", "**/*");
assertThat(globPatternExtractor.getIncludeDirectoriesPatterns()).containsExactly("c");
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
index 0c81e9b..5272098 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
@@ -134,11 +134,12 @@
/** Instantiates a {@link SkylarkInfo} with fields a=1, b=2, c=3 (and nothing else). */
private static SkylarkInfo instantiateWithA1B2C3(SkylarkProvider provider) throws Exception{
// Code under test begins with the entry point in BaseFunction.
- Object result = provider.call(
- ImmutableList.of(),
- ImmutableMap.of("a", 1, "b", 2, "c", 3),
- /*ast=*/ null,
- /*env=*/ null);
+ Object result =
+ provider.call(
+ ImmutableList.of(),
+ ImmutableMap.of("a", 1, "b", 2, "c", 3),
+ /*ast=*/ null,
+ /*thread=*/ null);
assertThat(result).isInstanceOf(SkylarkInfo.class);
return (SkylarkInfo) result;
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index 9ce4643..e1b2bdb 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -138,7 +138,9 @@
public StarlarkFile ast(Path buildFile) throws IOException {
byte[] bytes = FileSystemUtils.readWithKnownFileSize(buildFile, buildFile.getFileSize());
ParserInput input = ParserInput.create(bytes, buildFile.asFragment());
- return StarlarkFile.parse(input, eventHandler);
+ StarlarkFile file = StarlarkFile.parse(input);
+ Event.replayEventsOn(eventHandler, file.errors());
+ return file;
}
/** Evaluates the {@code buildFileAST} into a {@link Package}. */
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
index 4ca6764..4ab2bd7 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
@@ -343,7 +343,6 @@
Pair<Set<Label>, Boolean> result = parseListKeepGoing("foo/...");
assertThat(result.first).containsExactlyElementsIn(rulesBeneathFoo);
assertContainsEvent("syntax error at 'build'");
- assertContainsEvent("name 'invalid' is not defined");
reporter.addHandler(failFastHandler);
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 7baff22..002297c 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -59,6 +59,7 @@
import com.google.devtools.build.lib.syntax.StarlarkFile;
import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
+import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.Collection;
@@ -705,6 +706,9 @@
protected void evalAndExport(String... lines) throws Exception {
ParserInput input = ParserInput.fromLines(lines);
StarlarkFile file = StarlarkFile.parseAndValidateSkylark(input, ev.getStarlarkThread());
+ if (!file.errors().isEmpty()) {
+ throw new SyntaxError(file.errors());
+ }
SkylarkImportLookupFunction.execAndExport(
file, FAKE_LABEL, ev.getEventHandler(), ev.getStarlarkThread());
}
@@ -1255,14 +1259,6 @@
}
@Test
- public void testStructMembersAreImmutable() throws Exception {
- checkErrorContains(
- "cannot assign to 's.x'",
- "s = struct(x = 'a')",
- "s.x = 'b'\n");
- }
-
- @Test
public void testStructDictMembersAreMutable() throws Exception {
eval(
"s = struct(x = {'a' : 1})",
diff --git a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java
index a614466..4a4e846 100644
--- a/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServerTest.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.skylarkdebugging.SkylarkDebuggingProtos;
@@ -763,11 +764,13 @@
return StarlarkThread.builder(mutability).useDefaultSemantics().build();
}
- private StarlarkFile parseBuildFile(String path, String... lines) throws IOException {
- Path file = scratch.file(path, lines);
- byte[] bytes = FileSystemUtils.readWithKnownFileSize(file, file.getFileSize());
- ParserInput inputSource = ParserInput.create(bytes, file.asFragment());
- return StarlarkFile.parse(inputSource, events.reporter());
+ private StarlarkFile parseBuildFile(String filename, String... lines) throws IOException {
+ Path path = scratch.file(filename, lines);
+ byte[] bytes = FileSystemUtils.readWithKnownFileSize(path, path.getFileSize());
+ ParserInput input = ParserInput.create(bytes, path.asFragment());
+ StarlarkFile file = StarlarkFile.parse(input);
+ Event.replayEventsOn(events.reporter(), file.errors());
+ return file;
}
private StarlarkFile parseSkylarkFile(String path, String... lines) throws IOException {
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
index 12ca90a..18e5fbe 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ASTPrettyPrintTest.java
@@ -18,6 +18,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
import java.io.IOException;
import org.junit.Test;
@@ -66,8 +67,12 @@
* string.
*/
private void assertExprPrettyMatches(String source, String expected) {
- Expression node = parseExpression(source);
- assertPrettyMatches(node, expected);
+ try {
+ Expression node = parseExpression(source);
+ assertPrettyMatches(node, expected);
+ } catch (SyntaxError ex) {
+ Event.replayEventsOn(getEventHandler(), ex.errors());
+ }
}
/**
@@ -75,8 +80,12 @@
* given string.
*/
private void assertExprTostringMatches(String source, String expected) {
- Expression node = parseExpression(source);
- assertThat(node.toString()).isEqualTo(expected);
+ try {
+ Expression node = parseExpression(source);
+ assertThat(node.toString()).isEqualTo(expected);
+ } catch (SyntaxError ex) {
+ Event.replayEventsOn(getEventHandler(), ex.errors());
+ }
}
/**
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 41b0ff0..00592ae 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
@@ -32,9 +32,9 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Test of evaluation behavior. (Implicitly uses lexer + parser.)
- */
+/** Test of evaluation behavior. (Implicitly uses lexer + parser.) */
+// TODO(adonovan): separate tests of parser, resolver, Starlark core evaluator,
+// and BUILD and .bzl features.
@RunWith(JUnit4.class)
public class EvaluationTest extends EvaluationTestCase {
@@ -107,11 +107,6 @@
.testStatement("1 if True else 2", 1)
.testStatement("1 if False else 2", 2)
.testStatement("1 + 2 if 3 + 4 else 5 + 6", 3);
-
- setFailFast(false);
- parseExpression("1 if 2");
- assertContainsError(
- "missing else clause in conditional expression or semicolon before if");
}
@Test
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 0b651fc..df33118 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.truth.Truth;
+import com.google.devtools.build.lib.events.Event;
import java.util.Arrays;
import java.util.Set;
import java.util.stream.Collectors;
@@ -52,7 +53,10 @@
private static void assertBoundNames(String assignment, String... expectedBoundNames) {
ParserInput input = ParserInput.fromLines(assignment);
- StarlarkFile file = StarlarkFile.parse(input, StarlarkThread.FAIL_FAST_HANDLER);
+ StarlarkFile file = StarlarkFile.parse(input);
+ for (Event error : file.errors()) {
+ throw new AssertionError(error);
+ }
Expression lhs = ((AssignmentStatement) file.getStatements().get(0)).getLHS();
Set<String> boundNames =
Identifier.boundIdentifiers(lhs).stream()
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java b/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
index 74bba9b..412aa59 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
@@ -15,15 +15,12 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
+import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -33,6 +30,10 @@
*/
@RunWith(JUnit4.class)
public class LexerTest {
+
+ // TODO(adonovan): make these these tests less unnecessarily stateful.
+
+ private final List<Event> errors = new ArrayList<>();
private String lastError;
private Location lastErrorLocation;
@@ -43,19 +44,10 @@
private Lexer createLexer(String input) {
PathFragment somePath = PathFragment.create("/some/path.txt");
ParserInput inputSource = ParserInput.create(input, somePath);
- Reporter reporter = new Reporter(new EventBus());
- reporter.addHandler(new EventHandler() {
- @Override
- public void handle(Event event) {
- if (EventKind.ERRORS.contains(event.getKind())) {
- lastErrorLocation = event.getLocation();
- lastError = lastErrorLocation.getPath() + ":"
- + event.getLocation().getStartLineAndColumn().getLine() + ": " + event.getMessage();
- }
- }
- });
-
- return new Lexer(inputSource, reporter);
+ errors.clear();
+ lastErrorLocation = null;
+ lastError = null;
+ return new Lexer(inputSource, errors);
}
private ArrayList<Token> allTokens(Lexer lexer) {
@@ -65,6 +57,17 @@
tok = lexer.nextToken();
result.add(tok.copy());
} while (tok.kind != TokenKind.EOF);
+
+ for (Event error : errors) {
+ lastErrorLocation = error.getLocation();
+ lastError =
+ error.getLocation().getPath()
+ + ":"
+ + error.getLocation().getStartLineAndColumn().getLine()
+ + ": "
+ + error.getMessage();
+ }
+
return result;
}
@@ -482,16 +485,16 @@
public void testContainsErrors() throws Exception {
Lexer lexerSuccess = createLexer("foo");
allTokens(lexerSuccess); // ensure the file has been completely scanned
- assertThat(lexerSuccess.containsErrors()).isFalse();
+ assertThat(errors).isEmpty();
Lexer lexerFail = createLexer("f$o");
allTokens(lexerFail);
- assertThat(lexerFail.containsErrors()).isTrue();
+ assertThat(errors).isNotEmpty();
String s = "'unterminated";
lexerFail = createLexer(s);
allTokens(lexerFail);
- assertThat(lexerFail.containsErrors()).isTrue();
+ assertThat(errors).isNotEmpty();
assertThat(lastErrorLocation.getStartOffset()).isEqualTo(0);
assertThat(lastErrorLocation.getEndOffset()).isEqualTo(s.length());
assertThat(values(tokens(s))).isEqualTo("STRING(unterminated) NEWLINE EOF");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java b/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
index 6be69bf..93b03a8 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/NodeVisitorTest.java
@@ -15,7 +15,6 @@
import static com.google.common.truth.Truth.assertThat;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import org.junit.Test;
@@ -26,13 +25,17 @@
@RunWith(JUnit4.class)
public final class NodeVisitorTest {
- private StarlarkFile parse(String... lines) throws IOException {
+ private static StarlarkFile parse(String... lines) throws SyntaxError {
ParserInput input = ParserInput.fromLines(lines);
- return StarlarkFile.parse(input, StarlarkThread.FAIL_FAST_HANDLER);
+ StarlarkFile file = StarlarkFile.parse(input);
+ if (!file.ok()) {
+ throw new SyntaxError(file.errors());
+ }
+ return file;
}
@Test
- public void everyIdentifierAndParameterIsVisitedInOrder() throws IOException {
+ public void everyIdentifierAndParameterIsVisitedInOrder() throws Exception {
final List<String> idents = new ArrayList<>();
final List<String> params = new ArrayList<>();
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 45c8655..04d41da 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -47,20 +47,31 @@
events.setFailFast(failFast);
}
- private void clearEvents() {
- events.clear();
+ // Joins the lines, parse, and returns an expression.
+ private static Expression parseExpression(String... lines) throws SyntaxError {
+ ParserInput input = ParserInput.fromLines(lines);
+ return Expression.parse(input);
}
- // Joins the lines, parse, and returns an expression.
- private Expression parseExpression(String... lines) {
- ParserInput input = ParserInput.fromLines(lines);
- return Expression.parse(input, events.reporter());
+ // Parses the expression, asserts that parsing fails,
+ // and returns the first error message.
+ private static String parseExpressionError(String src) {
+ ParserInput input = ParserInput.fromLines(src);
+ try {
+ Expression.parse(input);
+ throw new AssertionError("parseExpression(%s) succeeded unexpectedly: " + src);
+ } catch (SyntaxError ex) {
+ return ex.errors().get(0).getMessage();
+ }
}
// Joins the lines, parses, and returns a file.
+ // Errors are reported to this.events.
private StarlarkFile parseFile(String... lines) {
ParserInput input = ParserInput.fromLines(lines);
- return StarlarkFile.parse(input, events.reporter());
+ StarlarkFile file = StarlarkFile.parse(input);
+ Event.replayEventsOn(events.reporter(), file.errors());
+ return file;
}
// Joins the lines, parses, and returns the sole statement.
@@ -152,23 +163,14 @@
@Test
public void testNonAssociativeOperators() throws Exception {
- events.setFailFast(false);
-
- parseExpression("0 < 2 < 4");
- assertContainsError("Operator '<' is not associative with operator '<'");
- clearEvents();
-
- parseExpression("0 == 2 < 4");
- assertContainsError("Operator '==' is not associative with operator '<'");
- clearEvents();
-
- parseExpression("1 in [1, 2] == True");
- assertContainsError("Operator 'in' is not associative with operator '=='");
- clearEvents();
-
- parseExpression("1 >= 2 <= 3");
- assertContainsError("Operator '>=' is not associative with operator '<='");
- clearEvents();
+ assertThat(parseExpressionError("0 < 2 < 4"))
+ .contains("Operator '<' is not associative with operator '<'");
+ assertThat(parseExpressionError("0 == 2 < 4"))
+ .contains("Operator '==' is not associative with operator '<'");
+ assertThat(parseExpressionError("1 in [1, 2] == True"))
+ .contains("Operator 'in' is not associative with operator '=='");
+ assertThat(parseExpressionError("1 >= 2 <= 3"))
+ .contains("Operator '>=' is not associative with operator '<='");
}
@Test
@@ -340,7 +342,7 @@
assertLocation(0, 14, slice.getLocation());
}
- private void evalSlice(String statement, Object... expectedArgs) {
+ private static void evalSlice(String statement, Object... expectedArgs) throws SyntaxError {
SliceExpression e = (SliceExpression) parseExpression(statement);
// There is no way to evaluate the expression here, so we rely on string comparison.
@@ -357,8 +359,10 @@
public void testErrorRecovery() throws Exception {
setFailFast(false);
- String expr = "f(1, [x for foo foo foo foo], 3)";
- FuncallExpression e = (FuncallExpression) parseExpression(expr);
+ // We call parseFile, not parseExpression, as the latter is all-or-nothing.
+ String src = "f(1, [x for foo foo foo foo], 3)";
+ FuncallExpression e =
+ (FuncallExpression) ((ExpressionStatement) parseStatement(src)).getExpression();
assertContainsError("syntax error at 'foo'");
@@ -378,7 +382,7 @@
assertThat(arg1val.getName()).isEqualTo("$error$");
assertLocation(5, 29, arg1val.getLocation());
- assertThat(expr.substring(5, 28)).isEqualTo("[x for foo foo foo foo]");
+ assertThat(src.substring(5, 28)).isEqualTo("[x for foo foo foo foo]");
assertThat(arg1val.getLocation().getEndLineAndColumn().getColumn()).isEqualTo(29);
IntegerLiteral arg2 = (IntegerLiteral) e.getArguments().get(2).getValue();
@@ -387,23 +391,19 @@
@Test
public void testDoesntGetStuck() throws Exception {
- setFailFast(false);
-
// Make sure the parser does not get stuck when trying
// to parse an expression containing a syntax error.
// This usually results in OutOfMemoryError because the
// parser keeps filling up the error log.
// We need to make sure that we will always advance
// in the token stream.
- parseExpression("f(1, ], 3)");
- parseExpression("f(1, ), 3)");
- parseExpression("[ ) for v in 3)");
-
- assertContainsError(""); // "" matches any, i.e., there were some events
+ parseExpressionError("f(1, ], 3)");
+ parseExpressionError("f(1, ), 3)");
+ parseExpressionError("[ ) for v in 3)");
}
@Test
- public void testSecondaryLocation() {
+ public void testSecondaryLocation() throws SyntaxError {
String expr = "f(1 % 2)";
FuncallExpression call = (FuncallExpression) parseExpression(expr);
Argument.Passed arg = call.getArguments().get(0);
@@ -411,7 +411,7 @@
}
@Test
- public void testPrimaryLocation() {
+ public void testPrimaryLocation() throws SyntaxError {
String expr = "f(1 + 2)";
FuncallExpression call = (FuncallExpression) parseExpression(expr);
Argument.Passed arg = call.getArguments().get(0);
@@ -427,32 +427,25 @@
@Test
public void testAssignKeyword() {
- setFailFast(false);
- parseExpression("with = 4");
- assertContainsError("keyword 'with' not supported");
- assertContainsError("syntax error at 'with': expected expression");
+ assertThat(parseExpressionError("with = 4")).contains("keyword 'with' not supported");
}
@Test
public void testBreak() {
- setFailFast(false);
- parseExpression("break");
- assertContainsError("syntax error at 'break': expected expression");
+ assertThat(parseExpressionError("break"))
+ .contains("syntax error at 'break': expected expression");
}
@Test
public void testTry() {
- setFailFast(false);
- parseExpression("try: 1 + 1");
- assertContainsError("'try' not supported, all exceptions are fatal");
- assertContainsError("syntax error at 'try': expected expression");
+ assertThat(parseExpressionError("try: 1 + 1"))
+ .contains("'try' not supported, all exceptions are fatal");
}
@Test
public void testDel() {
- setFailFast(false);
- parseExpression("del d['a']");
- assertContainsError("'del' not supported, use '.pop()' to delete");
+ assertThat(parseExpressionError("del d['a']"))
+ .contains("'del' not supported, use '.pop()' to delete");
}
@Test
@@ -474,10 +467,7 @@
@Test
public void testInvalidAssign() {
- setFailFast(false);
- parseExpression("1 + (b = c)");
- assertContainsError("syntax error");
- clearEvents();
+ assertThat(parseExpressionError("1 + (b = c)")).contains("syntax error");
}
@Test
@@ -641,7 +631,7 @@
assertThat(getText(stmtStr, stmt)).isEqualTo(stmtStr);
}
- private void assertExpressionLocationCorrect(String exprStr) {
+ private static void assertExpressionLocationCorrect(String exprStr) throws SyntaxError {
Expression expr = parseExpression(exprStr);
assertThat(getText(exprStr, expr)).isEqualTo(exprStr);
// Also try it with another token at the end (newline), which broke the location in the past.
@@ -714,16 +704,9 @@
@Test
public void testTupleWithTrailingComma() throws Exception {
- setFailFast(false);
-
// Unlike Python, we require parens here.
- parseExpression("0, 1, 2, 3,");
- assertContainsError("Trailing comma");
- clearEvents();
-
- parseExpression("1 + 2,");
- assertContainsError("Trailing comma");
- clearEvents();
+ assertThat(parseExpressionError("0, 1, 2, 3,")).contains("Trailing comma");
+ assertThat(parseExpressionError("1 + 2,")).contains("Trailing comma");
ListExpression tuple = (ListExpression) parseExpression("(0, 1, 2, 3,)");
assertThat(tuple.isTuple()).isTrue();
@@ -825,31 +808,12 @@
@Test
public void testListComprehensionSyntax() throws Exception {
- setFailFast(false);
-
- parseExpression("[x for");
- assertContainsError("syntax error at 'newline'");
- clearEvents();
-
- parseExpression("[x for x");
- assertContainsError("syntax error at 'newline'");
- clearEvents();
-
- parseExpression("[x for x in");
- assertContainsError("syntax error at 'newline'");
- clearEvents();
-
- parseExpression("[x for x in []");
- assertContainsError("syntax error at 'newline'");
- clearEvents();
-
- parseExpression("[x for x for y in ['a']]");
- assertContainsError("syntax error at 'for'");
- clearEvents();
-
- parseExpression("[x for x for y in 1, 2]");
- assertContainsError("syntax error at 'for'");
- clearEvents();
+ assertThat(parseExpressionError("[x for")).contains("syntax error at 'newline'");
+ assertThat(parseExpressionError("[x for x")).contains("syntax error at 'newline'");
+ assertThat(parseExpressionError("[x for x in")).contains("syntax error at 'newline'");
+ assertThat(parseExpressionError("[x for x in []")).contains("syntax error at 'newline'");
+ assertThat(parseExpressionError("[x for x for y in ['a']]")).contains("syntax error at 'for'");
+ assertThat(parseExpressionError("[x for x for y in 1, 2]")).contains("syntax error at 'for'");
}
@Test
@@ -912,18 +876,6 @@
}
@Test
- public void testParserContainsErrorsIfSyntaxException() throws Exception {
- setFailFast(false);
- parseExpression("'foo' %%");
- assertContainsError("syntax error at '%'");
- }
-
- @Test
- public void testParserDoesNotContainErrorsIfSuccess() throws Exception {
- parseExpression("'foo'");
- }
-
- @Test
public void testParserContainsErrors() throws Exception {
setFailFast(false);
parseFile("*");
@@ -1398,4 +1350,10 @@
collectAllStringsInStringLiteralsVisitor.visit(file);
assertThat(uniqueStringInstances).containsExactly("cat", "dog", "fish");
}
+
+ @Test
+ public void testConditionalExpressions() throws Exception {
+ assertThat(parseExpressionError("1 if 2"))
+ .contains("missing else clause in conditional expression or semicolon before if");
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 19c021a..5907fd1 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -46,8 +46,10 @@
import org.junit.runners.JUnit4;
/** Tests of Starlark evaluation. */
+// This test uses 'extends' to make a copy of EvaluationTest whose
+// mode is overridden to SKYLARK, changing various environmental parameters.
@RunWith(JUnit4.class)
-public class SkylarkEvaluationTest extends EvaluationTest {
+public final class SkylarkEvaluationTest extends EvaluationTest {
@Before
public final void setup() throws Exception {
@@ -974,15 +976,25 @@
flowStatementAfterLoop("continue");
}
+ // TODO(adonovan): move this and all tests that use it to Validation tests.
+ private void assertValidationError(String expectedError, final String... lines) throws Exception {
+ SyntaxError error = assertThrows(SyntaxError.class, () -> eval(lines));
+ assertThat(error).hasMessageThat().contains(expectedError);
+ }
+
private void flowStatementInsideFunction(String statement) throws Exception {
- checkEvalErrorContains(statement + " statement must be inside a for loop",
+ assertValidationError(
+ statement + " statement must be inside a for loop",
+ //
"def foo():",
" " + statement,
"x = foo()");
}
- private void flowStatementAfterLoop(String statement) throws Exception {
- checkEvalErrorContains(statement + " statement must be inside a for loop",
+ private void flowStatementAfterLoop(String statement) throws Exception {
+ assertValidationError(
+ statement + " statement must be inside a for loop",
+ //
"def foo2():",
" for i in range(0, 3):",
" pass",
@@ -991,6 +1003,11 @@
}
@Test
+ public void testStructMembersAreImmutable() throws Exception {
+ assertValidationError("cannot assign to 's.x'", "s = struct(x = 'a')", "s.x = 'b'\n");
+ }
+
+ @Test
public void testNoneAssignment() throws Exception {
new SkylarkTest()
.setUp("def foo(x=None):", " x = 1", " x = None", " return 2", "s = foo()")
@@ -1578,17 +1595,21 @@
@Test
public void testInvalidAugmentedAssignment_ListExpression() throws Exception {
- new SkylarkTest().testIfErrorContains(
+ assertValidationError(
"cannot perform augmented assignment on a list or tuple expression",
+ //
"def f(a, b):",
" [a, b] += []",
"f(1, 2)");
}
+
@Test
public void testInvalidAugmentedAssignment_NotAnLValue() throws Exception {
- newTest().testIfErrorContains(
- "cannot assign to 'x + 1'", "x + 1 += 2");
+ assertValidationError(
+ "cannot assign to 'x + 1'",
+ //
+ "x + 1 += 2");
}
@Test
@@ -1841,12 +1862,13 @@
}
@Test
+ // TODO(adonovan): move to Validation tests.
public void testTypo() throws Exception {
- new SkylarkTest()
- .testIfErrorContains(
- "name 'my_variable' is not defined (did you mean 'myVariable'?)",
- "myVariable = 2",
- "x = my_variable + 1");
+ assertValidationError(
+ "name 'my_variable' is not defined (did you mean 'myVariable'?)",
+ //
+ "myVariable = 2",
+ "x = my_variable + 1");
}
@Test
@@ -2223,6 +2245,7 @@
}
@Test
+ // TODO(adonovan): move to Validation tests.
public void testExperimentalFlagGuardedValue() throws Exception {
// This test uses an arbitrary experimental flag to verify this functionality. If this
// experimental flag were to go away, this test may be updated to use any experimental flag.
@@ -2234,6 +2257,7 @@
"GlobalSymbol is experimental and thus unavailable with the current "
+ "flags. It may be enabled by setting --experimental_build_setting_api";
+
new SkylarkTest(ImmutableMap.of("GlobalSymbol", val), "--experimental_build_setting_api=true")
.setUp("var = GlobalSymbol")
.testLookup("var", "foo");
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
index 24a7fa0..a590be9 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
@@ -15,49 +15,44 @@
import static com.google.common.truth.Truth.assertThat;
+import com.google.common.base.Joiner;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
-import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
import com.google.devtools.build.lib.testutil.MoreAsserts;
-import com.google.devtools.build.lib.testutil.Scratch;
-import com.google.devtools.build.lib.vfs.FileSystemUtils;
-import com.google.devtools.build.lib.vfs.Path;
-import java.io.IOException;
+import com.google.devtools.build.lib.vfs.PathFragment;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
/** Unit tests for StarlarkFile. */
+// TODO(adonovan): move tests of parser into ParserTest
+// and tests of evaluator into Starlark scripts.
@RunWith(JUnit4.class)
-public class StarlarkFileTest extends EvaluationTestCase {
+public class StarlarkFileTest {
- private Scratch scratch = new Scratch();
-
- @Override
- public StarlarkThread newStarlarkThread() throws Exception {
- return newBuildStarlarkThread();
+ private static StarlarkThread newThread() {
+ return StarlarkThread.builder(Mutability.create("test")).useDefaultSemantics().build();
}
/**
- * Parses the contents of the specified string (using DUMMY_PATH as the fake filename) and returns
- * the AST. Resets the error handler beforehand.
+ * Parses the contents of the specified string (using 'foo.star' as the apparent filename) and
+ * returns the AST. Resets the error handler beforehand.
*/
- private StarlarkFile parseBuildFile(String... lines) throws IOException {
- Path file = scratch.file("/a/build/file/BUILD", lines);
- byte[] bytes = FileSystemUtils.readWithKnownFileSize(file, file.getFileSize());
- ParserInput input = ParserInput.create(bytes, file.asFragment());
- return StarlarkFile.parse(input, getEventHandler());
+ private static StarlarkFile parseFile(String... lines) {
+ String src = Joiner.on("\n").join(lines);
+ ParserInput input = ParserInput.create(src, PathFragment.create("foo.star"));
+ return StarlarkFile.parse(input);
}
@Test
- public void testParseBuildFileOK() throws Exception {
- StarlarkFile buildfile =
- parseBuildFile(
+ public void testExecuteBuildFileOK() throws Exception {
+ StarlarkFile file =
+ parseFile(
"# a file in the build language",
"",
"x = [1,2,'foo',4] + [1,2, \"%s%d\" % ('foo', 1)]");
-
- assertThat(buildfile.exec(thread, getEventHandler())).isTrue();
+ StarlarkThread thread = newThread();
+ EvalUtils.exec(file, thread);
// Test final environment is correctly modified:
//
@@ -68,74 +63,48 @@
}
@Test
- public void testEvalException() throws Exception {
- setFailFast(false);
- StarlarkFile buildfile = parseBuildFile("x = 1", "y = [2,3]", "", "z = x + y");
+ public void testExecException() throws Exception {
+ StarlarkFile file = parseFile("x = 1", "y = [2,3]", "", "z = x + y");
- assertThat(buildfile.exec(thread, getEventHandler())).isFalse();
- Event e = assertContainsError("unsupported operand type(s) for +: 'int' and 'list'");
- assertThat(e.getLocation().getStartLineAndColumn().getLine()).isEqualTo(4);
+ StarlarkThread thread = newThread();
+ try {
+ EvalUtils.exec(file, thread);
+ throw new AssertionError("execution succeeded unexpectedly");
+ } catch (EvalException ex) {
+ assertThat(ex.getMessage()).contains("unsupported operand type(s) for +: 'int' and 'list'");
+ assertThat(ex.getLocation().getStartLineAndColumn().getLine()).isEqualTo(4);
+ }
}
@Test
public void testParsesFineWithNewlines() throws Exception {
- StarlarkFile buildFileAST = parseBuildFile("foo()", "bar()", "something = baz()", "bar()");
- assertThat(buildFileAST.getStatements()).hasSize(4);
+ StarlarkFile file = parseFile("foo()", "bar()", "something = baz()", "bar()");
+ assertThat(file.getStatements()).hasSize(4);
}
@Test
public void testFailsIfNewlinesAreMissing() throws Exception {
- setFailFast(false);
+ StarlarkFile file = parseFile("foo() bar() something = baz() bar()");
- StarlarkFile buildFileAST = parseBuildFile("foo() bar() something = baz() bar()");
-
- Event event = assertContainsError("syntax error at \'bar\': expected newline");
- assertThat(event.getLocation().getPath().toString()).isEqualTo("/a/build/file/BUILD");
- assertThat(event.getLocation().getStartLineAndColumn().getLine()).isEqualTo(1);
- assertThat(buildFileAST.containsErrors()).isTrue();
+ Event event =
+ MoreAsserts.assertContainsEvent(file.errors(), "syntax error at \'bar\': expected newline");
+ assertThat(event.getLocation().toString()).isEqualTo("foo.star:1:7");
}
@Test
public void testImplicitStringConcatenationFails() throws Exception {
- setFailFast(false);
- StarlarkFile buildFileAST = parseBuildFile("a = 'foo' 'bar'");
- Event event = assertContainsError(
- "Implicit string concatenation is forbidden, use the + operator");
- assertThat(event.getLocation().getPath().toString()).isEqualTo("/a/build/file/BUILD");
- assertThat(event.getLocation().getStartLineAndColumn().getLine()).isEqualTo(1);
- assertThat(event.getLocation().getStartLineAndColumn().getColumn()).isEqualTo(10);
- assertThat(buildFileAST.containsErrors()).isTrue();
+ StarlarkFile file = parseFile("a = 'foo' 'bar'");
+ Event event =
+ MoreAsserts.assertContainsEvent(
+ file.errors(), "Implicit string concatenation is forbidden, use the + operator");
+ assertThat(event.getLocation().toString()).isEqualTo("foo.star:1:10");
}
@Test
public void testImplicitStringConcatenationAcrossLinesIsIllegal() throws Exception {
- setFailFast(false);
- StarlarkFile buildFileAST = parseBuildFile("a = 'foo'\n 'bar'");
+ StarlarkFile file = parseFile("a = 'foo'\n 'bar'");
- Event event = assertContainsError("indentation error");
- assertThat(event.getLocation().getPath().toString()).isEqualTo("/a/build/file/BUILD");
- assertThat(event.getLocation().getStartLineAndColumn().getLine()).isEqualTo(2);
- assertThat(event.getLocation().getStartLineAndColumn().getColumn()).isEqualTo(2);
- assertThat(buildFileAST.containsErrors()).isTrue();
- }
-
- @Test
- public void testWithSyntaxErrorsDoesNotPrintDollarError() throws Exception {
- setFailFast(false);
- StarlarkFile buildFile =
- parseBuildFile(
- "abi = '$(ABI)-glibc-' + glibc_version + '-' + $(TARGET_CPU) + '-linux'",
- "libs = [abi + opt_level + '/lib/libcc.a']",
- "shlibs = [abi + opt_level + '/lib/libcc.so']",
- "+* shlibs", // syntax error at '+'
- "cc_library(name = 'cc',",
- " srcs = libs,",
- " includes = [ abi + opt_level + '/include' ])");
- assertThat(buildFile.containsErrors()).isTrue();
- assertContainsError("syntax error at '*': expected expression");
- assertThat(buildFile.exec(thread, getEventHandler())).isFalse();
- MoreAsserts.assertDoesNotContainEvent(getEventCollector(), "$error$");
- // This message should not be printed anymore.
- MoreAsserts.assertDoesNotContainEvent(getEventCollector(), "contains syntax error(s)");
+ Event event = MoreAsserts.assertContainsEvent(file.errors(), "indentation error");
+ assertThat(event.getLocation().toString()).isEqualTo("foo.star:2:2");
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
index 6d5aece..ffb9f37 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
@@ -191,7 +191,7 @@
StarlarkThread thread = newStarlarkThread();
thread.update("a", 1);
- Object result = thread.debugEval(ParserInput.create("a", null));
+ Object result = thread.debugEval(Expression.parse(ParserInput.create("a", null)));
assertThat(result).isEqualTo(1);
}
@@ -202,7 +202,9 @@
thread.update("a", 1);
EvalException e =
- assertThrows(EvalException.class, () -> thread.debugEval(ParserInput.create("b", null)));
+ assertThrows(
+ EvalException.class,
+ () -> thread.debugEval(Expression.parse(ParserInput.create("b", null))));
assertThat(e).hasMessageThat().isEqualTo("name 'b' is not defined");
}
@@ -211,13 +213,9 @@
StarlarkThread thread = newStarlarkThread();
thread.update("a", "string");
- Object result =
- thread.debugEval(ParserInput.create("a.startswith('str')", PathFragment.EMPTY_FRAGMENT));
- assertThat(result).isEqualTo(Boolean.TRUE);
-
- thread.debugExec(ParserInput.create("a = 1", PathFragment.EMPTY_FRAGMENT));
-
- result = thread.debugEval(ParserInput.create("a", PathFragment.EMPTY_FRAGMENT));
- assertThat(result).isEqualTo(1);
+ assertThat(thread.debugEval(Expression.parse(ParserInput.fromLines("a.startswith('str')"))))
+ .isEqualTo(true);
+ thread.debugExec(ParserInput.fromLines("a = 1"));
+ assertThat(thread.debugEval(Expression.parse(ParserInput.fromLines("a")))).isEqualTo(1);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadTest.java
index a3746ee..f12b59a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadTest.java
@@ -63,7 +63,7 @@
@Test
public void testReference() throws Exception {
setFailFast(false);
- EvalException e = assertThrows(EvalException.class, () -> eval("foo"));
+ SyntaxError e = assertThrows(SyntaxError.class, () -> eval("foo"));
assertThat(e).hasMessageThat().isEqualTo("name 'foo' is not defined");
update("foo", "bar");
assertThat(eval("foo")).isEqualTo("bar");
@@ -72,8 +72,7 @@
// Test assign and reference through interpreter:
@Test
public void testAssignAndReference() throws Exception {
- setFailFast(false);
- EvalException e = assertThrows(EvalException.class, () -> eval("foo"));
+ SyntaxError e = assertThrows(SyntaxError.class, () -> eval("foo"));
assertThat(e).hasMessageThat().isEqualTo("name 'foo' is not defined");
eval("foo = 'bar'");
assertThat(eval("foo")).isEqualTo("bar");
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 35425cc..3836db8 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
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.StarlarkThread.FailFastException;
import com.google.devtools.build.lib.syntax.Statement;
+import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.syntax.ValidationEnvironment;
import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestMode;
@@ -152,12 +153,17 @@
protected final StarlarkFile parseStarlarkFileWithoutValidation(String... lines) {
ParserInput input = ParserInput.fromLines(lines);
- return StarlarkFile.parse(input, getEventHandler());
+ StarlarkFile file = StarlarkFile.parse(input);
+ Event.replayEventsOn(getEventHandler(), file.errors());
+ return file;
}
private StarlarkFile parseStarlarkFile(String... lines) {
- StarlarkFile ast = parseStarlarkFileWithoutValidation(lines);
- return ast.validate(thread, /*isBuildFile=*/ false, getEventHandler());
+ ParserInput input = ParserInput.fromLines(lines);
+ StarlarkFile file = StarlarkFile.parse(input);
+ ValidationEnvironment.validateFile(file, thread, /*isBuildFile=*/ false);
+ Event.replayEventsOn(getEventHandler(), file.errors());
+ return file;
}
/** Parses and validates a file and returns its statements. */
@@ -174,8 +180,8 @@
}
/** Parses an expression. */
- protected final Expression parseExpression(String... lines) {
- return Expression.parse(ParserInput.fromLines(lines), getEventHandler());
+ protected final Expression parseExpression(String... lines) throws SyntaxError {
+ return Expression.parse(ParserInput.fromLines(lines));
}
public EvaluationTestCase update(String varname, Object value) throws Exception {
@@ -187,26 +193,34 @@
return thread.moduleLookup(varname);
}
+ // TODO(adonovan): this function does far too much:
+ // - two modes, BUILD vs Skylark
+ // - parse + validate + BUILD dialect checks + execute.
+ // Break the tests down into tests of just the scanner, parser, validator, build dialect checks,
+ // or execution, and assert that all passes except the one of interest succeed.
+ // All BUILD-dialect stuff belongs in bazel proper (lib.packages), not here.
public Object eval(String... lines) throws Exception {
ParserInput input = ParserInput.fromLines(lines);
- if (testMode == TestMode.SKYLARK) {
- // TODO(adonovan): inline this call and factor with 'else' case.
- return StarlarkFile.eval(input, thread);
- } else {
- StarlarkFile file = StarlarkFile.parse(input, thread.getEventHandler());
- if (ValidationEnvironment.validateFile(
- file, thread, /*isBuildFile=*/ true, thread.getEventHandler())) {
- PackageFactory.checkBuildSyntax(file, thread.getEventHandler());
+ StarlarkFile file = StarlarkFile.parse(input);
+ ValidationEnvironment.validateFile(file, thread, testMode == TestMode.BUILD);
+ if (testMode == TestMode.SKYLARK) { // .bzl and other dialects
+ if (!file.ok()) {
+ throw new SyntaxError(file.errors());
}
- return file.eval(thread);
+ } else {
+ // For BUILD mode, validation events are reported but don't (yet)
+ // prevent execution. We also apply BUILD dialect syntax checks.
+ Event.replayEventsOn(getEventHandler(), file.errors());
+ PackageFactory.checkBuildSyntax(file, getEventHandler());
}
+ return file.eval(thread);
}
public void checkEvalError(String msg, String... input) throws Exception {
try {
eval(input);
fail("Expected error '" + msg + "' but got no error");
- } catch (EvalException | FailFastException e) {
+ } catch (SyntaxError | EvalException | FailFastException e) {
assertThat(e).hasMessageThat().isEqualTo(msg);
}
}
@@ -215,7 +229,7 @@
try {
eval(input);
fail("Expected error containing '" + msg + "' but got no error");
- } catch (EvalException | FailFastException e) {
+ } catch (SyntaxError | EvalException | FailFastException e) {
assertThat(e).hasMessageThat().contains(msg);
}
}
@@ -223,7 +237,7 @@
public void checkEvalErrorDoesNotContain(String msg, String... input) throws Exception {
try {
eval(input);
- } catch (EvalException | FailFastException e) {
+ } catch (SyntaxError | EvalException | FailFastException e) {
assertThat(e).hasMessageThat().doesNotContain(msg);
}
}
