Simplify ValidationEnvironment.

We need only a set of symbols, types are not used anymore.

--
MOS_MIGRATED_REVID=91299566

4 files changed, 46 insertions, 75 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 b2f7850f6b..0a5a8d797d 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
@@ -291,9 +291,7 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
     this.prerequisiteValidator = prerequisiteValidator;
     this.skylarkAccessibleJavaClasses = skylarkAccessibleJavaClasses;
     this.skylarkValidationEnvironment = SkylarkModules.getValidationEnvironment(
-        ImmutableMap.<String, SkylarkType>builder()
-            .putAll(skylarkAccessibleJavaClasses)
-            .build());
+        skylarkAccessibleJavaClasses.keySet());
   }
 
   public PrerequisiteValidator getPrerequisiteValidator() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
index be2ae1c184..83704f5502 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/MethodLibrary.java
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.syntax.SkylarkFunction.cast;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Ordering;
@@ -45,8 +44,6 @@ import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkModule;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
-import com.google.devtools.build.lib.syntax.SkylarkType;
-import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -1100,34 +1097,24 @@ public class MethodLibrary {
 
   public static final List<Function> dictFunctions = ImmutableList.of(items, get, keys, values);
 
-  private static final Map<Function, SkylarkType> pureGlobalFunctions = ImmutableMap
-      .<Function, SkylarkType>builder()
-      // TODO(bazel-team): String methods are added two times, because there are
-      // a lot of cases when they are used as global functions in the depot. Those
-      // should be cleaned up first.
-      .put(bool, SkylarkType.BOOL)
-      .put(int_, SkylarkType.INT)
-      .put(len, SkylarkType.INT)
-      .put(minus, SkylarkType.INT)
-      .put(select, SkylarkType.of(SelectorValue.class))
-      .put(str, SkylarkType.STRING)
-      .build();
-
-  private static final Map<Function, SkylarkType> skylarkGlobalFunctions = ImmutableMap
-      .<Function, SkylarkType>builder()
-      .putAll(pureGlobalFunctions)
-      .put(list, SkylarkType.of(SkylarkList.class))
-      .put(struct, SkylarkType.of(ClassObject.SkylarkClassObject.class))
-      .put(hasattr, SkylarkType.BOOL)
-      .put(getattr, SkylarkType.UNKNOWN)
-      .put(set, SkylarkType.of(SkylarkNestedSet.class))
-      .put(dir, SkylarkType.of(SkylarkList.class, String.class))
-      .put(enumerate, SkylarkType.of(SkylarkList.class))
-      .put(range, SkylarkType.of(SkylarkList.class, Integer.class))
-      .put(type, SkylarkType.of(String.class))
-      .put(fail, SkylarkType.NONE)
-      .put(print, SkylarkType.NONE)
-      .put(zip, SkylarkType.LIST)
+  private static final List<Function> pureGlobalFunctions =
+      ImmutableList.of(bool, int_, len, minus, select, str);
+
+  private static final List<Function> skylarkGlobalFunctions = ImmutableList
+      .<Function>builder()
+      .addAll(pureGlobalFunctions)
+      .add(list)
+      .add(struct)
+      .add(hasattr)
+      .add(getattr)
+      .add(set)
+      .add(dir)
+      .add(enumerate)
+      .add(range)
+      .add(type)
+      .add(fail)
+      .add(print)
+      .add(zip)
       .build();
 
   /**
@@ -1142,7 +1129,7 @@ public class MethodLibrary {
     setupMethodEnvironment(env, SkylarkList.class, listPureFunctions);
     if (env.isSkylarkEnabled()) {
       env.registerFunction(SkylarkList.class, indexOperator.getName(), indexOperator);
-      setupMethodEnvironment(env, skylarkGlobalFunctions.keySet());
+      setupMethodEnvironment(env, skylarkGlobalFunctions);
     } else {
       env.registerFunction(List.class, indexOperator.getName(), indexOperator);
       env.registerFunction(ImmutableList.class, indexOperator.getName(), indexOperator);
@@ -1150,7 +1137,7 @@ public class MethodLibrary {
       // It is allowed in BUILD files only for backward-compatibility.
       setupMethodEnvironment(env, List.class, listFunctions);
       setupMethodEnvironment(env, stringFunctions);
-      setupMethodEnvironment(env, pureGlobalFunctions.keySet());
+      setupMethodEnvironment(env, pureGlobalFunctions);
     }
   }
 
@@ -1170,11 +1157,9 @@ public class MethodLibrary {
   /**
    * Collect global functions for the validation environment.
    */
-  public static void setupValidationEnvironment(Map<String, SkylarkType> builtIn) {
-    // TODO(bazel-team): To be simplified (we need only the names, not the types).
-    for (Map.Entry<Function, SkylarkType> function : skylarkGlobalFunctions.entrySet()) {
-      String name = function.getKey().getName();
-      builtIn.put(name, SkylarkFunctionType.of(name, function.getValue()));
+  public static void setupValidationEnvironment(Set<String> builtIn) {
+    for (Function function : skylarkGlobalFunctions) {
+      builtIn.add(function.getName());
     }
   }
 }
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 4e80db50d9..e75ebb25e2 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,6 +17,7 @@ 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.MethodLibrary;
 import com.google.devtools.build.lib.packages.SkylarkNativeModule;
@@ -27,13 +28,12 @@ import com.google.devtools.build.lib.syntax.SkylarkBuiltin;
 import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
 import com.google.devtools.build.lib.syntax.SkylarkFunction;
 import com.google.devtools.build.lib.syntax.SkylarkModule;
-import com.google.devtools.build.lib.syntax.SkylarkType;
-import com.google.devtools.build.lib.syntax.SkylarkType.SkylarkFunctionType;
 import com.google.devtools.build.lib.syntax.ValidationEnvironment;
 
 import java.lang.reflect.Field;
-import java.util.HashMap;
+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.
@@ -112,27 +112,25 @@ public class SkylarkModules {
    * Returns a new ValidationEnvironment with the elements of the Skylark modules.
    */
   public static ValidationEnvironment getValidationEnvironment() {
-    return getValidationEnvironment(ImmutableMap.<String, SkylarkType>of());
+    return getValidationEnvironment(ImmutableSet.<String>of());
   }
 
   /**
    * Returns a new ValidationEnvironment with the elements of the Skylark modules and extraObjects.
    */
-  public static ValidationEnvironment getValidationEnvironment(
-      ImmutableMap<String, SkylarkType> extraObjects) {
-    Map<String, SkylarkType> builtIn = new HashMap<>();
+  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.put(moduleClass.getAnnotation(SkylarkModule.class).name(),
-            SkylarkType.of(moduleClass));
+        builtIn.add(moduleClass.getAnnotation(SkylarkModule.class).name());
       }
     }
     MethodLibrary.setupValidationEnvironment(builtIn);
     for (Class<?> module : MODULES) {
       collectSkylarkTypesFromFields(module, builtIn);
     }
-    builtIn.putAll(extraObjects);
+    builtIn.addAll(extraObjects);
     return new ValidationEnvironment(builtIn);
   }
 
@@ -175,21 +173,17 @@ public class SkylarkModules {
    * Collects the SkylarkFunctions 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,
-      Map<String, SkylarkType> builtIn) {
+  private static void collectSkylarkTypesFromFields(Class<?> classObject, Set<String> builtIn) {
     for (Field field : classObject.getDeclaredFields()) {
       if (field.isAnnotationPresent(SkylarkBuiltin.class)) {
         SkylarkBuiltin annotation = field.getAnnotation(SkylarkBuiltin.class);
         if (SkylarkFunction.class.isAssignableFrom(field.getType())) {
           // Ignore non-global values.
           if (annotation.objectType().equals(Object.class)) {
-            builtIn.put(annotation.name(), SkylarkType.UNKNOWN);
+            builtIn.add(annotation.name());
           }
-        } else if (Function.class.isAssignableFrom(field.getType())) {
-          builtIn.put(annotation.name(),
-              SkylarkFunctionType.of(annotation.name(), SkylarkType.UNKNOWN));
         } else {
-          builtIn.put(annotation.name(), SkylarkType.of(field.getType()));
+          builtIn.add(annotation.name());
         }
       }
     }
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 2bbdbbb584..5d38ccfec8 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
@@ -15,7 +15,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.events.Location;
 
 import java.util.HashMap;
@@ -34,7 +34,7 @@ public class ValidationEnvironment {
 
   private final ValidationEnvironment parent;
 
-  private Map<String, SkylarkType> variableTypes = new HashMap<>();
+  private Set<String> variables = new HashSet<>();
 
   private Map<String, Location> variableLocations = new HashMap<>();
 
@@ -47,26 +47,22 @@ public class ValidationEnvironment {
   // Whether this validation environment is not modified therefore clonable or not.
   private boolean clonable;
 
-  public ValidationEnvironment(Map<String, SkylarkType> builtinVariableTypes) {
+  public ValidationEnvironment(Set<String> builtinVariables) {
     parent = null;
-    variableTypes = new HashMap<>(builtinVariableTypes);
-    readOnlyVariables.addAll(builtinVariableTypes.keySet());
+    variables.addAll(builtinVariables);
+    readOnlyVariables.addAll(builtinVariables);
     clonable = true;
   }
 
-  private ValidationEnvironment(Map<String, SkylarkType> builtinVariableTypes,
-      Set<String> readOnlyVariables) {
+  private ValidationEnvironment(Set<String> builtinVariables, Set<String> readOnlyVariables) {
     parent = null;
-    this.variableTypes = new HashMap<>(builtinVariableTypes);
+    this.variables = new HashSet<>(builtinVariables);
     this.readOnlyVariables = new HashSet<>(readOnlyVariables);
     clonable = false;
   }
 
   // ValidationEnvironment for a new Environment()
-  private static ImmutableMap<String, SkylarkType> globalTypes =
-      new ImmutableMap.Builder<String, SkylarkType> ()
-      .put("False", SkylarkType.BOOL).put("True", SkylarkType.BOOL)
-      .put("None", SkylarkType.TOP).build();
+  private static ImmutableSet<String> globalTypes = ImmutableSet.of("False", "True", "None");
 
   public ValidationEnvironment() {
     this(globalTypes);
@@ -75,7 +71,7 @@ public class ValidationEnvironment {
   @Override
   public ValidationEnvironment clone() {
     Preconditions.checkState(clonable);
-    return new ValidationEnvironment(variableTypes, readOnlyVariables);
+    return new ValidationEnvironment(variables, readOnlyVariables);
   }
 
   /**
@@ -84,7 +80,6 @@ public class ValidationEnvironment {
   public ValidationEnvironment(ValidationEnvironment parent) {
     // Don't copy readOnlyVariables: Variables may shadow global values.
     this.parent = parent;
-    this.variableTypes = new HashMap<String, SkylarkType>();
     this.clonable = false;
   }
 
@@ -108,7 +103,7 @@ public class ValidationEnvironment {
         futureReadOnlyVariables.peek().add(varname);
       }
     }
-    variableTypes.put(varname, SkylarkType.UNKNOWN);
+    variables.add(varname);
     variableLocations.put(varname, location);
     clonable = false;
   }
@@ -123,8 +118,7 @@ public class ValidationEnvironment {
    * Returns true if the symbol exists in the validation environment.
    */
   public boolean hasSymbolInEnvironment(String varname) {
-    return variableTypes.containsKey(varname)
-        || topLevel().variableTypes.containsKey(varname);
+    return variables.contains(varname) || topLevel().variables.contains(varname);
   }
 
   private ValidationEnvironment topLevel() {