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

5 files changed, 35 insertions, 53 deletions

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 543c71e28e..40253e566f 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 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
    * 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 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     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 470d6668f9..df4876c599 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 @@ package com.google.devtools.build.lib.rules;
 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.SkylarkSignature;
 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 @@ public class SkylarkModules {
     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 @@ public class SkylarkModules {
       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 5357d4d4db..3acbec1cab 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 @@ public class Environment {
   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 @@ public class Environment {
    * 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 c2923896fb..8e2a5c3acc 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 @@ public class SkylarkEnvironment extends Environment implements Serializable {
    * 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 @@ public class SkylarkEnvironment extends Environment implements Serializable {
   /**
    * 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 7aedca2c82..9b71f96acc 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 @@ public class ValidationEnvironment {
 
   // 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 @@ public class ValidationEnvironment {
   /**
    * 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 class ValidationEnvironment {
   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
    *