[7.0.0] Experimental extend rule and subrule functionality (bazelbuil…

…d#20429)

Following commits were cleanly cherry-picked:

e4c7cb6 ilist@google.com Support incoming configuration transition in
extended rules
faa2f3d  ilist@google.com	 Support subrules in extended rules
d70d198 ilist@google.com Add an escape hatch for any type attributes
in the initializer
c00c272 ilist@google.com Cleanup incoming configuration transitions
on Starlark rules
82a469f ilist@google.com Copy and lift labels in arguments of
initializers
6733c18  ilist@google.com	 Remove unused OBJECT_LIST type
e7a3dab  ilist@google.com	 Remove unused FILESET_ENTRY label class
c959990 ilist@google.com Implement type checks and label conversion
for Starlark values
c4f4073 ilist@google.com Move convertFromBuildLangType into BuildType
class

Additional commit "Define function_transition_allowlist in
StarlarkRuleClassFunctionsTest" just fixes the mocking of the the tests.

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

@@ -338,10 +338,6 @@ private NestedSet<Label> transitiveLabels() {
    *
    * <p>This is done within {@link RuleConfiguredTargetBuilder} so that every rule always and
    * automatically propagates the validation action output group.
-   *
-   * <p>Note that in addition to {@link LabelClass#DEPENDENCY}, there is also {@link
-   * LabelClass#FILESET_ENTRY}, however the fileset implementation takes care of propagating the
-   * validation action output group itself.
    */
   private void propagateTransitiveValidationOutputGroups() {
     if (outputGroupBuilders.containsKey(OutputGroupInfo.VALIDATION_TRANSITIVE)) {

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

@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.analysis.starlark;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.RUN_UNDER;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.TIMEOUT_DEFAULT;
 import static com.google.devtools.build.lib.analysis.BaseRuleClasses.getTestRuntimeLabelList;
@@ -43,11 +44,10 @@
 import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
 import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
 import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
+import com.google.devtools.build.lib.analysis.config.transitions.ComposingTransitionFactory;
 import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
-import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
 import com.google.devtools.build.lib.analysis.config.transitions.StarlarkExposedRuleTransitionFactory;
 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;
@@ -101,6 +101,7 @@
 import com.google.devtools.build.lib.util.Pair;
 import com.google.errorprone.annotations.FormatMethod;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
@@ -380,10 +381,6 @@ public StarlarkRuleFunction rule(
           executableUnchecked != Starlark.UNBOUND,
           "Omit executable parameter when extending rules.");
       failIf(testUnchecked != Starlark.UNBOUND, "Omit test parameter when extending rules.");
-      // TODO b/300201845 - add cfg support
-      failIf(cfg != Starlark.NONE, "cfg is not supported in extended rules yet.");
-      // TODO b/300201845 - add subrules support
-      failIf(!subrules.isEmpty(), "subrules are not supported in extended rules yet.");
       failIf(
           implicitOutputs != Starlark.NONE,
           "implicit_outputs is not supported when extending rules (deprecated).");
@@ -496,7 +493,7 @@ public static StarlarkRuleFunction createRule(
       builder = new RuleClass.Builder("", type, true, baseParent);
     }
 
-    builder.initializer(initializer);
+    builder.initializer(initializer, labelConverter);
 
     builder.setDefaultExtendableAllowlist(
         ruleDefinitionEnvironment.getToolsLabel("//tools/allowlists/extend_rule_allowlist"));
@@ -646,38 +643,38 @@ public static StarlarkRuleFunction createRule(
     if (!buildSetting.equals(Starlark.NONE)) {
       builder.setBuildSetting((BuildSetting) buildSetting);
     }
+
+    TransitionFactory<RuleTransitionData> transitionFactory = null;
     if (!cfg.equals(Starlark.NONE)) {
       if (cfg instanceof StarlarkDefinedConfigTransition) {
+        // defined in Starlark via, cfg = transition
         StarlarkDefinedConfigTransition starlarkDefinedConfigTransition =
             (StarlarkDefinedConfigTransition) cfg;
-        builder.cfg(new StarlarkRuleTransitionProvider(starlarkDefinedConfigTransition));
+        transitionFactory = new StarlarkRuleTransitionProvider(starlarkDefinedConfigTransition);
         hasStarlarkDefinedTransition = true;
-      } else if (cfg instanceof PatchTransition) {
-        builder.cfg((PatchTransition) cfg);
       } else if (cfg instanceof StarlarkExposedRuleTransitionFactory) {
+        // only used for native Android transitions (platforms and feature flags)
         StarlarkExposedRuleTransitionFactory transition =
             (StarlarkExposedRuleTransitionFactory) cfg;
-        builder.cfg(transition);
         transition.addToStarlarkRule(ruleDefinitionEnvironment, builder);
-      } else if (cfg instanceof TransitionFactory) {
-        // This may be redundant with StarlarkExposedRuleTransitionFactory infra
-        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.");
-        }
+        transitionFactory = transition;
       } else {
         throw Starlark.errorf(
             "`cfg` must be set to a transition object initialized by the transition() function.");
       }
     }
+    if (parent != null && parent.getTransitionFactory() != null) {
+      if (transitionFactory == null) {
+        transitionFactory = parent.getTransitionFactory();
+      } else {
+        transitionFactory =
+            ComposingTransitionFactory.of(transitionFactory, parent.getTransitionFactory());
+      }
+      hasStarlarkDefinedTransition = true;
+    }
+    if (transitionFactory != null) {
+      builder.cfg(transitionFactory);
+    }
 
     boolean hasFunctionTransitionAllowlist = false;
     // Check for existence of the function transition allowlist attribute.
@@ -964,6 +961,16 @@ public StarlarkAspect aspect(
         ImmutableSet.copyOf(subrules));
   }
 
+  private static ImmutableSet<String> getLegacyAnyTypeAttrs(RuleClass ruleClass) {
+    Attribute attr = ruleClass.getAttributeByNameMaybe("$legacy_any_type_attrs");
+    if (attr == null
+        || attr.getType() != STRING_LIST
+        || !(attr.getDefaultValueUnchecked() instanceof List<?>)) {
+      return ImmutableSet.of();
+    }
+    return ImmutableSet.copyOf(STRING_LIST.cast(attr.getDefaultValueUnchecked()));
+  }
+
   /**
    * The implementation for the magic function "rule" that creates Starlark rule classes.
    *
@@ -1028,9 +1035,17 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
       if (ruleClass == null) {
         throw new EvalException("Invalid rule class hasn't been exported by a bzl file");
       }
+      PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
+      if (pkgContext == null) {
+        throw new EvalException(
+            "Cannot instantiate a rule when loading a .bzl file. "
+                + "Rules may be instantiated only in a BUILD thread.");
+      }
 
       validateRulePropagatedAspects(ruleClass);
 
+      ImmutableSet<String> legacyAnyTypeAttrs = getLegacyAnyTypeAttrs(ruleClass);
+
       // Remove {@link BazelStarlarkContext} to prevent calls to load and analysis time functions.
       // Mutating values in initializers is mostly not a problem, because the attribute values are
       // copied before calling the initializers (<-TODO) and before they are set on the target.
@@ -1050,8 +1065,6 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
             continue;
           }
 
-          // TODO: b/298561048 - lift parameters to more accurate type - for example strings to
-          // Labels
           // You might feel tempted to inspect the signature of the initializer function. The
           // temptation might come from handling default values, making them work for better for the
           // users.
@@ -1060,7 +1073,19 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
           for (var attr : currentRuleClass.getAttributes()) {
             if (attr.isPublic() && attr.starlarkDefined()) {
               if (kwargs.containsKey(attr.getName())) {
-                initializerKwargs.put(attr.getName(), kwargs.get(attr.getName()));
+                Object value = kwargs.get(attr.getName());
+                if (value == Starlark.NONE) {
+                  continue;
+                }
+                Object reifiedValue =
+                    legacyAnyTypeAttrs.contains(attr.getName())
+                        ? value
+                        : BuildType.copyAndLiftStarlarkValue(
+                            currentRuleClass.getName(),
+                            attr,
+                            value,
+                            pkgContext.getBuilder().getLabelConverter());
+                initializerKwargs.put(attr.getName(), reifiedValue);
               }
             }
           }
@@ -1091,7 +1116,19 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
               throw Starlark.errorf(
                   "Initializer can only set Starlark defined attributes, not '%s'", arg);
             }
-            kwargs.putEntry(nativeName, newKwargs.get(arg));
+            Object value = newKwargs.get(arg);
+            Object reifiedValue =
+                attr == null
+                        || value == Starlark.NONE
+                        || legacyAnyTypeAttrs.contains(attr.getName())
+                    ? value
+                    : BuildType.copyAndLiftStarlarkValue(
+                        currentRuleClass.getName(),
+                        attr,
+                        value,
+                        // Reify to the location of the initializer definition
+                        currentRuleClass.getLabelConverterForInitializer());
+            kwargs.putEntry(nativeName, reifiedValue);
           }
         }
       } finally {
@@ -1101,12 +1138,6 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
       BuildLangTypedAttributeValuesMap attributeValues =
           new BuildLangTypedAttributeValuesMap(kwargs);
       try {
-        PackageContext pkgContext = thread.getThreadLocal(PackageContext.class);
-        if (pkgContext == null) {
-          throw new EvalException(
-              "Cannot instantiate a rule when loading a .bzl file. "
-                  + "Rules may be instantiated only in a BUILD thread.");
-        }
         RuleFactory.createAndAddRule(
             pkgContext.getBuilder(),
             ruleClass,
@@ -1146,7 +1177,12 @@ public void export(EventHandler handler, Label starlarkLabel, String ruleClassNa
       // exploit dependency resolution for "free"
       ImmutableList<Pair<String, Descriptor>> subruleAttributes;
       try {
-        subruleAttributes = StarlarkSubrule.discoverAttributes(builder.getSubrules());
+        var parentSubrules = builder.getParentSubrules();
+        ImmutableList<StarlarkSubruleApi> subrulesNotInParents =
+            builder.getSubrules().stream()
+                .filter(subrule -> !parentSubrules.contains(subrule))
+                .collect(toImmutableList());
+        subruleAttributes = StarlarkSubrule.discoverAttributes(subrulesNotInParents);
       } catch (EvalException e) {
         errorf(handler, "%s", e.getMessage());
         return;

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

@@ -326,10 +326,14 @@ ImmutableSet<? extends StarlarkSubruleApi> getSubrules() {
     if (isForAspect()) {
       return getRuleContext().getMainAspect().getDefinition().getSubrules();
     } else {
-      return getRuleContext().getRule().getRuleClassObject().getSubrules();
+      return getRuleClassUnderEvaluation().getSubrules();
     }
   }
 
+  public RuleClass getRuleClassUnderEvaluation() {
+    return ruleClassUnderEvaluation;
+  }
+
   void setLockedForSubrule(@Nullable SubruleContext lockedBy) {
     this.lockedForSubruleEvaluation = lockedBy;
   }

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

@@ -192,7 +192,7 @@ private EvalException getUndeclaredSubruleError(StarlarkRuleContext starlarkRule
     } else {
       return Starlark.errorf(
           "rule '%s' must declare '%s' in 'subrules'",
-          starlarkRuleContext.getRuleContext().getRule().getRuleClass(), this.getName());
+          starlarkRuleContext.getRuleClassUnderEvaluation(), this.getName());
     }
   }