Be explicit about semantics of Skylark environments

All callers that do not use Environment.Builder#setSemantics should call #useDefaultSemantics. A follow-up CL will enforce this requirement.

Motivation: It's more important that we are strict about semantics than about the other builder args. It's too easy for a mistake in semantics to go unnoticed.

RELNOTES: None
PiperOrigin-RevId: 172912829

10 files changed, 35 insertions, 14 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 6f4172ea7a..4688db294c 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
@@ -56,7 +56,10 @@ public final class SkylarkModules {
 
   private static Environment.Frame createGlobals(List<Class<?>> modules) {
     try (Mutability mutability = Mutability.create("SkylarkModules")) {
-      Environment env = Environment.builder(mutability).setGlobals(BazelLibrary.GLOBALS).build();
+      Environment env = Environment.builder(mutability)
+          .useDefaultSemantics()
+          .setGlobals(BazelLibrary.GLOBALS)
+          .build();
       for (Class<?> moduleClass : modules) {
         Runtime.registerModuleGlobals(env, moduleClass);
       }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index 081a2a1f6a..18ac7b4060 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -58,6 +58,7 @@ public class SkylarkRepositoryFunction extends RepositoryFunction {
       // This Skylark environment ignores command line flags.
       com.google.devtools.build.lib.syntax.Environment buildEnv =
           com.google.devtools.build.lib.syntax.Environment.builder(mutability)
+              .useDefaultSemantics()
               .setGlobals(rule.getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
               .setEventHandler(env.getListener())
               .build();
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 69e2ff2ffa..0a9d68bff5 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
@@ -182,20 +182,22 @@ public class WorkspaceFactory {
   private void execute(BuildFileAST ast, @Nullable Map<String, Extension> importedExtensions,
       StoredEventHandler localReporter)
       throws InterruptedException {
-    // Note that this Skylark environment ignores command line flags.
-    Environment.Builder environmentBuilder =
-        Environment.builder(mutability)
-            .setGlobals(BazelLibrary.GLOBALS)
-            .setEventHandler(localReporter);
     if (importedExtensions != null) {
-      Map<String, Extension> map = new HashMap<String, Extension>(parentImportMap);
+      Map<String, Extension> map = new HashMap<>(parentImportMap);
       map.putAll(importedExtensions);
-      importMap = ImmutableMap.<String, Extension>copyOf(importedExtensions);
+      importMap = ImmutableMap.copyOf(importedExtensions);
     } else {
       importMap = parentImportMap;
     }
-    environmentBuilder.setImportedExtensions(importMap);
-    Environment workspaceEnv = environmentBuilder.setPhase(Phase.WORKSPACE).build();
+    Environment workspaceEnv =
+        Environment.builder(mutability)
+            // Note that this Skylark environment ignores command line flags.
+            .useDefaultSemantics()
+            .setGlobals(BazelLibrary.GLOBALS)
+            .setEventHandler(localReporter)
+            .setImportedExtensions(importMap)
+            .setPhase(Phase.WORKSPACE)
+            .build();
     addWorkspaceFunctions(workspaceEnv, localReporter);
     for (Map.Entry<String, Object> binding : parentVariableBindings.entrySet()) {
       try {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
index d019e91b75..110c9e1f8c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java
@@ -258,7 +258,9 @@ public class BazelLibrary {
     List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type);
 
     try (Mutability mutability = Mutability.create("BUILD")) {
-      Environment env = Environment.builder(mutability).build();
+      Environment env = Environment.builder(mutability)
+          .useDefaultSemantics()
+          .build();
       Runtime.setupConstants(env);
       Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions);
       Runtime.setupMethodEnvironment(env, bazelGlobalFunctions);
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 1978ba57f9..90e20daf75 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
@@ -981,7 +981,9 @@ public final class Environment implements Freezable {
 
   private static Environment.Frame createConstantsGlobals() {
     try (Mutability mutability = Mutability.create("CONSTANTS")) {
-      Environment env = Environment.builder(mutability).build();
+      Environment env = Environment.builder(mutability)
+          .useDefaultSemantics()
+          .build();
       Runtime.setupConstants(env);
       return env.getGlobals();
     }
@@ -989,7 +991,9 @@ public final class Environment implements Freezable {
 
   private static Environment.Frame createDefaultGlobals() {
     try (Mutability mutability = Mutability.create("BUILD")) {
-      Environment env = Environment.builder(mutability).build();
+      Environment env = Environment.builder(mutability)
+          .useDefaultSemantics()
+          .build();
       Runtime.setupConstants(env);
       Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions);
       return env.getGlobals();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
index 08100da710..a774a84b88 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
@@ -178,6 +178,7 @@ public class SkylarkSignatureProcessor {
         // Note that this Skylark environment ignores command line flags.
         Environment env =
             Environment.builder(mutability)
+                .useDefaultSemantics()
                 .setGlobals(Environment.CONSTANTS_ONLY)
                 .setEventHandler(Environment.FAIL_FAST_HANDLER)
                 .build()
diff --git a/src/main/java/com/google/devtools/skylark/Skylark.java b/src/main/java/com/google/devtools/skylark/Skylark.java
index df7932e16b..cfa4d6f18e 100644
--- a/src/main/java/com/google/devtools/skylark/Skylark.java
+++ b/src/main/java/com/google/devtools/skylark/Skylark.java
@@ -56,6 +56,7 @@ class Skylark {
   private final Mutability mutability = Mutability.create("interpreter");
   private final Environment env =
       Environment.builder(mutability)
+          .useDefaultSemantics()
           .setGlobals(Environment.DEFAULT_GLOBALS)
           .setEventHandler(PRINT_HANDLER)
           .build();
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
index 7521474c92..a442f7cbb4 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
@@ -61,6 +61,7 @@ public abstract class SkylarkTestCase extends BuildViewTestCase {
                 .build();
         Environment env =
             Environment.builder(mutability)
+                .useDefaultSemantics()
                 .setEventHandler(getEventHandler())
                 .setGlobals(SkylarkModules.getGlobals(modules))
                 .setPhase(Phase.LOADING)
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
index 8bf49f8351..1be64cbf9e 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java
@@ -100,6 +100,7 @@ public class EnvironmentTest extends EvaluationTestCase {
     try (Mutability mut = Mutability.create("outer")) {
       outerEnv =
           Environment.builder(mut)
+              .useDefaultSemantics()
               .setGlobals(Environment.DEFAULT_GLOBALS)
               .build()
               .update("foo", "bar")
@@ -107,7 +108,9 @@ public class EnvironmentTest extends EvaluationTestCase {
     }
     try (Mutability mut = Mutability.create("inner")) {
       innerEnv = Environment.builder(mut)
-          .setGlobals(outerEnv.getGlobals()).build()
+          .useDefaultSemantics()
+          .setGlobals(outerEnv.getGlobals())
+          .build()
           .update("foo", "bat")
           .update("quux", 42);
     }
@@ -200,6 +203,7 @@ public class EnvironmentTest extends EvaluationTestCase {
     try (Mutability mutability = Mutability.create("testFrozen")) {
       env =
           Environment.builder(mutability)
+              .useDefaultSemantics()
               .setGlobals(Environment.DEFAULT_GLOBALS)
               .setEventHandler(Environment.FAIL_FAST_HANDLER)
               .build();
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
index 428aefedd3..900d7ea655 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
@@ -71,6 +71,7 @@ public class EvaluationTestCase {
   public Environment newBuildEnvironment() {
     Environment env =
         Environment.builder(mutability)
+            .useDefaultSemantics()
             .setGlobals(BazelLibrary.GLOBALS)
             .setEventHandler(getEventHandler())
             .setPhase(Phase.LOADING)
@@ -85,6 +86,7 @@ public class EvaluationTestCase {
    */
   public Environment newSkylarkEnvironment() {
     return Environment.builder(mutability)
+        .useDefaultSemantics()
         .setGlobals(BazelLibrary.GLOBALS)
         .setEventHandler(getEventHandler())
         .build();