1 files changed, 9 insertions, 80 deletions

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 df4876c599..c52c58c88c 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
@@ -16,38 +16,27 @@ 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.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.SkylarkNativeModule;
-import com.google.devtools.build.lib.syntax.BaseFunction;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvaluationContext;
 import com.google.devtools.build.lib.syntax.MethodLibrary;
+import com.google.devtools.build.lib.syntax.Runtime;
 import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
-import com.google.devtools.build.lib.syntax.SkylarkModule;
-import com.google.devtools.build.lib.syntax.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.ValidationEnvironment;
 
-import java.lang.reflect.Field;
-import java.util.Map;
-
 /**
- * A class to handle all Skylark modules, to create and setup Validation and regular Environments.
+ * The basis for a Skylark Environment with all build-related modules registered.
  */
-// TODO(bazel-team): move that to the syntax package and
-// let each extension register itself in a static { } statement.
-public class SkylarkModules {
+public final class SkylarkModules {
+
+  private SkylarkModules() { }
 
   /**
    * The list of built in Skylark modules.
    * Documentation is generated automatically for all these modules.
-   * They are also registered with the {@link ValidationEnvironment}
-   * and the {@link SkylarkEnvironment}.
-   * Note that only functions with a {@link SkylarkSignature} annotations are handled properly.
+   * They are also registered with the {@link Environment}.
    */
-  // TODO(bazel-team): find a more general, more automated way of registering classes and building
-  // initial environments. And don't give syntax.Environment and packages.MethodLibrary a special
-  // treatment, have them use the same registration mechanism as other classes currently below.
   public static final ImmutableList<Class<?>> MODULES = ImmutableList.of(
       SkylarkAttr.class,
       SkylarkCommandLine.class,
@@ -55,30 +44,6 @@ public class SkylarkModules {
       SkylarkRuleClassFunctions.class,
       SkylarkRuleImplementationFunctions.class);
 
-  private static final ImmutableMap<Class<?>, ImmutableList<BaseFunction>> FUNCTION_MAP;
-  private static final ImmutableMap<String, Object> OBJECTS;
-
-  static {
-    try {
-      ImmutableMap.Builder<Class<?>, ImmutableList<BaseFunction>> functionMap =
-          ImmutableMap.builder();
-      ImmutableMap.Builder<String, Object> objects = ImmutableMap.builder();
-      for (Class<?> moduleClass : MODULES) {
-        if (moduleClass.isAnnotationPresent(SkylarkModule.class)) {
-          objects.put(moduleClass.getAnnotation(SkylarkModule.class).name(),
-              moduleClass.newInstance());
-        }
-        ImmutableList.Builder<BaseFunction> functions = ImmutableList.builder();
-        collectSkylarkFunctionsAndObjectsFromFields(moduleClass, functions, objects);
-        functionMap.put(moduleClass, functions.build());
-      }
-      FUNCTION_MAP = functionMap.build();
-      OBJECTS = objects.build();
-    } catch (InstantiationException | IllegalAccessException e) {
-      throw new RuntimeException(e);
-    }
-  }
-
   /**
    * Returns a new SkylarkEnvironment with the elements of the Skylark modules.
    */
@@ -96,21 +61,12 @@ public class SkylarkModules {
 
   private static void setupEnvironment(Environment env) {
     MethodLibrary.setupMethodEnvironment(env);
-    for (Map.Entry<Class<?>, ImmutableList<BaseFunction>> entry : FUNCTION_MAP.entrySet()) {
-      for (BaseFunction function : entry.getValue()) {
-        if (function.getObjectType() != null) {
-          env.registerFunction(function.getObjectType(), function.getName(), function);
-        } else {
-          env.update(function.getName(), function);
-        }
-      }
-    }
-    for (Map.Entry<String, Object> entry : OBJECTS.entrySet()) {
-      env.update(entry.getKey(), entry.getValue());
+    for (Class<?> moduleClass : MODULES) {
+      Runtime.registerModuleGlobals(env, moduleClass);
     }
     // 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);
+    env.update(Runtime.PKG_NAME, Runtime.NONE);
   }
 
   /**
@@ -125,31 +81,4 @@ public class SkylarkModules {
     return EvaluationContext.newSkylarkContext(
         getNewEnvironment(eventHandler), getValidationEnvironment());
   }
-
-  /**
-   * Collects the BaseFunctions from the fields of the class of the object parameter
-   * and adds them into the builder.
-   */
-  private static void collectSkylarkFunctionsAndObjectsFromFields(Class<?> type,
-      ImmutableList.Builder<BaseFunction> functions, ImmutableMap.Builder<String, Object> objects) {
-    try {
-      for (Field field : type.getDeclaredFields()) {
-        if (field.isAnnotationPresent(SkylarkSignature.class)) {
-          // Fields in Skylark modules are sometimes private.
-          // Nevertheless they have to be annotated with SkylarkSignature.
-          field.setAccessible(true);
-          SkylarkSignature annotation = field.getAnnotation(SkylarkSignature.class);
-          Object value = field.get(null);
-          if (BaseFunction.class.isAssignableFrom(field.getType())) {
-            functions.add((BaseFunction) value);
-          } else {
-            objects.put(annotation.name(), value);
-          }
-        }
-      }
-    } catch (IllegalArgumentException | IllegalAccessException e) {
-      // This should never happen.
-      throw new RuntimeException(e);
-    }
-  }
 }