Move BazelLibrary from syntax/ to packages/

This helps the Skylark interpreter to not depend on Bazel concepts, though it adds a temporary dependency of Skylint on packages/. The fix for that will be to create a Build API interface for BazelLibrary (e.g., "BazelLibraryAPI").

Refactored some GlobalFrame construction logic to be more uniform. Instead of constructing a whole Environment just to get a frame, we build the frame directly, using ImmutableMap.Builder to accumulate bindings. This convention may further change once we convert MethodLibrary and the like to @SkylarkGlobalLibrary, but for now it's more readable.

RELNOTES: None
PiperOrigin-RevId: 194960824

7 files changed, 86 insertions, 53 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java
index a85893197e..b57b96bd26 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java
@@ -15,8 +15,8 @@
 package com.google.devtools.build.lib.analysis.skylark;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.packages.BazelLibrary;
 import com.google.devtools.build.lib.packages.SkylarkNativeModule;
-import com.google.devtools.build.lib.syntax.BazelLibrary;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.Environment.GlobalFrame;
 import com.google.devtools.build.lib.syntax.Mutability;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/BazelLibrary.java
index 4e5845f63f..205f98f167 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BazelLibrary.java
@@ -12,20 +12,35 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.syntax;
+package com.google.devtools.build.lib.packages;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
-import java.util.List;
+import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.BuiltinFunction;
+import com.google.devtools.build.lib.syntax.Environment.GlobalFrame;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.EvalUtils;
+import com.google.devtools.build.lib.syntax.MethodLibrary;
+import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.SelectorList;
+import com.google.devtools.build.lib.syntax.SelectorValue;
+import com.google.devtools.build.lib.syntax.SkylarkDict;
+import com.google.devtools.build.lib.syntax.SkylarkList;
+import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
+import com.google.devtools.build.lib.syntax.SkylarkType;
 
 /**
  * A helper class containing additional built in functions for Bazel (BUILD files and .bzl files).
  */
 public class BazelLibrary {
 
+  // TODO(bazel-team): Move to MethodLibrary alongside other pure-Skylark builtins.
   @SkylarkSignature(
     name = "type",
     returnType = String.class,
@@ -205,21 +220,26 @@ public class BazelLibrary {
         }
       };
 
-  private static Environment.GlobalFrame createGlobals() {
-    List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type);
-
-    try (Mutability mutability = Mutability.create("BUILD")) {
-      Environment env = Environment.builder(mutability)
-          .useDefaultSemantics()
-          .build();
-      Runtime.setupConstants(env);
-      Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions);
-      Runtime.setupMethodEnvironment(env, bazelGlobalFunctions);
-      return env.getGlobals();
+  /** Adds bindings for all the builtin functions of this class to the given map builder. */
+  public static void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) {
+    for (BaseFunction function : allFunctions) {
+      builder.put(function.getName(), function);
     }
   }
 
-  public static final Environment.GlobalFrame GLOBALS = createGlobals();
+  private static final ImmutableList<BaseFunction> allFunctions =
+      ImmutableList.of(select, depset, type);
+
+  /** A global frame containing pure Skylark builtins and some Bazel builtins. */
+  public static final GlobalFrame GLOBALS = createGlobals();
+
+  private static GlobalFrame createGlobals() {
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    Runtime.addConstantsToBuilder(builder);
+    MethodLibrary.addBindingsToBuilder(builder);
+    BazelLibrary.addBindingsToBuilder(builder);
+    return GlobalFrame.createForBuiltins(builder.build());
+  }
 
   static {
     SkylarkSignatureProcessor.configureSkylarkFunctions(BazelLibrary.class);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 873934e2e7..ac66963090 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -38,7 +38,6 @@ import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.BaseFunction;
-import com.google.devtools.build.lib.syntax.BazelLibrary;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
 import com.google.devtools.build.lib.syntax.BuiltinFunction;
 import com.google.devtools.build.lib.syntax.ClassObject;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 6868a98099..73358746d9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
 import com.google.devtools.build.lib.syntax.BaseFunction;
-import com.google.devtools.build.lib.syntax.BazelLibrary;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
 import com.google.devtools.build.lib.syntax.BuiltinFunction;
 import com.google.devtools.build.lib.syntax.ClassObject;
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 46253faa49..c536a688b3 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
@@ -281,19 +281,36 @@ public final class Environment implements Freezable {
       this.bindings = new LinkedHashMap<>();
     }
 
-    public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent, @Nullable Label label) {
+    public GlobalFrame(
+        Mutability mutability,
+        @Nullable GlobalFrame parent,
+        @Nullable Label label,
+        @Nullable Map<String, Object> bindings) {
       this.mutability = Preconditions.checkNotNull(mutability);
       this.parent = parent;
       this.label = label;
       this.bindings = new LinkedHashMap<>();
+      if (bindings != null) {
+        this.bindings.putAll(bindings);
+      }
     }
 
     public GlobalFrame(Mutability mutability) {
-      this(mutability, null, null);
+      this(mutability, null, null, null);
+    }
+
+    public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent) {
+      this(mutability, parent, null, null);
     }
 
