Deletes @AutoCodec annotation on AspectsListBuilder.StarlarkAspectDet…

…ails. This makes it necessary to serialize the parametersExtractor present in the base AspectDetails class from StarlarkDefinedAspect.getDefaultParametersExtractor. PiperOrigin-RevId: 424759672
CodeIntelligenceTesting · Jan 28, 2022 · 3fb43f1 · 3fb43f1
1 parent 3d37dc2
commit 3fb43f1
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 43 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java b/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java
@@ -21,7 +21,6 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import java.util.HashMap;
@@ -111,7 +110,6 @@ public boolean validateTopLevelAspectsParameters(ImmutableMap<String, String> as
   }
 
   /** Wraps the information necessary to construct an Aspect. */
-  @VisibleForSerialization
   public abstract static class AspectDetails<C extends AspectClass> {
     final C aspectClass;
     final Function<Rule, AspectParameters> parametersExtractor;
@@ -181,13 +179,10 @@ protected Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters
     }
   }
 
-  @VisibleForSerialization
-  @AutoCodec
-  static class StarlarkAspectDetails extends AspectDetails<StarlarkAspectClass> {
+  private static class StarlarkAspectDetails extends AspectDetails<StarlarkAspectClass> {
     private final StarlarkDefinedAspect aspect;
 
-    @VisibleForSerialization
-    StarlarkAspectDetails(StarlarkDefinedAspect aspect, String baseAspectName) {
+    private StarlarkAspectDetails(StarlarkDefinedAspect aspect, String baseAspectName) {
       super(aspect.getAspectClass(), aspect.getDefaultParametersExtractor(), baseAspectName);
       this.aspect = aspect;
     }
@@ -237,7 +232,7 @@ public Aspect getTopLevelAspect(ImmutableMap<String, String> aspectParameters)
     }
   }
 
-  @SerializationConstant @AutoCodec.VisibleForSerialization
+  @SerializationConstant @VisibleForSerialization
   static final Function<Rule, AspectParameters> EMPTY_FUNCTION = input -> AspectParameters.EMPTY;
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java
@@ -223,46 +223,51 @@ public boolean isExported() {
 
   @Override
   public Function<Rule, AspectParameters> getDefaultParametersExtractor() {
-    return rule -> {
-      AttributeMap ruleAttrs = RawAttributeMapper.of(rule);
-      AspectParameters.Builder builder = new AspectParameters.Builder();
-      for (Attribute aspectAttr : attributes) {
-        String param = aspectAttr.getName();
-        if (Attribute.isImplicit(param) || Attribute.isLateBound(param)) {
-          // These attributes are the private matters of the aspect
-          continue;
-        }
+    // This is serialized along with AspectsListBuilder.StarlarkAspectDetail and should not be
+    // turned into a lambda.
+    return new Function<Rule, AspectParameters>() {
+      @Override
+      public AspectParameters apply(Rule rule) {
+        AttributeMap ruleAttrs = RawAttributeMapper.of(rule);
+        AspectParameters.Builder builder = new AspectParameters.Builder();
+        for (Attribute aspectAttr : attributes) {
+          String param = aspectAttr.getName();
+          if (Attribute.isImplicit(param) || Attribute.isLateBound(param)) {
+            // These attributes are the private matters of the aspect
+            continue;
+          }
 
-        Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param);
-        if (paramAttributes.contains(aspectAttr.getName())) {
-          // These are preconditions because if they are false, RuleFunction.call() should
-          // already have generated an error.
-          Preconditions.checkArgument(
-              ruleAttr != null,
-              "Cannot apply aspect %s to %s that does not define attribute '%s'.",
-              getName(),
-              rule.getTargetKind(),
-              param);
-          Preconditions.checkArgument(
-              ruleAttr.getType() == Type.STRING
-                  || ruleAttr.getType() == Type.INTEGER
-                  || ruleAttr.getType() == Type.BOOLEAN,
-              "Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor"
-                  + " string.",
-              getName(),
-              rule.getTargetKind(),
-              param);
-        }
+          Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param);
+          if (paramAttributes.contains(aspectAttr.getName())) {
+            // These are preconditions because if they are false, RuleFunction.call() should
+            // already have generated an error.
+            Preconditions.checkArgument(
+                ruleAttr != null,
+                "Cannot apply aspect %s to %s that does not define attribute '%s'.",
+                getName(),
+                rule.getTargetKind(),
+                param);
+            Preconditions.checkArgument(
+                ruleAttr.getType() == Type.STRING
+                    || ruleAttr.getType() == Type.INTEGER
+                    || ruleAttr.getType() == Type.BOOLEAN,
+                "Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor"
+                    + " string.",
+                getName(),
+                rule.getTargetKind(),
+                param);
+          }
 
-        if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
-          // If the attribute has a select() (which aspect attributes don't yet support), the
-          // error gets reported in RuleClass.checkAspectAllowedValues.
-          if (!ruleAttrs.isConfigurable(param)) {
-            builder.addAttribute(param, ruleAttrs.get(param, ruleAttr.getType()).toString());
+          if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) {
+            // If the attribute has a select() (which aspect attributes don't yet support), the
+            // error gets reported in RuleClass.checkAspectAllowedValues.
+            if (!ruleAttrs.isConfigurable(param)) {
+              builder.addAttribute(param, ruleAttrs.get(param, ruleAttr.getType()).toString());
+            }
           }
         }
+        return builder.build();
       }
-      return builder.build();
     };
   }