Add static name resolution
This implements a part of the name resolution document: https://github.com/bazelbuild/proposals/blob/master/docs/2018-06-18-name-resolution.md
and it is enabled with the flag --incompatible_static_name_resolution
There are two visible changes:
1. Local variables can be used before their definition point.
2. Local variables will shadow global variables, even if they are not initialiazed yet.
https://github.com/bazelbuild/bazel/issues/5637
RELNOTES: None.
PiperOrigin-RevId: 210562752
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 387f6ef..f0f695e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -81,15 +81,17 @@
*/
public final class Environment implements Freezable, Debuggable {
- /**
- * A phase for enabling or disabling certain builtin functions
- */
- public enum Phase { WORKSPACE, LOADING, ANALYSIS }
+ /** A phase for enabling or disabling certain builtin functions */
+ public enum Phase {
+ WORKSPACE,
+ LOADING,
+ ANALYSIS
+ }
/**
- * A mapping of bindings, either mutable or immutable according to an associated
- * {@link Mutability}. The order of the bindings within a single {@link Frame} is deterministic
- * but unspecified.
+ * A mapping of bindings, either mutable or immutable according to an associated {@link
+ * Mutability}. The order of the bindings within a single {@link Frame} is deterministic but
+ * unspecified.
*
* <p>Any non-frozen {@link Frame} must have the same {@link Mutability} as the current {@link
* Environment}, to avoid interference from other evaluation contexts. For example, a {@link
@@ -127,14 +129,14 @@
void put(Environment env, String varname, Object value) throws MutabilityException;
/**
- * TODO(laurentlb): Remove this method when possible. It should probably not
- * be part of the public interface.
+ * TODO(laurentlb): Remove this method when possible. It should probably not be part of the
+ * public interface.
*/
void remove(Environment env, String varname) throws MutabilityException;
/**
- * Returns a map containing all bindings of this {@link Frame} and of its transitive
- * parents, taking into account shadowing precedence.
+ * Returns a map containing all bindings of this {@link Frame} and of its transitive parents,
+ * taking into account shadowing precedence.
*
* <p>The bindings are returned in a deterministic order (for a given sequence of initial values
* and updates).
@@ -149,7 +151,7 @@
: new MutableLexicalFrame(mutability);
}
- static LexicalFrame createForUserDefinedFunctionCall(Mutability mutability, int numArgs) {
+ static LexicalFrame create(Mutability mutability, int numArgs) {
Preconditions.checkState(!mutability.isFrozen());
return new MutableLexicalFrame(mutability, /*initialCapacity=*/ numArgs);
}
@@ -248,22 +250,19 @@
* {@link GlobalFrame}s can represent different builtin scopes with a linear precedence ordering.
*
* <p>A {@link GlobalFrame} can also be constructed in a two-phase process. To do this, call the
- * nullary constructor to create an uninitialized {@link GlobalFrame}, then call
- * {@link #initialize}. It is illegal to use any other method in-between these two calls, or to
- * call {@link #initialize} on an already initialized {@link GlobalFrame}.
+ * nullary constructor to create an uninitialized {@link GlobalFrame}, then call {@link
+ * #initialize}. It is illegal to use any other method in-between these two calls, or to call
+ * {@link #initialize} on an already initialized {@link GlobalFrame}.
*/
-
public static final class GlobalFrame implements Frame {
/**
* Final, except that it may be initialized after instantiation. Null mutability indicates that
* this Frame is uninitialized.
*/
- @Nullable
- private Mutability mutability;
+ @Nullable private Mutability mutability;
/** Final, except that it may be initialized after instantiation. */
- @Nullable
- private GlobalFrame parent;
+ @Nullable private GlobalFrame parent;
/**
* If this frame is a global frame, the label for the corresponding target, e.g. {@code
@@ -271,8 +270,7 @@
*
* <p>Final, except that it may be initialized after instantiation.
*/
- @Nullable
- private Label label;
+ @Nullable private Label label;
/** Bindings are maintained in order of creation. */
private final LinkedHashMap<String, Object> bindings;
@@ -322,10 +320,12 @@
}
public void initialize(
- Mutability mutability, @Nullable GlobalFrame parent,
- @Nullable Label label, Map<String, Object> bindings) {
- Preconditions.checkState(this.mutability == null,
- "Attempted to initialize an already initialized Frame");
+ Mutability mutability,
+ @Nullable GlobalFrame parent,
+ @Nullable Label label,
+ Map<String, Object> bindings) {
+ Preconditions.checkState(
+ this.mutability == null, "Attempted to initialize an already initialized Frame");
this.mutability = Preconditions.checkNotNull(mutability);
this.parent = parent;
this.label = label;
@@ -432,8 +432,7 @@
}
@Override
- public void put(Environment env, String varname, Object value)
- throws MutabilityException {
+ public void put(Environment env, String varname, Object value) throws MutabilityException {
checkInitialized();
Mutability.checkMutable(this, env.mutability());
bindings.put(varname, value);
@@ -565,9 +564,10 @@
return;
}
if (!(obj instanceof Extension)) {
- throw new IllegalStateException(String.format(
- "Expected an equal Extension, but got a %s instead of an Extension",
- obj == null ? "null" : obj.getClass().getName()));
+ throw new IllegalStateException(
+ String.format(
+ "Expected an equal Extension, but got a %s instead of an Extension",
+ obj == null ? "null" : obj.getClass().getName()));
}
Extension other = (Extension) obj;
ImmutableMap<String, Object> otherBindings = other.getBindings();
@@ -575,11 +575,12 @@
Set<String> names = bindings.keySet();
Set<String> otherNames = otherBindings.keySet();
if (!names.equals(otherNames)) {
- throw new IllegalStateException(String.format(
- "Expected Extensions to be equal, but they don't define the same bindings: "
- + "in this one but not given one: [%s]; in given one but not this one: [%s]",
- Joiner.on(", ").join(Sets.difference(names, otherNames)),
- Joiner.on(", ").join(Sets.difference(otherNames, names))));
+ throw new IllegalStateException(
+ String.format(
+ "Expected Extensions to be equal, but they don't define the same bindings: "
+ + "in this one but not given one: [%s]; in given one but not this one: [%s]",
+ Joiner.on(", ").join(Sets.difference(names, otherNames)),
+ Joiner.on(", ").join(Sets.difference(otherNames, names))));
}
ArrayList<String> badEntries = new ArrayList<>();
@@ -638,10 +639,11 @@
}
if (!transitiveContentHashCode.equals(other.getTransitiveContentHashCode())) {
- throw new IllegalStateException(String.format(
- "Expected Extensions to be equal, but transitive content hashes don't match: %s != %s",
- transitiveContentHashCode,
- other.getTransitiveContentHashCode()));
+ throw new IllegalStateException(
+ String.format(
+ "Expected Extensions to be equal, but transitive content hashes don't match:"
+ + " %s != %s",
+ transitiveContentHashCode, other.getTransitiveContentHashCode()));
}
}
@@ -652,8 +654,8 @@
}
/**
- * Static Frame for lexical variables that are always looked up in the current Environment
- * or for the definition Environment of the function currently being evaluated.
+ * Static Frame for lexical variables that are always looked up in the current Environment or for
+ * the definition Environment of the function currently being evaluated.
*/
private Frame lexicalFrame;
@@ -666,20 +668,18 @@
private GlobalFrame globalFrame;
/**
- * Dynamic Frame for variables that are always looked up in the runtime Environment,
- * and never in the lexical or "global" Environment as it was at the time of function definition.
- * For instance, PACKAGE_NAME.
+ * Dynamic Frame for variables that are always looked up in the runtime Environment, and never in
+ * the lexical or "global" Environment as it was at the time of function definition. For instance,
+ * PACKAGE_NAME.
*/
private final Frame dynamicFrame;
- /**
- * The semantics options that affect how Skylark code is evaluated.
- */
+ /** The semantics options that affect how Skylark code is evaluated. */
private final SkylarkSemantics semantics;
/**
- * An EventHandler for errors and warnings. This is not used in the BUILD language,
- * however it might be used in Skylark code called from the BUILD language, so shouldn't be null.
+ * An EventHandler for errors and warnings. This is not used in the BUILD language, however it
+ * might be used in Skylark code called from the BUILD language, so shouldn't be null.
*/
private final EventHandler eventHandler;
@@ -690,30 +690,28 @@
/**
* Is this Environment being executed during the loading phase? Many builtin functions are only
- * enabled during the loading phase, and check this flag.
- * TODO(laurentlb): Remove from Environment
+ * enabled during the loading phase, and check this flag. TODO(laurentlb): Remove from Environment
*/
private final Phase phase;
/**
- * When in a lexical (Skylark) Frame, this set contains the variable names that are global,
- * as determined not by global declarations (not currently supported),
- * but by previous lookups that ended being global or dynamic.
- * This is necessary because if in a function definition something
+ * When in a lexical (Skylark) Frame, this set contains the variable names that are global, as
+ * determined not by global declarations (not currently supported), but by previous lookups that
+ * ended being global or dynamic. This is necessary because if in a function definition something
* reads a global variable after which a local variable with the same name is assigned an
* Exception needs to be thrown.
*/
@Nullable private LinkedHashSet<String> knownGlobalVariables;
/**
- * When in a lexical (Skylark) frame, this lists the names of the functions in the call stack.
- * We currently use it to artificially disable recursion.
+ * When in a lexical (Skylark) frame, this lists the names of the functions in the call stack. We
+ * currently use it to artificially disable recursion.
*/
@Nullable private Continuation continuation;
/**
- * Gets the label of the BUILD file that is using this environment. For example, if a target
- * //foo has a dependency on //bar which is a Skylark rule defined in //rules:my_rule.bzl being
+ * Gets the label of the BUILD file that is using this environment. For example, if a target //foo
+ * has a dependency on //bar which is a Skylark rule defined in //rules:my_rule.bzl being
* evaluated in this environment, then this would return //foo.
*/
@Nullable private final Label callerLabel;
@@ -739,9 +737,7 @@
knownGlobalVariables = new LinkedHashSet<>();
}
- /**
- * Exits a scope by restoring state from the current continuation
- */
+ /** Exits a scope by restoring state from the current continuation */
void exitScope() {
Preconditions.checkNotNull(continuation);
lexicalFrame = continuation.lexicalFrame;
@@ -753,8 +749,8 @@
private final String transitiveHashCode;
/**
- * Checks that the current Environment is in the loading or the workspace phase.
- * TODO(laurentlb): Move to SkylarkUtils
+ * Checks that the current Environment is in the loading or the workspace phase. TODO(laurentlb):
+ * Move to SkylarkUtils
*
* @param symbol name of the function being only authorized thus.
*/
@@ -765,8 +761,8 @@
}
/**
- * Checks that the current Environment is in the loading phase.
- * TODO(laurentlb): Move to SkylarkUtils
+ * Checks that the current Environment is in the loading phase. TODO(laurentlb): Move to
+ * SkylarkUtils
*
* @param symbol name of the function being only authorized thus.
*/
@@ -778,8 +774,9 @@
/**
* Is this a global Environment?
- * @return true if the current code is being executed at the top-level,
- * as opposed to inside the body of a function.
+ *
+ * @return true if the current code is being executed at the top-level, as opposed to inside the
+ * body of a function.
*/
boolean isGlobal() {
return lexicalFrame instanceof GlobalFrame;
@@ -797,8 +794,9 @@
}
/**
- * Returns an EventHandler for errors and warnings.
- * The BUILD language doesn't use it directly, but can call Skylark code that does use it.
+ * Returns an EventHandler for errors and warnings. The BUILD language doesn't use it directly,
+ * but can call Skylark code that does use it.
+ *
* @return an EventHandler
*/
public EventHandler getEventHandler() {
@@ -824,9 +822,7 @@
return continuation != null ? continuation.function : null;
}
- /**
- * Returns the FuncallExpression and the BaseFunction for the top-level call being evaluated.
- */
+ /** Returns the FuncallExpression and the BaseFunction for the top-level call being evaluated. */
public Pair<FuncallExpression, BaseFunction> getTopCall() {
Continuation continuation = this.continuation;
if (continuation == null) {
@@ -924,7 +920,7 @@
}
/** Declares imported extensions for load() statements. */
- public Builder setImportedExtensions (Map<String, Extension> importMap) {
+ public Builder setImportedExtensions(Map<String, Extension> importMap) {
Preconditions.checkState(this.importedExtensions == null);
this.importedExtensions = importMap;
return this;
@@ -971,16 +967,15 @@
return new Builder(mutability);
}
- /**
- * Returns the caller's label.
- */
+ /** Returns the caller's label. */
public Label getCallerLabel() {
return callerLabel;
}
/**
- * Sets a binding for a special dynamic variable in this Environment.
- * This is not for end-users, and will throw an AssertionError in case of conflict.
+ * Sets a binding for a special dynamic variable in this Environment. This is not for end-users,
+ * and will throw an AssertionError in case of conflict.
+ *
* @param varname the name of the dynamic variable to be bound
* @param value a value to bind to the variable
* @return this Environment, in fluid style
@@ -988,8 +983,7 @@
public Environment setupDynamic(String varname, Object value) {
if (lookup(varname) != null) {
throw new AssertionError(
- String.format("Trying to bind dynamic variable '%s' but it is already bound",
- varname));
+ String.format("Trying to bind dynamic variable '%s' but it is already bound", varname));
}
try {
dynamicFrame.put(this, varname, value);
@@ -1014,10 +1008,11 @@
}
/**
- * Modifies a binding in the current Frame of this Environment, as would an
- * {@link AssignmentStatement}. Does not try to modify an inherited binding.
- * This will shadow any inherited binding, which may be an error
- * that you want to guard against before calling this function.
+ * Modifies a binding in the current Frame of this Environment, as would an {@link
+ * AssignmentStatement}. Does not try to modify an inherited binding. This will shadow any
+ * inherited binding, which may be an error that you want to guard against before calling this
+ * function.
+ *
* @param varname the name of the variable to be bound
* @param value the value to bind to the variable
* @return this Environment, in fluid style
@@ -1042,8 +1037,7 @@
// imported from a parent Environment by updating the current Environment, which will not
// trigger a MutabilityException.
throw new AssertionError(
- Printer.format("Can't update %s to %r in frozen environment", varname, value),
- e);
+ Printer.format("Can't update %s to %r in frozen environment", varname, value), e);
}
return this;
}
@@ -1051,6 +1045,7 @@
/**
* Initializes a binding in this Environment. It is an error if the variable is already bound.
* This is not for end-users, and will throw an AssertionError in case of conflict.
+ *
* @param varname the name of the variable to be bound
* @param value the value to bind to the variable
* @return this Environment, in fluid style
@@ -1063,8 +1058,9 @@
}
/**
- * Initializes a binding in this environment. Overrides any previous binding.
- * This is not for end-users, and will throw an AssertionError in case of conflict.
+ * Initializes a binding in this environment. Overrides any previous binding. This is not for
+ * end-users, and will throw an AssertionError in case of conflict.
+ *
* @param varname the name of the variable to be bound
* @param value the value to bind to the variable
* @return this Environment, in fluid style
@@ -1078,6 +1074,15 @@
}
/**
+ * Returns the value of a variable defined in the current lexical frame. Do not search in any
+ * parent scope. This function should be used once the AST has been analysed and we know which
+ * variables are local.
+ */
+ public Object localLookup(String varname) {
+ return lexicalFrame.get(varname);
+ }
+
+ /**
* Returns the value from the environment whose name is "varname" if it exists, otherwise null.
*/
public Object lookup(String varname) {
@@ -1247,14 +1252,16 @@
}
/**
- * An Exception thrown when an attempt is made to import a symbol from a file
- * that was not properly loaded.
+ * An Exception thrown when an attempt is made to import a symbol from a file that was not
+ * properly loaded.
*/
static class LoadFailedException extends Exception {
LoadFailedException(String importString) {
- super(String.format("file '%s' was not correctly loaded. "
- + "Make sure the 'load' statement appears in the global scope in your file",
- importString));
+ super(
+ String.format(
+ "file '%s' was not correctly loaded. "
+ + "Make sure the 'load' statement appears in the global scope in your file",
+ importString));
}
LoadFailedException(String importString, String symbolString, Iterable<String> allKeys) {
@@ -1308,8 +1315,8 @@
}
/**
- * Returns a hash code calculated from the hash code of this Environment and the
- * transitive closure of other Environments it loads.
+ * Returns a hash code calculated from the hash code of this Environment and the transitive
+ * closure of other Environments it loads.
*/
public String getTransitiveContentHashCode() {
return transitiveHashCode;
@@ -1351,16 +1358,17 @@
* A handler that immediately throws {@link FailFastException} whenever an error or warning
* occurs.
*
- * We do not reuse an existing unchecked exception type, because callers (e.g., test assertions)
- * need to be able to distinguish between organically occurring exceptions and exceptions thrown
- * by this handler.
+ * <p>We do not reuse an existing unchecked exception type, because callers (e.g., test
+ * assertions) need to be able to distinguish between organically occurring exceptions and
+ * exceptions thrown by this handler.
*/
- public static final EventHandler FAIL_FAST_HANDLER = new EventHandler() {
- @Override
- public void handle(Event event) {
- if (EventKind.ERRORS_AND_WARNINGS.contains(event.getKind())) {
- throw new FailFastException(event.toString());
- }
- }
- };
+ public static final EventHandler FAIL_FAST_HANDLER =
+ new EventHandler() {
+ @Override
+ public void handle(Event event) {
+ if (EventKind.ERRORS_AND_WARNINGS.contains(event.getKind())) {
+ throw new FailFastException(event.toString());
+ }
+ }
+ };
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index e2bd0e3..8e06f69 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.syntax;
+import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.util.SpellChecker;
import java.io.IOException;
import java.util.Set;
@@ -32,6 +33,9 @@
public final class Identifier extends Expression {
private final String name;
+ // The scope of the variable. The value is set when the AST has been analysed by
+ // ValidationEnvironment.
+ @Nullable private ValidationEnvironment.Scope scope;
public Identifier(String name) {
this.name = name;
@@ -55,6 +59,7 @@
@Override
public boolean equals(@Nullable Object object) {
+ // TODO(laurentlb): Remove this. AST nodes should probably not be comparable.
if (object instanceof Identifier) {
Identifier that = (Identifier) object;
return this.name.equals(that.name);
@@ -64,12 +69,19 @@
@Override
public int hashCode() {
+ // TODO(laurentlb): Remove this.
return name.hashCode();
}
+ void setScope(ValidationEnvironment.Scope scope) {
+ Preconditions.checkState(this.scope == null);
+ this.scope = scope;
+ }
+
@Override
Object doEval(Environment env) throws EvalException {
- Object value = env.lookup(name);
+ Object value =
+ scope == ValidationEnvironment.Scope.Local ? env.localLookup(name) : env.lookup(name);
if (value == null) {
throw createInvalidIdentifierException(env.getVariableNames());
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 8410dd7..5783d11 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -65,8 +65,7 @@
}
ImmutableList<String> names = signature.getSignature().getNames();
- LexicalFrame lexicalFrame =
- LexicalFrame.createForUserDefinedFunctionCall(env.mutability(), /*numArgs=*/ names.size());
+ LexicalFrame lexicalFrame = LexicalFrame.create(env.mutability(), /*numArgs=*/ names.size());
try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.SKYLARK_USER_FN, getName())) {
env.enterScope(this, lexicalFrame, ast, definitionGlobals);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 14d55ac..78e4da2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -40,7 +40,7 @@
*/
public final class ValidationEnvironment extends SyntaxTreeVisitor {
- private enum Scope {
+ enum Scope {
/** Symbols defined inside a function or a comprehension. */
Local,
/** Symbols defined at a module top-level, e.g. functions, loaded symbols. */
@@ -98,8 +98,9 @@
}
/**
- * Add all definitions to the current block. This is done because symbols are sometimes used
- * before their definition point (e.g. a functions are not necessarily declared in order).
+ * First pass: add all definitions to the current block. This is done because symbols are
+ * sometimes used before their definition point (e.g. a functions are not necessarily declared in
+ * order).
*
* <p>The old behavior (when incompatibleStaticNameResolution is false) doesn't have this first
* pass.
@@ -167,9 +168,11 @@
@Override
public void visit(Identifier node) {
- if (!hasSymbolInEnvironment(node.getName())) {
+ @Nullable Block b = blockThatDefines(node.getName());
+ if (b == null) {
throw new ValidationException(node.createInvalidIdentifierException(getAllSymbols()));
}
+ node.setScope(b.scope);
}
private void validateLValue(Location loc, Expression expr) {
@@ -304,14 +307,14 @@
block.variables.add(varname);
}
- /** Returns true if the symbol exists in the validation environment (or a parent). */
- private boolean hasSymbolInEnvironment(String varname) {
+ /** Returns the nearest Block that defines a symbol. */
+ private Block blockThatDefines(String varname) {
for (Block b = block; b != null; b = b.parent) {
if (b.variables.contains(varname)) {
- return true;
+ return b;
}
}
- return false;
+ return null;
}
/** Returns the set of all accessible symbols (both local and global) */
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 f11cfe7..1b4d0eb 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
@@ -26,9 +26,7 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Tests of Environment.
- */
+/** Tests of Environment. */
@RunWith(JUnit4.class)
public class EnvironmentTest extends EvaluationTestCase {
@@ -95,7 +93,8 @@
try (Mutability mut = Mutability.create("test")) {
IllegalArgumentException expected =
assertThrows(IllegalArgumentException.class, () -> Environment.builder(mut).build());
- assertThat(expected).hasMessageThat()
+ assertThat(expected)
+ .hasMessageThat()
.contains("must call either setSemantics or useDefaultSemantics");
}
}
@@ -114,12 +113,13 @@
.update("wiz", 3);
}
try (Mutability mut = Mutability.create("inner")) {
- innerEnv = Environment.builder(mut)
- .useDefaultSemantics()
- .setGlobals(outerEnv.getGlobals())
- .build()
- .update("foo", "bat")
- .update("quux", 42);
+ innerEnv =
+ Environment.builder(mut)
+ .useDefaultSemantics()
+ .setGlobals(outerEnv.getGlobals())
+ .build()
+ .update("foo", "bat")
+ .update("quux", 42);
}
assertThat(outerEnv.getVariableNames())
@@ -256,7 +256,6 @@
@Test
public void testVariableIsReferencedBeforeAssignment() throws Exception {
Environment env = newSkylarkEnvironment().update("global_var", 666);
-
try {
BuildFileAST.eval(env, "def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
throw new AssertionError("failed to fail");
@@ -272,29 +271,25 @@
@Test
public void testVarOrderDeterminism() throws Exception {
Mutability parentMutability = Mutability.create("parent env");
- Environment parentEnv = Environment.builder(parentMutability)
- .useDefaultSemantics()
- .build();
+ Environment parentEnv = Environment.builder(parentMutability).useDefaultSemantics().build();
parentEnv.update("a", 1);
parentEnv.update("c", 2);
parentEnv.update("b", 3);
Environment.GlobalFrame parentFrame = parentEnv.getGlobals();
parentMutability.freeze();
Mutability mutability = Mutability.create("testing");
- Environment env = Environment.builder(mutability)
- .useDefaultSemantics()
- .setGlobals(parentFrame)
- .build();
+ Environment env =
+ Environment.builder(mutability).useDefaultSemantics().setGlobals(parentFrame).build();
env.update("x", 4);
env.update("z", 5);
env.update("y", 6);
// The order just has to be deterministic, but for definiteness this test spells out the exact
// order returned by the implementation: parent frame before current environment, and bindings
// within a frame ordered by when they were added.
- assertThat(env.getVariableNames())
- .containsExactly("a", "c", "b", "x", "z", "y").inOrder();
+ assertThat(env.getVariableNames()).containsExactly("a", "c", "b", "x", "z", "y").inOrder();
assertThat(env.getGlobals().getTransitiveBindings())
- .containsExactly("a", 1, "c", 2, "b", 3, "x", 4, "z", 5, "y", 6).inOrder();
+ .containsExactly("a", 1, "c", 2, "b", 3, "x", 4, "z", 5, "y", 6)
+ .inOrder();
}
@Test
@@ -304,33 +299,30 @@
Extension a = new Extension(ImmutableMap.of(), "a123");
Extension b = new Extension(ImmutableMap.of(), "b456");
Extension c = new Extension(ImmutableMap.of(), "c789");
- Environment env1 = Environment.builder(Mutability.create("testing1"))
- .useDefaultSemantics()
- .setImportedExtensions(ImmutableMap.of("a", a, "b", b, "c", c))
- .setFileContentHashCode("z")
- .build();
- Environment env2 = Environment.builder(Mutability.create("testing2"))
- .useDefaultSemantics()
- .setImportedExtensions(ImmutableMap.of("c", c, "b", b, "a", a))
- .setFileContentHashCode("z")
- .build();
+ Environment env1 =
+ Environment.builder(Mutability.create("testing1"))
+ .useDefaultSemantics()
+ .setImportedExtensions(ImmutableMap.of("a", a, "b", b, "c", c))
+ .setFileContentHashCode("z")
+ .build();
+ Environment env2 =
+ Environment.builder(Mutability.create("testing2"))
+ .useDefaultSemantics()
+ .setImportedExtensions(ImmutableMap.of("c", c, "b", b, "a", a))
+ .setFileContentHashCode("z")
+ .build();
assertThat(env1.getTransitiveContentHashCode()).isEqualTo(env2.getTransitiveContentHashCode());
}
@Test
public void testExtensionEqualityDebugging_RhsIsNull() {
- assertCheckStateFailsWithMessage(
- new Extension(ImmutableMap.of(), "abc"),
- null,
- "got a null");
+ assertCheckStateFailsWithMessage(new Extension(ImmutableMap.of(), "abc"), null, "got a null");
}
@Test
public void testExtensionEqualityDebugging_RhsHasBadType() {
assertCheckStateFailsWithMessage(
- new Extension(ImmutableMap.of(), "abc"),
- 5,
- "got a java.lang.Integer");
+ new Extension(ImmutableMap.of(), "abc"), 5, "got a java.lang.Integer");
}
@Test
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 6991363..64ebe3e 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
@@ -1796,6 +1796,32 @@
}
@Test
+ public void testLocalVariableDefinedBelow() throws Exception {
+ new SkylarkTest("--incompatible_static_name_resolution=true")
+ .setUp(
+ "def beforeEven(li):", // returns the value before the first even number
+ " for i in li:",
+ " if i % 2 == 0:",
+ " return a",
+ " else:",
+ " a = i",
+ "res = beforeEven([1, 3, 4, 5])")
+ .testLookup("res", 3);
+ }
+
+ @Test
+ public void testShadowisNotInitialized() throws Exception {
+ new SkylarkTest("--incompatible_static_name_resolution=true")
+ .testIfErrorContains(
+ /* error message */ "name 'gl' is not defined",
+ "gl = 5",
+ "def foo():", // returns the value before the first even number
+ " if False: gl = 2",
+ " return gl",
+ "res = foo()");
+ }
+
+ @Test
public void testFunctionCallRecursion() throws Exception {
new SkylarkTest().testIfErrorContains("Recursion was detected when calling 'f' from 'g'",
"def main():",
