Create ValidationEnvironment from Environment
Allow ValidationEnvironment to be created from initial Environment so that
there is no need to manually keep two different sets of constructors in synch.
--
MOS_MIGRATED_REVID=101588695
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 543c71e..40253e5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -229,7 +229,7 @@
* Used to make the label instances unique, so that we don't create a new
* instance for every rule.
*/
- private static LoadingCache<String, Label> LABELS = CacheBuilder.newBuilder().build(
+ private static final LoadingCache<String, Label> LABELS = CacheBuilder.newBuilder().build(
new CacheLoader<String, Label>() {
@Override
public Label load(String from) {
@@ -307,8 +307,9 @@
this.configurationCollectionFactory = configurationCollectionFactory;
this.prerequisiteValidator = prerequisiteValidator;
this.skylarkAccessibleJavaClasses = skylarkAccessibleJavaClasses;
- this.skylarkValidationEnvironment = SkylarkModules.getValidationEnvironment(
- skylarkAccessibleJavaClasses.keySet());
+ this.skylarkValidationEnvironment = new ValidationEnvironment(
+ // TODO(bazel-team): refactor constructors so we don't have those null-s
+ createSkylarkRuleClassEnvironment(null, null));
}
public PrerequisiteValidator getPrerequisiteValidator() {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
index 470d666..df4876c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
@@ -17,7 +17,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.SkylarkNativeModule;
import com.google.devtools.build.lib.syntax.BaseFunction;
@@ -30,9 +29,7 @@
import com.google.devtools.build.lib.syntax.ValidationEnvironment;
import java.lang.reflect.Field;
-import java.util.HashSet;
import java.util.Map;
-import java.util.Set;
/**
* A class to handle all Skylark modules, to create and setup Validation and regular Environments.
@@ -111,32 +108,17 @@
for (Map.Entry<String, Object> entry : OBJECTS.entrySet()) {
env.update(entry.getKey(), entry.getValue());
}
+ // Even though PACKAGE_NAME has no value _now_ and will be bound later,
+ // it needs to be visible for the ValidationEnvironment to be happy.
+ env.update(Environment.PKG_NAME, Environment.NONE);
}
/**
* Returns a new ValidationEnvironment with the elements of the Skylark modules.
*/
public static ValidationEnvironment getValidationEnvironment() {
- return getValidationEnvironment(ImmutableSet.<String>of());
- }
-
- /**
- * Returns a new ValidationEnvironment with the elements of the Skylark modules and extraObjects.
- */
- public static ValidationEnvironment getValidationEnvironment(ImmutableSet<String> extraObjects) {
- Set<String> builtIn = new HashSet<>();
- collectSkylarkTypesFromFields(Environment.class, builtIn);
- for (Class<?> moduleClass : MODULES) {
- if (moduleClass.isAnnotationPresent(SkylarkModule.class)) {
- builtIn.add(moduleClass.getAnnotation(SkylarkModule.class).name());
- }
- }
- MethodLibrary.setupValidationEnvironment(builtIn);
- for (Class<?> module : MODULES) {
- collectSkylarkTypesFromFields(module, builtIn);
- }
- builtIn.addAll(extraObjects);
- return new ValidationEnvironment(builtIn);
+ // TODO(bazel-team): refactor constructors so we don't have those null-s
+ return new ValidationEnvironment(getNewEnvironment(null));
}
public static EvaluationContext newEvaluationContext(EventHandler eventHandler) {
@@ -170,24 +152,4 @@
throw new RuntimeException(e);
}
}
-
- /**
- * Collects the BaseFunctions from the fields of the class of the object parameter
- * and adds their class and their corresponding return value to the builder.
- */
- private static void collectSkylarkTypesFromFields(Class<?> classObject, Set<String> builtIn) {
- for (Field field : classObject.getDeclaredFields()) {
- if (field.isAnnotationPresent(SkylarkSignature.class)) {
- SkylarkSignature annotation = field.getAnnotation(SkylarkSignature.class);
- if (BaseFunction.class.isAssignableFrom(field.getType())) {
- // Ignore non-global values.
- if (annotation.objectType().equals(Object.class)) {
- builtIn.add(annotation.name());
- }
- } else {
- builtIn.add(annotation.name());
- }
- }
- }
- }
}
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 5357d4d..3acbec1 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
@@ -101,6 +101,15 @@
protected Set<String> propagatingVariables = new HashSet<>();
/**
+ * Is this a global environment?
+ * @return true if this is a global (top-level) environment
+ * as opposed to inside the body of a function
+ */
+ public boolean isGlobal() {
+ return true;
+ }
+
+ /**
* 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.
*/
@@ -191,9 +200,10 @@
* Updates the value of variable "varname" in the environment, corresponding
* to an {@link AssignmentStatement}.
*/
- public void update(String varname, Object value) {
+ public Environment update(String varname, Object value) {
Preconditions.checkNotNull(value, "update(value == null)");
env.put(varname, value);
+ return this;
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
index c292389..8e2a5c3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
@@ -119,7 +119,7 @@
* Clones this Skylark global environment.
*/
public SkylarkEnvironment cloneEnv(EventHandler eventHandler) {
- Preconditions.checkArgument(isGlobalEnvironment());
+ Preconditions.checkArgument(isGlobal());
SkylarkEnvironment newEnv = new SkylarkEnvironment(eventHandler, this.fileContentHashCode);
for (Entry<String, Object> entry : env.entrySet()) {
newEnv.env.put(entry.getKey(), entry.getValue());
@@ -142,7 +142,8 @@
/**
* Returns true if this is a Skylark global environment.
*/
- public boolean isGlobalEnvironment() {
+ @Override
+ public boolean isGlobal() {
return parent == null;
}
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 7aedca2..9b71f96 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
@@ -47,13 +47,21 @@
// Whether this validation environment is not modified therefore clonable or not.
private boolean clonable;
-
+
/**
* Tracks the number of nested for loops that contain the statement that is currently being
* validated
*/
private int loopCount = 0;
+ /**
+ * Create a ValidationEnvironment for a given global Environment
+ */
+ public ValidationEnvironment(Environment env) {
+ this(env.getVariableNames());
+ Preconditions.checkArgument(env.isGlobal());
+ }
+
public ValidationEnvironment(Set<String> builtinVariables) {
parent = null;
variables.addAll(builtinVariables);
@@ -135,7 +143,7 @@
/**
* Starts a session with temporarily disabled readonly checking for variables between branches.
* This is useful to validate control flows like if-else when we know that certain parts of the
- * code cannot both be executed.
+ * code cannot both be executed.
*/
public void startTemporarilyDisableReadonlyCheckSession() {
futureReadOnlyVariables.add(new HashSet<String>());
@@ -190,14 +198,14 @@
public boolean isInsideLoop() {
return (loopCount > 0);
}
-
+
/**
* Signals that the block of a for loop was entered
*/
public void enterLoop() {
++loopCount;
}
-
+
/**
* Signals that the block of a for loop was left
*
