Remove onlyLoadingPhase on objects.

The mechanism was easy to workaround (store the object in a different
variable) and a source of bugs. This affected only 'rule', 'native' and
'attr' objects. It turns out the blacklisting was not useful (native and
attr are already filtered, rule is not a problem).

--
MOS_MIGRATED_REVID=101359277

src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java

@@ -186,7 +186,6 @@ private static String attributeToNative(String oldName, Location loc, boolean is
   @SkylarkSignature(name = "rule", doc =
       "Creates a new rule. Store it in a global value, so that it can be loaded and called "
       + "from BUILD files.",
-      onlyLoadingPhase = true,
       returnType = BaseFunction.class,
       mandatoryPositionals = {
         @Param(name = "implementation", type = BaseFunction.class,

src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java

@@ -76,9 +76,6 @@ public abstract class BaseFunction {
   // Documentation for variables, if any
   @Nullable protected List<String> paramDoc;
 
-  // True if this function is only allowed during the Loading Phase
-  protected boolean onlyLoadingPhase;
-
   // The types actually enforced by the Skylark runtime, as opposed to those enforced by the JVM,
   // or those displayed to the user in the documentation.
   @Nullable protected List<SkylarkType> enforcedArgumentTypes;
@@ -119,11 +116,6 @@ public boolean isConfigured() {
     return signature != null;
   }
 
-  /** Returns true if the function is only available during loading phase */
-  public boolean isOnlyLoadingPhase() {
-    return onlyLoadingPhase;
-  }
-
   /**
    * Creates an unconfigured BaseFunction with the given name.
    *
@@ -489,7 +481,6 @@ public void configure(SkylarkSignature annotation) {
         getName(), annotation, unconfiguredDefaultValues, paramDoc, getEnforcedArgumentTypes());
     this.objectType = annotation.objectType().equals(Object.class)
         ? null : annotation.objectType();
-    this.onlyLoadingPhase = annotation.onlyLoadingPhase();
     configure();
   }
 

src/main/java/com/google/devtools/build/lib/syntax/BuiltinFunction.java

@@ -294,7 +294,6 @@ public void configure(BuiltinFunction.Factory factory) {
     this.signature = factory.getSignature();
     this.extraArgs = factory.getExtraArgs();
     this.objectType = factory.getObjectType();
-    this.onlyLoadingPhase = factory.isOnlyLoadingPhase();
     configure();
   }
 

src/main/java/com/google/devtools/build/lib/syntax/Environment.java

@@ -89,14 +89,9 @@ private NoneType() {}
   protected Map<PathFragment, SkylarkEnvironment> importedExtensions;
 
   /**
-   * A set of disable variables propagating through function calling. This is needed because
+   * A set of disabled namespaces propagating through function calling. This is needed because
    * UserDefinedFunctions lock the definition Environment which should be immutable.
    */
-  protected Set<String> disabledVariables = new HashSet<>();
-
-  /**
-   * A set of disable namespaces propagating through function calling. See disabledVariables.
-   */
   protected Set<Class<?>> disabledNameSpaces = new HashSet<>();
 
   /**
@@ -166,9 +161,6 @@ protected boolean hasVariable(String varname) {
    *
    */
   public Object lookup(String varname) throws NoSuchVariableException {
-    if (disabledVariables.contains(varname)) {
-      throw new NoSuchVariableException(varname);
-    }
     Object value = env.get(varname);
     if (value == null) {
       if (parent != null) {

src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java

@@ -75,7 +75,6 @@ public static SkylarkEnvironment createEnvironmentForFunctionCalling(
       // This should never happen.
       throw new IllegalStateException(e);
     }
-    childEnv.disabledVariables = callerEnv.disabledVariables;
     childEnv.disabledNameSpaces = callerEnv.disabledNameSpaces;
     return childEnv;
   }
@@ -163,9 +162,6 @@ public boolean isSkylarkEnabled() {
    */
   @Override
   public Object lookup(String varname) throws NoSuchVariableException {
-    if (disabledVariables.contains(varname)) {
-      throw new NoSuchVariableException(varname);
-    }
     Object value = env.get(varname);
     if (value == null) {
       if (parent != null && parent.hasVariable(varname)) {
@@ -201,22 +197,15 @@ public Class<?> getVariableType(String varname) {
    * only during the loading phase.
    */
   public void disableOnlyLoadingPhaseObjects() {
-    List<String> objectsToRemove = new ArrayList<>();
     List<Class<?>> modulesToRemove = new ArrayList<>();
     for (Map.Entry<String, Object> entry : env.entrySet()) {
       Object object = entry.getValue();
-      if (object instanceof BaseFunction) {
-        if (((BaseFunction) object).isOnlyLoadingPhase()) {
-          objectsToRemove.add(entry.getKey());
-        }
-      } else if (object.getClass().isAnnotationPresent(SkylarkModule.class)) {
+      if (object.getClass().isAnnotationPresent(SkylarkModule.class)) {
         if (object.getClass().getAnnotation(SkylarkModule.class).onlyLoadingPhase()) {
-          objectsToRemove.add(entry.getKey());
           modulesToRemove.add(entry.getValue().getClass());
         }
       }
     }
-    disabledVariables.addAll(objectsToRemove);
     disabledNameSpaces.addAll(modulesToRemove);
   }
 

src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignature.java

@@ -52,8 +52,6 @@
 
   Class<?> returnType() default Object.class;
 
-  boolean onlyLoadingPhase() default false;
-
   // TODO(bazel-team): determine this way whether to accept mutable Lists
   // boolean mutableLists() default false;