-    public GlobalFrame(Mutability mutability, GlobalFrame parent) {
-      this(mutability, parent, null);
+    public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent, @Nullable Label label) {
+      this(mutability, parent, label, null);
+    }
+
+    /** Constructs a global frame for the given builtin bindings. */
+    public static GlobalFrame createForBuiltins(Map<String, Object> bindings) {
+      Mutability mutability = Mutability.create("<builtins>").freeze();
+      return new GlobalFrame(mutability, null, null, bindings);
     }
 
     private void checkInitialized() {
@@ -1217,34 +1234,29 @@ public final class Environment implements Freezable {
     return transitiveHashCode;
   }
 
-  /** A read-only {@link Environment.GlobalFrame} with global constants in it only */
+  /** A read-only {@link Environment.GlobalFrame} with False/True/None constants only. */
   static final GlobalFrame CONSTANTS_ONLY = createConstantsGlobals();
 
-  /** A read-only {@link Environment.GlobalFrame} with initial globals */
+  /**
+   * A read-only {@link Environment.GlobalFrame} with initial globals as defined in
+   * MethodLibrary.
+   */
   public static final GlobalFrame DEFAULT_GLOBALS = createDefaultGlobals();
 
   /** To be removed when all call-sites are updated. */
   public static final GlobalFrame SKYLARK = DEFAULT_GLOBALS;
 
   private static Environment.GlobalFrame createConstantsGlobals() {
-    try (Mutability mutability = Mutability.create("CONSTANTS")) {
-      Environment env = Environment.builder(mutability)
-          .useDefaultSemantics()
-          .build();
-      Runtime.setupConstants(env);
-      return env.getGlobals();
-    }
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    Runtime.addConstantsToBuilder(builder);
+    return GlobalFrame.createForBuiltins(builder.build());
   }
 
   private static Environment.GlobalFrame createDefaultGlobals() {
-    try (Mutability mutability = Mutability.create("BUILD")) {
-      Environment env = Environment.builder(mutability)
-          .useDefaultSemantics()
-          .build();
-      Runtime.setupConstants(env);
-      Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions);
-      return env.getGlobals();
-    }
+    ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
+    Runtime.addConstantsToBuilder(builder);
+    MethodLibrary.addBindingsToBuilder(builder);
+    return GlobalFrame.createForBuiltins(builder.build());
   }
 
   /** An exception thrown by {@link #FAIL_FAST_HANDLER}. */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index fe519b0a27..97c582ad8d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -960,7 +960,14 @@ public class MethodLibrary {
   )
   public static final class BoolModule {}
 
-  static final List<BaseFunction> defaultGlobalFunctions =
+  /** Adds bindings for all the builtin functions of this class to the given map builder. */
+  public static void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) {
+    for (BaseFunction function : allFunctions) {
+      builder.put(function.getName(), function);
+    }
+  }
+
+  private static final ImmutableList<BaseFunction> allFunctions =
       ImmutableList.of(
           all, any, bool, dict, dir, fail, getattr, hasattr, hash, enumerate, int_, len, list, max,
           min, print, range, repr, reversed, sorted, str, tuple, zip);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java b/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java
index 86f95b38b7..66060299b2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkGlobalLibrary;
@@ -137,14 +138,16 @@ public final class Runtime {
   )
   public static final String REPOSITORY_NAME = "REPOSITORY_NAME";
 
-  /**
-   * Set up a given environment for supported class methods.
-   */
-  static Environment setupConstants(Environment env) {
+  /** Adds bindings for False/True/None constants to the given map builder. */
+  public static void addConstantsToBuilder(ImmutableMap.Builder<String, Object> builder) {
     // In Python 2.x, True and False are global values and can be redefined by the user.
-    // In Python 3.x, they are keywords. We implement them as values, for the sake of
-    // simplicity. We define them as Boolean objects.
-    return env.setup("False", FALSE).setup("True", TRUE).setup("None", NONE);
+    // In Python 3.x, they are keywords. We implement them as values. Currently they can't be
+    // redefined because builtins can't be overridden. In the future we should permit shadowing of
+    // most builtins but still prevent shadowing of these constants.
+    builder
+        .put("False", FALSE)
+        .put("True", TRUE)
+        .put("None", NONE);
   }
 
 
@@ -390,11 +393,4 @@ public final class Runtime {
   public static void registerModuleGlobals(Environment env, Class<?> moduleClass) {
     setupModuleGlobals(env, moduleClass);
   }
-
-  static void setupMethodEnvironment(
-      Environment env, Iterable<BaseFunction> functions) {
-    for (BaseFunction function : functions) {
-      env.setup(function.getName(), function);
-    }
-  }
 }