Refactor Skylark Environment-s
Make Environment-s freezable: Introduce a class Mutability
as a revokable capability to mutate objects in an Environment.
For now, only Environment-s carry this capability.
Make sure that every Mutability is revoked in the same function that create...
This reinstates a change that previously rolled-back because it broke the
serializability of SkylarkLookupValue. Bad news: serializing it succeeds for the
wrong reason, because a SkylarkEnvironment was stored as a result (now an
Environment.Extension) that was Serializable but inherited its bindings from an Environment (now an Environment.BaseExtension) which wasn't Serializable.
Apparently, Java doesn't try to serialize the bindings then (or at least doesn't
error out when it fails), because these bindings map variable names to pretty
arbitrary objects, and a lot of those we find in practice aren't Serializable.
Thus the current code passes the same tests as the previous code, but obviously
the serialization is just as ineffective as it used to be.
--
MOS_MIGRATED_REVID=102776694
diff --git a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
index aa8d8f8..c7f85d6 100644
--- a/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/events/util/EventCollectionApparatus.java
@@ -18,7 +18,7 @@
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.PrintingEventHandler;
import com.google.devtools.build.lib.events.Reporter;
-import com.google.devtools.build.lib.syntax.EvaluationContext;
+import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.util.io.OutErr;
@@ -67,9 +67,9 @@
*/
public void setFailFast(boolean failFast) {
if (failFast) {
- reporter.addHandler(EvaluationContext.FAIL_FAST_HANDLER);
+ reporter.addHandler(Environment.FAIL_FAST_HANDLER);
} else {
- reporter.removeHandler(EvaluationContext.FAIL_FAST_HANDLER);
+ reporter.removeHandler(Environment.FAIL_FAST_HANDLER);
}
}
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 24ac455..6306e46 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
@@ -30,9 +30,9 @@
import com.google.devtools.build.lib.packages.PackageFactory.LegacyGlobber;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.syntax.BuildFileAST;
+import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.syntax.ParserInputSource;
-import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.testutil.TestUtils;
@@ -134,7 +134,7 @@
LegacyBuilder resultBuilder = factory.evaluateBuildFile(
externalPkg, packageId, buildFileAST, buildFile,
globber, ImmutableList.<Event>of(), ConstantRuleVisibility.PUBLIC, false, false,
- new MakeEnvironment.Builder(), ImmutableMap.<PathFragment, SkylarkEnvironment>of(),
+ new MakeEnvironment.Builder(), ImmutableMap.<PathFragment, Extension>of(),
ImmutableList.<Label>of());
Package result = resultBuilder.build();
Event.replayEventsOn(events.reporter(), result.getEvents());
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
index eff5bad..00e9062 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java
@@ -22,11 +22,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventCollector;
-import com.google.devtools.build.lib.events.EventKind;
-import com.google.devtools.build.lib.events.Reporter;
-import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.packages.CachingPackageLocator;
-import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.testutil.Scratch;
import com.google.devtools.build.lib.vfs.Path;
@@ -37,13 +33,14 @@
import java.io.IOException;
import java.util.Arrays;
+/**
+ * Unit tests for BuildFileAST.
+ */
@RunWith(JUnit4.class)
-public class BuildFileASTTest {
+public class BuildFileASTTest extends EvaluationTestCase {
private Scratch scratch = new Scratch();
- private EventCollectionApparatus events = new EventCollectionApparatus(EventKind.ALL_EVENTS);
-
private class ScratchPathPackageLocator implements CachingPackageLocator {
@Override
public Path getBuildFileForPackage(PackageIdentifier packageName) {
@@ -53,13 +50,18 @@
private CachingPackageLocator locator = new ScratchPathPackageLocator();
+ @Override
+ public Environment newEnvironment() throws Exception {
+ return newBuildEnvironment();
+ }
+
/**
* Parses the contents of the specified string (using DUMMY_PATH as the fake
* filename) and returns the AST. Resets the error handler beforehand.
*/
private BuildFileAST parseBuildFile(String... lines) throws IOException {
Path file = scratch.file("/a/build/file/BUILD", lines);
- return BuildFileAST.parseBuildFile(file, events.reporter(), locator, false);
+ return BuildFileAST.parseBuildFile(file, getEventHandler(), locator, false);
}
@Test
@@ -69,11 +71,9 @@
"",
"x = [1,2,'foo',4] + [1,2, \"%s%d\" % ('foo', 1)]");
- Environment env = new Environment();
- Reporter reporter = new Reporter();
- BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, reporter, null, false);
+ BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false);
- assertTrue(buildfile.exec(env, reporter));
+ assertTrue(buildfile.exec(env, getEventHandler()));
// Test final environment is correctly modified:
//
@@ -91,15 +91,11 @@
"",
"z = x + y");
- Environment env = new Environment();
- Reporter reporter = new Reporter();
- EventCollector collector = new EventCollector(EventKind.ALL_EVENTS);
- reporter.addHandler(collector);
- BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, reporter, null, false);
+ setFailFast(false);
+ BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false);
- assertFalse(buildfile.exec(env, reporter));
- Event e = MoreAsserts.assertContainsEvent(collector,
- "unsupported operand type(s) for +: 'int' and 'List'");
+ assertFalse(buildfile.exec(env, getEventHandler()));
+ Event e = assertContainsEvent("unsupported operand type(s) for +: 'int' and 'List'");
assertEquals(4, e.getLocation().getStartLineAndColumn().getLine());
}
@@ -114,13 +110,12 @@
@Test
public void testFailsIfNewlinesAreMissing() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST =
parseBuildFile("foo() bar() something = baz() bar()");
- Event event = events.collector().iterator().next();
- assertEquals("syntax error at \'bar\': expected newline", event.getMessage());
+ Event event = assertContainsEvent("syntax error at \'bar\': expected newline");
assertEquals("/a/build/file/BUILD",
event.getLocation().getPath().toString());
assertEquals(1, event.getLocation().getStartLineAndColumn().getLine());
@@ -129,11 +124,10 @@
@Test
public void testImplicitStringConcatenationFails() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST = parseBuildFile("a = 'foo' 'bar'");
- Event event = events.collector().iterator().next();
- assertEquals("Implicit string concatenation is forbidden, use the + operator",
- event.getMessage());
+ Event event = assertContainsEvent(
+ "Implicit string concatenation is forbidden, use the + operator");
assertEquals("/a/build/file/BUILD",
event.getLocation().getPath().toString());
assertEquals(1, event.getLocation().getStartLineAndColumn().getLine());
@@ -143,11 +137,10 @@
@Test
public void testImplicitStringConcatenationAcrossLinesIsIllegal() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST = parseBuildFile("a = 'foo'\n 'bar'");
- Event event = events.collector().iterator().next();
- assertEquals("indentation error", event.getMessage());
+ Event event = assertContainsEvent("indentation error");
assertEquals("/a/build/file/BUILD",
event.getLocation().getPath().toString());
assertEquals(2, event.getLocation().getStartLineAndColumn().getLine());
@@ -172,7 +165,7 @@
@Test
public void testWithSyntaxErrorsDoesNotPrintDollarError() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFile = parseBuildFile(
"abi = cxx_abi + '-glibc-' + glibc_version + '-' + generic_cpu + '-' + sysname",
"libs = [abi + opt_level + '/lib/libcc.a']",
@@ -182,14 +175,11 @@
" srcs = libs,",
" includes = [ abi + opt_level + '/include' ])");
assertTrue(buildFile.containsErrors());
- Event event = events.collector().iterator().next();
- assertEquals("syntax error at '+': expected expression", event.getMessage());
- Environment env = new Environment();
- assertFalse(buildFile.exec(env, events.reporter()));
- assertNull(findEvent(events.collector(), "$error$"));
+ assertContainsEvent("syntax error at '+': expected expression");
+ assertFalse(buildFile.exec(env, getEventHandler()));
+ assertNull(findEvent(getEventCollector(), "$error$"));
// This message should not be printed anymore.
- Event event2 = findEvent(events.collector(), "contains syntax error(s)");
- assertNull(event2);
+ assertNull(findEvent(getEventCollector(), "contains syntax error(s)"));
}
@Test
@@ -202,7 +192,7 @@
+ "include(\"//foo/bar:BUILD\")\n"
+ "b = 4\n");
- BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
+ BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(),
locator, false);
assertFalse(buildFileAST.containsErrors());
@@ -217,15 +207,14 @@
"include(\"//foo/bar:defs\")\n"
+ "b = a + 1\n");
- BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
+ BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(),
locator, false);
assertFalse(buildFileAST.containsErrors());
assertThat(buildFileAST.getStatements()).hasSize(3);
- Environment env = new Environment();
- Reporter reporter = new Reporter();
- assertFalse(buildFileAST.exec(env, reporter));
+ setFailFast(false);
+ assertFalse(buildFileAST.exec(env, getEventHandler()));
assertEquals(2, env.lookup("b"));
}
@@ -247,63 +236,53 @@
assertFalse(buildFileAST.containsErrors());
assertThat(buildFileAST.getStatements()).hasSize(8);
- Environment env = new Environment();
- Reporter reporter = new Reporter();
- assertFalse(buildFileAST.exec(env, reporter));
+ setFailFast(false);
+ assertFalse(buildFileAST.exec(env, getEventHandler()));
assertEquals(5, env.lookup("b"));
assertEquals(7, env.lookup("c"));
}
@Test
public void testFailInclude() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST = parseBuildFile("include(\"//nonexistent\")");
assertThat(buildFileAST.getStatements()).hasSize(1);
- events.assertContainsEvent("Include of '//nonexistent' failed");
+ assertContainsEvent("Include of '//nonexistent' failed");
}
- private class EmptyPackageLocator implements CachingPackageLocator {
- @Override
- public Path getBuildFileForPackage(PackageIdentifier packageName) {
- return null;
- }
- }
-
- private CachingPackageLocator emptyLocator = new EmptyPackageLocator();
-
@Test
public void testFailInclude2() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
Path buildFile = scratch.file("/foo/bar/BUILD",
"include(\"//nonexistent:foo\")\n");
- BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, events.reporter(),
- emptyLocator, false);
+ BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(
+ buildFile, getEventHandler(), Environment.EMPTY_PACKAGE_LOCATOR, false);
assertThat(buildFileAST.getStatements()).hasSize(1);
- events.assertContainsEvent("Package 'nonexistent' not found");
+ assertContainsEvent("Package 'nonexistent' not found");
}
@Test
public void testInvalidInclude() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST = parseBuildFile("include(2)");
assertThat(buildFileAST.getStatements()).isEmpty();
- events.assertContainsEvent("syntax error at '2'");
+ assertContainsEvent("syntax error at '2'");
}
@Test
public void testRecursiveInclude() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
Path buildFile = scratch.file("/foo/bar/BUILD",
"include(\"//foo/bar:BUILD\")\n");
- BuildFileAST.parseBuildFile(buildFile, events.reporter(), locator, false);
- events.assertContainsEvent("Recursive inclusion");
+ BuildFileAST.parseBuildFile(buildFile, getEventHandler(), locator, false);
+ assertContainsEvent("Recursive inclusion");
}
@Test
public void testParseErrorInclude() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
scratch.file("/foo/bar/file",
"a = 2 + % 3\n"); // parse error
@@ -311,14 +290,13 @@
parseBuildFile("include(\"//foo/bar:file\")");
// Check the location is properly reported
- Event event = events.collector().iterator().next();
+ Event event = assertContainsEvent("syntax error at '%': expected expression");
assertEquals("/foo/bar/file:1:9", event.getLocation().print());
- assertEquals("syntax error at '%': expected expression", event.getMessage());
}
@Test
public void testNonExistentIncludeReported() throws Exception {
- events.setFailFast(false);
+ setFailFast(false);
BuildFileAST buildFileAST = parseBuildFile("include('//foo:bar')");
assertThat(buildFileAST.getStatements()).hasSize(1);
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index a324b13..0202124 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -31,8 +31,8 @@
public class EnvironmentTest extends EvaluationTestCase {
@Override
- public EvaluationContext newEvaluationContext() {
- return EvaluationContext.newBuildContext(getEventHandler());
+ public Environment newEnvironment() {
+ return newBuildEnvironment();
}
// Test the API directly
@@ -104,25 +104,41 @@
@Test
public void testGetVariableNames() throws Exception {
- update("foo", "bar");
- update("wiz", 3);
+ Environment outerEnv;
+ Environment innerEnv;
+ try (Mutability mut = Mutability.create("outer")) {
+ outerEnv = Environment.builder(mut)
+ .setGlobals(Environment.BUILD).build()
+ .update("foo", "bar")
+ .update("wiz", 3);
+ }
+ try (Mutability mut = Mutability.create("inner")) {
+ innerEnv = Environment.builder(mut)
+ .setGlobals(outerEnv.getGlobals()).build()
+ .update("foo", "bat")
+ .update("quux", 42);
+ }
- Environment nestedEnv = new Environment(getEnvironment());
- nestedEnv.update("foo", "bat");
- nestedEnv.update("quux", 42);
-
- assertEquals(Sets.newHashSet("True", "False", "None", "foo", "wiz"),
- getEnvironment().getVariableNames());
- assertEquals(Sets.newHashSet("True", "False", "None", "foo", "wiz", "quux"),
- nestedEnv.getVariableNames());
+ assertEquals(Sets.newHashSet("foo", "wiz",
+ "False", "None", "True",
+ "-", "bool", "dict", "enumerate", "int", "len", "list",
+ "range", "repr", "select", "sorted", "str", "zip"),
+ outerEnv.getVariableNames());
+ assertEquals(Sets.newHashSet("foo", "wiz", "quux",
+ "False", "None", "True",
+ "-", "bool", "dict", "enumerate", "int", "len", "list",
+ "range", "repr", "select", "sorted", "str", "zip"),
+ innerEnv.getVariableNames());
}
@Test
public void testToString() throws Exception {
update("subject", new StringLiteral("Hello, 'world'.", '\''));
update("from", new StringLiteral("Java", '"'));
- assertEquals("Environment{False -> false, None -> None, True -> true, from -> \"Java\", "
- + "subject -> 'Hello, \\'world\\'.', }", getEnvironment().toString());
+ assertThat(getEnvironment().toString())
+ .startsWith("Environment(lexicalFrame=null, "
+ + "globalFrame=Frame[test]{\"from\": \"Java\", \"subject\": 'Hello, \\'world\\'.'}=>"
+ + "(BUILD){");
}
@Test
@@ -134,4 +150,58 @@
assertThat(e).hasMessage("update(value == null)");
}
}
+
+ @Test
+ public void testFrozen() throws Exception {
+ Environment env;
+ try (Mutability mutability = Mutability.create("testFrozen")) {
+ env = Environment.builder(mutability)
+ .setGlobals(Environment.BUILD).setEventHandler(Environment.FAIL_FAST_HANDLER).build();
+ env.update("x", 1);
+ assertEquals(env.lookup("x"), 1);
+ env.update("y", 2);
+ assertEquals(env.lookup("y"), 2);
+ assertEquals(env.lookup("x"), 1);
+ env.update("x", 3);
+ assertEquals(env.lookup("x"), 3);
+ }
+ try {
+ // This update to an existing variable should fail because the environment was frozen.
+ env.update("x", 4);
+ throw new Exception("failed to fail"); // not an AssertionError like fail()
+ } catch (AssertionError e) {
+ assertThat(e).hasMessage("Can't update x to 4 in frozen environment");
+ }
+ try {
+ // This update to a new variable should also fail because the environment was frozen.
+ env.update("newvar", 5);
+ throw new Exception("failed to fail"); // not an AssertionError like fail()
+ } catch (AssertionError e) {
+ assertThat(e).hasMessage("Can't update newvar to 5 in frozen environment");
+ }
+ }
+
+ @Test
+ public void testReadOnly() throws Exception {
+ Environment env = newSkylarkEnvironment()
+ .setup("special_var", 42)
+ .update("global_var", 666);
+
+ // We don't even get a runtime exception trying to modify these,
+ // because we get compile-time exceptions even before we reach runtime!
+ try {
+ env.eval("special_var = 41");
+ throw new AssertionError("failed to fail");
+ } catch (IllegalArgumentException e) {
+ assertThat(e).hasMessage("ERROR 1:1: Variable special_var is read only");
+ }
+
+ try {
+ env.eval("def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
+ throw new AssertionError("failed to fail");
+ } catch (EvalExceptionWithStackTrace e) {
+ assertThat(e.getMessage()).contains("Variable 'global_var' is referenced before assignment. "
+ + "The variable is defined in the global scope.");
+ }
+ }
}
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 cf23858..83a55be 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
@@ -482,9 +482,9 @@
@Test
public void testDictComprehensions_ToString() throws Exception {
assertEquals("{x: x for x in [1, 2]}",
- evaluationContext.parseExpression("{x : x for x in [1, 2]}").toString());
+ parseExpression("{x : x for x in [1, 2]}").toString());
assertEquals("{x + 'a': x for x in [1, 2]}",
- evaluationContext.parseExpression("{x + 'a' : x for x in [1, 2]}").toString());
+ parseExpression("{x + 'a' : x for x in [1, 2]}").toString());
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
index b2d661b..e26234a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java
@@ -35,48 +35,76 @@
* Base class for test cases that use parsing and evaluation services.
*/
public class EvaluationTestCase {
-
+
private EventCollectionApparatus eventCollectionApparatus;
private PackageFactory factory;
private TestMode testMode = TestMode.SKYLARK;
-
- protected EvaluationContext evaluationContext;
+ protected Environment env;
+ protected Mutability mutability = Mutability.create("test");
public EvaluationTestCase() {
createNewInfrastructure();
}
-
+
@Before
public void setUp() throws Exception {
createNewInfrastructure();
- evaluationContext = newEvaluationContext();
+ env = newEnvironment();
}
- public EvaluationContext newEvaluationContext() throws Exception {
+ /**
+ * Creates a standard Environment for tests in the BUILD language.
+ * No PythonPreprocessing, mostly empty mutable Environment.
+ */
+ public Environment newBuildEnvironment() {
+ return Environment.builder(mutability)
+ .setGlobals(Environment.BUILD)
+ .setEventHandler(getEventHandler())
+ .setLoadingPhase()
+ .build();
+ }
+
+ /**
+ * Creates an Environment for Skylark with a mostly empty initial environment.
+ * For internal initialization or tests.
+ */
+ public Environment newSkylarkEnvironment() {
+ return Environment.builder(mutability)
+ .setSkylark()
+ .setGlobals(Environment.SKYLARK)
+ .setEventHandler(getEventHandler())
+ .build();
+ }
+
+ /**
+ * Creates a new Environment suitable for the test case. Subclasses may override it
+ * to fit their purpose and e.g. call newBuildEnvironment or newSkylarkEnvironment;
+ * or they may play with the testMode to run tests in either or both kinds of Environment.
+ * Note that all Environment-s may share the same Mutability, so don't close it.
+ * @return a fresh Environment.
+ */
+ public Environment newEnvironment() throws Exception {
if (testMode == null) {
throw new IllegalArgumentException(
"TestMode is null. Please set a Testmode via setMode() or set the "
- + "evaluatenContext manually by overriding newEvaluationContext()");
+ + "Environment manually by overriding newEnvironment()");
}
-
- return testMode.createContext(getEventHandler(), factory.getEnvironment());
+ return testMode.createEnvironment(getEventHandler(), null);
}
protected void createNewInfrastructure() {
eventCollectionApparatus = new EventCollectionApparatus(EventKind.ALL_EVENTS);
factory = new PackageFactory(TestRuleClassProvider.getRuleClassProvider());
}
-
+
/**
- * Sets the specified {@code TestMode} and tries to create the appropriate {@code
- * EvaluationContext}
- *
+ * Sets the specified {@code TestMode} and tries to create the appropriate {@code Environment}
* @param testMode
* @throws Exception
*/
protected void setMode(TestMode testMode) throws Exception {
this.testMode = testMode;
- evaluationContext = newEvaluationContext();
+ env = newEnvironment();
}
protected void enableSkylarkMode() throws Exception {
@@ -96,32 +124,33 @@
}
public Environment getEnvironment() {
- return evaluationContext.getEnvironment();
+ return env;
}
-
+
public boolean isSkylark() {
- return evaluationContext.isSkylark();
+ return env.isSkylark();
}
protected List<Statement> parseFile(String... input) {
- return evaluationContext.parseFile(input);
+ return env.parseFile(input);
}
+ /** Parses an Expression from string without a supporting file */
Expression parseExpression(String... input) {
- return evaluationContext.parseExpression(input);
+ return Parser.parseExpression(env.createLexer(input), getEventHandler());
}
public EvaluationTestCase update(String varname, Object value) throws Exception {
- evaluationContext.update(varname, value);
+ env.update(varname, value);
return this;
}
public Object lookup(String varname) throws Exception {
- return evaluationContext.lookup(varname);
+ return env.lookup(varname);
}
public Object eval(String... input) throws Exception {
- return evaluationContext.eval(input);
+ return env.eval(input);
}
public void checkEvalError(String msg, String... input) throws Exception {
@@ -189,15 +218,14 @@
/**
* Base class for test cases that run in specific modes (e.g. Build and/or Skylark)
- *
- */
+ */
protected abstract class ModalTestCase {
private final SetupActions setup;
-
- protected ModalTestCase() {
+
+ protected ModalTestCase() {
setup = new SetupActions();
}
-
+
/**
* Allows the execution of several statements before each following test
* @param statements The statement(s) to be executed
@@ -218,7 +246,7 @@
setup.registerUpdate(name, value);
return this;
}
-
+
/**
* Evaluates two parameters and compares their results.
* @param statement The statement to be evaluated
@@ -255,7 +283,7 @@
runTest(collectionTestable(statement, false, items));
return this;
}
-
+
/**
* Evaluates the given statement and compares its result to the collection of expected objects
* while considering their order
@@ -409,7 +437,7 @@
}
/**
- * A simple decorator that allows the execution of setup actions before running
+ * A simple decorator that allows the execution of setup actions before running
* a {@code Testable}
*/
class TestableDecorator implements Testable {
@@ -480,7 +508,7 @@
}
}
}
-
+
/**
* A class that executes each separate test in both modes (Build and Skylark)
*/
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 b5a8015..75913b4 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
@@ -32,42 +32,44 @@
import java.util.LinkedList;
import java.util.List;
-
/**
* Tests of parser behaviour.
*/
@RunWith(JUnit4.class)
public class ParserTest extends EvaluationTestCase {
- EvaluationContext buildContext;
- EvaluationContext buildContextWithPython;
+ Environment buildEnvironment;
@Before
@Override
public void setUp() throws Exception {
super.setUp();
- buildContext = EvaluationContext.newBuildContext(getEventHandler());
- buildContextWithPython = EvaluationContext.newBuildContext(
- getEventHandler(), new Environment(), /*parsePython*/true);
+ buildEnvironment = newBuildEnvironment();
}
private Parser.ParseResult parseFileWithComments(String... input) {
- return buildContext.parseFileWithComments(input);
- }
- @Override
- protected List<Statement> parseFile(String... input) {
- return buildContext.parseFile(input);
- }
- private List<Statement> parseFileWithPython(String... input) {
- return buildContextWithPython.parseFile(input);
- }
- private List<Statement> parseFileForSkylark(String... input) {
- return evaluationContext.parseFile(input);
- }
- private Statement parseStatement(String... input) {
- return buildContext.parseStatement(input);
+ return buildEnvironment.parseFileWithComments(input);
}
+ /** Parses build code (not Skylark) */
+ @Override
+ protected List<Statement> parseFile(String... input) {
+ return buildEnvironment.parseFile(input);
+ }
+
+ /** Parses a build code (not Skylark) with PythonProcessing enabled */
+ private List<Statement> parseFileWithPython(String... input) {
+ return Parser.parseFile(
+ buildEnvironment.createLexer(input),
+ getEventHandler(),
+ Environment.EMPTY_PACKAGE_LOCATOR,
+ /*parsePython=*/true).statements;
+ }
+
+ /** Parses Skylark code */
+ private List<Statement> parseFileForSkylark(String... input) {
+ return env.parseFile(input);
+ }
private static String getText(String text, ASTNode node) {
return text.substring(node.getLocation().getStartOffset(),
@@ -707,7 +709,7 @@
@Test
public void testParserContainsErrors() throws Exception {
setFailFast(false);
- parseStatement("+");
+ parseFile("+");
assertContainsEvent("syntax error at '+'");
}
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 88a2e31..8668f50 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
@@ -533,7 +533,7 @@
new SkylarkTest()
.update("mock", new Mock())
.testIfExactError("Keyword arguments are not allowed when calling a java method"
- + "\nwhile calling method 'string' on object of type Mock",
+ + "\nwhile calling method 'string' for type Mock",
"mock.string(key=True)");
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index 86ad1fb..7558b6c 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -45,8 +45,8 @@
SkylarkList.lazyList(new CustomIterable(), Integer.class);
@Override
- public EvaluationContext newEvaluationContext() throws Exception {
- return super.newEvaluationContext().update("lazy", list);
+ public Environment newEnvironment() throws Exception {
+ return newSkylarkEnvironment().update("lazy", list);
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
index c4bb7d8..b12f51b 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkShell.java
@@ -15,7 +15,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.rules.SkylarkModules;
import java.io.BufferedReader;
import java.io.IOException;
@@ -42,8 +41,9 @@
private final BufferedReader reader = new BufferedReader(
new InputStreamReader(System.in, Charset.defaultCharset()));
- private final EvaluationContext ev =
- SkylarkModules.newEvaluationContext(PRINT_HANDLER);
+ private final Mutability mutability = Mutability.create("shell");
+ private final Environment env = Environment.builder(mutability)
+ .setSkylark().setGlobals(Environment.SKYLARK).setEventHandler(PRINT_HANDLER).build();
public String read() {
StringBuilder input = new StringBuilder();
@@ -70,7 +70,7 @@
String input;
while ((input = read()) != null) {
try {
- Object result = ev.eval(input);
+ Object result = env.eval(input);
if (result != null) {
System.out.println(Printer.repr(result));
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
index 57b341f..3ebdf8d 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ValidationTests.java
@@ -95,7 +95,7 @@
@Test
public void testBuiltinSymbolsAreReadOnly() throws Exception {
- checkError("Variable rule is read only", "rule = 1");
+ checkError("Variable repr is read only", "repr = 1");
}
@Test
@@ -300,8 +300,8 @@
@Test
public void testFunctionReturnsFunction() {
parse(
- "def impl(ctx):",
- " return None",
+ "def rule(*, implementation): return None",
+ "def impl(ctx): return None",
"",
"skylark_rule = rule(implementation = impl)",
"",
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
index 5ec7ac2..ea887f9 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
@@ -14,31 +14,36 @@
package com.google.devtools.build.lib.testutil;
import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.rules.SkylarkModules;
import com.google.devtools.build.lib.syntax.Environment;
-import com.google.devtools.build.lib.syntax.EvaluationContext;
+import com.google.devtools.build.lib.syntax.Mutability;
/**
* Describes a particular testing mode by determining how the
- * appropriate {@code EvaluationContext} has to be created
+ * appropriate {@code Environment} has to be created
*/
public abstract class TestMode {
- public static final TestMode BUILD = new TestMode() {
- @Override
- public EvaluationContext createContext(EventHandler eventHandler, Environment environment)
- throws Exception {
- return EvaluationContext.newBuildContext(eventHandler, environment);
- }
- };
+ public static final TestMode BUILD =
+ new TestMode() {
+ @Override
+ public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
+ return Environment.builder(Mutability.create("build test"))
+ .setGlobals(environment == null ? Environment.BUILD : environment.getGlobals())
+ .setEventHandler(eventHandler)
+ .build();
+ }
+ };
- public static final TestMode SKYLARK = new TestMode() {
- @Override
- public EvaluationContext createContext(EventHandler eventHandler, Environment environment)
- throws Exception {
- return SkylarkModules.newEvaluationContext(eventHandler);
- }
- };
+ public static final TestMode SKYLARK =
+ new TestMode() {
+ @Override
+ public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
+ return Environment.builder(Mutability.create("skylark test"))
+ .setSkylark()
+ .setGlobals(environment == null ? Environment.SKYLARK : environment.getGlobals())
+ .setEventHandler(eventHandler)
+ .build();
+ }
+ };
- public abstract EvaluationContext createContext(
- EventHandler eventHandler, Environment environment) throws Exception;
-}
\ No newline at end of file
+ public abstract Environment createEnvironment(EventHandler eventHandler, Environment environment);
+}
