Use setLoadingOrWorkspacePhase instead of setLoadingPhase where relevant,

commit 3fedf9e618cbce3dbdd00559b2de0bb8e2d43171 introduced the distinction between the loading phase and the workspace phase.

This check broke the use of native.existing_rule(s) in skylark remote repository, added a regression test for it.

Fixes #1277

--
MOS_MIGRATED_REVID=122988569

6 files changed, 27 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
index 344d9c76ba..4939e3271b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryModule.java
@@ -101,7 +101,7 @@ public class SkylarkRepositoryModule {
             FuncallExpression ast,
             com.google.devtools.build.lib.syntax.Environment funcallEnv)
             throws EvalException {
-          funcallEnv.checkLoadingPhase("repository_rule", ast.getLocation());
+          funcallEnv.checkLoadingOrWorkspacePhase("repository_rule", ast.getLocation());
           // We'll set the name later, pass the empty string for now.
           Builder builder = new Builder("", RuleClassType.WORKSPACE, true);
 
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 6dc557797c..f86931571c 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
@@ -1158,7 +1158,7 @@ public final class PackageFactory {
       public Runtime.NoneType invoke(Map<String, Object> kwargs,
           FuncallExpression ast, Environment env)
           throws EvalException, InterruptedException {
-        env.checkLoadingPhase(ruleClass, ast.getLocation());
+        env.checkLoadingOrWorkspacePhase(ruleClass, ast.getLocation());
         try {
           addRule(ruleFactory, ruleClass, getContext(env, ast), kwargs, ast, env);
         } catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
index 91e5e81880..41762fe722 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -111,7 +111,7 @@ public class SkylarkNativeModule {
       new BuiltinFunction("existing_rule") {
         public Object invoke(String name, FuncallExpression ast, Environment env)
             throws EvalException, InterruptedException {
-          env.checkLoadingPhase("native.existing_rule", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("native.existing_rule", ast.getLocation());
           SkylarkDict<String, Object> rule = PackageFactory.callGetRuleFunction(name, ast, env);
           if (rule != null) {
             return rule;
@@ -142,7 +142,7 @@ public class SkylarkNativeModule {
         public SkylarkDict<String, SkylarkDict<String, Object>> invoke(
             FuncallExpression ast, Environment env)
             throws EvalException, InterruptedException {
-          env.checkLoadingPhase("native.existing_rules", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("native.existing_rules", ast.getLocation());
           return PackageFactory.callGetRulesFunction(ast, env);
         }
       };
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 95f4bd4b15..2fd7e1aa1d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -298,7 +298,7 @@ public final class SkylarkAttr {
             Environment env)
             throws EvalException {
           // TODO(bazel-team): Replace literal strings with constants.
-          env.checkLoadingPhase("attr.int", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.int", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultInt, MANDATORY_ARG, mandatory, VALUES_ARG, values),
@@ -342,7 +342,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.string", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.string", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultString, MANDATORY_ARG, mandatory, VALUES_ARG, values),
@@ -431,7 +431,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.label", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.label", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env,
@@ -488,7 +488,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.string_list", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.string_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env,
@@ -534,7 +534,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.int_list", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.int_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env,
@@ -634,7 +634,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.label_list", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.label_list", ast.getLocation());
           SkylarkDict<String, Object> kwargs = EvalUtils.<String, Object>optionMap(
               env,
               DEFAULT_ARG,
@@ -683,7 +683,7 @@ public final class SkylarkAttr {
         public Descriptor invoke(
             Boolean defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.bool", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.bool", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
@@ -720,7 +720,7 @@ public final class SkylarkAttr {
         public Descriptor invoke(
             Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.output", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.output", ast.getLocation());
           return createNonconfigurableAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
@@ -763,7 +763,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.output_list", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.output_list", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env,
@@ -804,7 +804,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.string_dict", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.string_dict", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
@@ -840,7 +840,7 @@ public final class SkylarkAttr {
             FuncallExpression ast,
             Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.string_list_dict", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.string_list_dict", ast.getLocation());
           return createAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory, NON_EMPTY_ARG, nonEmpty),
@@ -870,7 +870,7 @@ public final class SkylarkAttr {
         public Descriptor invoke(
             Object defaultO, Boolean mandatory, FuncallExpression ast, Environment env)
             throws EvalException {
-          env.checkLoadingPhase("attr.license", ast.getLocation());
+          env.checkLoadingOrWorkspacePhase("attr.license", ast.getLocation());
           return createNonconfigurableAttrDescriptor(
               EvalUtils.<String, Object>optionMap(
                   env, DEFAULT_ARG, defaultO, MANDATORY_ARG, mandatory),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 8d452b6aec..abf4cf12e9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -287,7 +287,7 @@ public class SkylarkRuleClassFunctions {
         Object implicitOutputs, Boolean executable, Boolean outputToGenfiles, SkylarkList fragments,
         SkylarkList hostFragments, FuncallExpression ast, Environment funcallEnv)
         throws EvalException, ConversionException {
-      funcallEnv.checkLoadingPhase("rule", ast.getLocation());
+      funcallEnv.checkLoadingOrWorkspacePhase("rule", ast.getLocation());
       RuleClassType type = test ? RuleClassType.TEST : RuleClassType.NORMAL;
       RuleClass parent = test ? getTestBaseRule(funcallEnv.getToolsRepository())
           : (executable ? binaryBaseRule : baseRule);
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 e26b257af7..d9a8b276f6 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
@@ -399,6 +399,16 @@ public final class Environment implements Freezable {
   }
 
   /**
+   * Checks that the current Environment is in the loading or the workspace phase.
+   * @param symbol name of the function being only authorized thus.
+   */
+  public void checkLoadingOrWorkspacePhase(String symbol, Location loc) throws EvalException {
+    if (phase == Phase.ANALYSIS) {
+      throw new EvalException(loc, symbol + "() cannot be called during the analysis phase");
+    }
+  }
+
+  /**
    * Checks that the current Environment is in the loading phase.
    * @param symbol name of the function being only authorized thus.
    */