Add TransitionType enum and add checks that transitions aren't used

inappropriately.

Fixes bazelbuild#13751.

Closes bazelbuild#13755.

PiperOrigin-RevId: 387382475

src/main/java/com/google/devtools/build/lib/analysis/config/ExecutionTransitionFactory.java

@@ -53,6 +53,11 @@ public static ExecutionTransitionFactory create() {
     return new ExecutionTransitionFactory(DEFAULT_EXEC_GROUP_NAME);
   }
 
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.ATTRIBUTE;
+  }
+
   /**
    * Returns a new {@link ExecutionTransitionFactory} for the given {@link
    * com.google.devtools.build.lib.packages.ExecGroup}.

src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactory.java

@@ -45,6 +45,9 @@ public static <T extends TransitionFactory.Data> TransitionFactory<T> of(
 
     Preconditions.checkNotNull(transitionFactory1);
     Preconditions.checkNotNull(transitionFactory2);
+    Preconditions.checkArgument(
+        transitionFactory1.transitionType().isCompatibleWith(transitionFactory2.transitionType()),
+        "transition factory types must be compatible");
     Preconditions.checkArgument(
         !transitionFactory1.isSplit() || !transitionFactory2.isSplit(),
         "can't compose two split transition factories");

src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionFactory.java

@@ -31,12 +31,36 @@
  */
 public interface TransitionFactory<T extends TransitionFactory.Data> {
 
+  /** Enum that describes what type of transition a TransitionFactory creates. */
+  enum TransitionType {
+    /** A transition that can be used for rules or attributes. */
+    ANY,
+    /** A transition that can be used for rules only. */
+    RULE,
+    /** A transition that can be used for attributes only. */
+    ATTRIBUTE;
+
+    public boolean isCompatibleWith(TransitionType other) {
+      if (this == ANY) {
+        return true;
+      }
+      if (other == ANY) {
+        return true;
+      }
+      return this == other;
+    }
+  }
+
   /** A marker interface for classes that provide data to TransitionFactory instances. */
   interface Data {}
 
   /** Returns a new {@link ConfigurationTransition}, based on the given data. */
   ConfigurationTransition create(T data);
 
+  default TransitionType transitionType() {
+    return TransitionType.ANY;
+  }
+
   // TODO(https://github.com/bazelbuild/bazel/issues/7814): Once everything uses TransitionFactory,
   // remove these methods.
   /** Returns {@code true} if the result of this {@link TransitionFactory} is a host transition. */

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttributeTransitionProvider.java

@@ -74,6 +74,11 @@ public SplitTransition create(AttributeTransitionData data) {
         starlarkDefinedConfigTransition, (ConfiguredAttributeMapper) attributeMap);
   }
 
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.ATTRIBUTE;
+  }
+
   @Override
   public boolean isSplit() {
     return true;

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -43,6 +43,7 @@
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
+import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory.TransitionType;
 import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
@@ -414,10 +415,18 @@ public StarlarkCallable rule(
       } else if (cfg instanceof PatchTransition) {
         builder.cfg((PatchTransition) cfg);
       } else if (cfg instanceof TransitionFactory) {
-        @SuppressWarnings("unchecked")
-        TransitionFactory<RuleTransitionData> transitionFactory =
-            (TransitionFactory<RuleTransitionData>) cfg;
-        builder.cfg(transitionFactory);
+        TransitionFactory<? extends TransitionFactory.Data> transitionFactory =
+            (TransitionFactory<? extends TransitionFactory.Data>) cfg;
+        if (transitionFactory.transitionType().isCompatibleWith(TransitionType.RULE)) {
+          @SuppressWarnings("unchecked") // Actually checked due to above isCompatibleWith call.
+          TransitionFactory<RuleTransitionData> ruleTransitionFactory =
+              (TransitionFactory<RuleTransitionData>) transitionFactory;
+          builder.cfg(ruleTransitionFactory);
+        } else {
+          throw Starlark.errorf(
+              "`cfg` must be set to a transition appropriate for a rule, not an attribute-specific"
+                  + " transition.");
+        }
       } else {
         // This is not technically true: it could also be a native transition, but this is the
         // most likely error case.

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java

@@ -130,6 +130,11 @@ public PatchTransition create(RuleTransitionData ruleData) {
         unused -> new FunctionPatchTransition(starlarkDefinedConfigTransition, ruleData.rule()));
   }
 
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.RULE;
+  }
+
   @Override
   public boolean isSplit() {
     // The transitions returned by this factory are guaranteed not to be splits.

src/main/java/com/google/devtools/build/lib/analysis/test/TestTrimmingTransitionFactory.java

@@ -130,4 +130,9 @@ public PatchTransition create(RuleTransitionData ruleData) {
     }
     return TestTrimmingTransition.TESTONLY_FALSE;
   }
+
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.RULE;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTaggedTrimmingTransitionFactory.java

@@ -122,4 +122,9 @@ public PatchTransition create(RuleTransitionData ruleData) {
 
     return new ConfigFeatureFlagTaggedTrimmingTransition(requiredLabels);
   }
+
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.RULE;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTransitionFactory.java

@@ -112,6 +112,11 @@ public PatchTransition create(RuleTransitionData ruleData) {
     }
   }
 
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.RULE;
+  }
+
   /**
    * Returns the attribute examined by this transition factory.
    */

src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchSplitTransitionProvider.java

@@ -171,6 +171,11 @@ public SplitTransition create(AttributeTransitionData data) {
     return new AppleBinaryTransition(platformType, minimumOsVersion);
   }
 
+  @Override
+  public TransitionType transitionType() {
+    return TransitionType.ATTRIBUTE;
+  }
+
   @Override
   public boolean isSplit() {
     return true;

src/main/java/com/google/devtools/build/lib/rules/python/PyStarlarkTransitions.java

@@ -68,6 +68,11 @@ public ConfigurationTransition create(AttributeTransitionData data) {
       }
     }
 
+    @Override
+    public TransitionType transitionType() {
+      return TransitionType.ATTRIBUTE;
+    }
+
     @Override
     public void repr(Printer printer) {
       printer.append("<py_transitions.cfg>");

src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java

@@ -302,6 +302,11 @@ public SplitTransition create(AttributeTransitionData data) {
       return new TestSplitTransition();
     }
 
+    @Override
+    public TransitionType transitionType() {
+      return TransitionType.ATTRIBUTE;
+    }
+
     @Override
     public boolean isSplit() {
       return true;

src/test/shell/integration/starlark_configurations_test.sh

@@ -701,4 +701,31 @@ function test_rc_flag_alias_unsupported_with_space_assignment_syntax() {
   write_default_bazelrc
 }
 
+# Regression test for https://github.com/bazelbuild/bazel/issues/13751.
+function test_rule_exec_transition_warning() {
+  local -r pkg=$FUNCNAME
+  mkdir -p $pkg
+
+  cat > "${pkg}/rules.bzl" <<EOF
+def _impl(ctx):
+    pass
+
+demo = rule(
+    implementation = _impl,
+    cfg = config.exec(),
+)
+EOF
+
+  cat > "${pkg}/BUILD" <<EOF
+load(":rules.bzl", "demo")
+
+demo(name = "demo")
+EOF
+
+
+  bazel build //$pkg:demo >& "$TEST_log" && fail "Expected failure"
+  expect_not_log "crashed due to an internal error"
+  expect_log "`cfg` must be set to a transition appropriate for a rule"
+}
+
 run_suite "${PRODUCT_NAME} starlark configurations tests"