Support declared providers for aspects

Fixes #2016

--
PiperOrigin-RevId: 149102037
MOS_MIGRATED_REVID=149102037

1 files changed, 61 insertions, 19 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index e80a788aef..bd0900b8a5 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -31,6 +31,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.Environment;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.EvalExceptionWithStackTrace;
+import com.google.devtools.build.lib.syntax.EvalUtils;
 import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import java.util.Map;
@@ -65,7 +66,7 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
               .setGlobals(skylarkAspect.getFuncallEnv().getGlobals())
               .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
               .build(); // NB: loading phase functions are not available: this is analysis already,
-                        // so we do *not* setLoadingPhase().
+      // so we do *not* setLoadingPhase().
       Object aspectSkylarkObject;
       try {
         aspectSkylarkObject =
@@ -79,26 +80,15 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
 
         if (ruleContext.hasErrors()) {
           return null;
-        } else if (!(aspectSkylarkObject instanceof SkylarkClassObject)) {
-          ruleContext.ruleError("Aspect implementation doesn't return a struct");
+        } else if (!(aspectSkylarkObject instanceof SkylarkClassObject)
+            && !(aspectSkylarkObject instanceof Iterable)) {
+          ruleContext.ruleError(
+              String.format(
+                  "Aspect implementation should return a struct or a list, but got %s",
+                  SkylarkType.typeOf(aspectSkylarkObject)));
           return null;
         }
-
-        ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(
-            aspectDescriptor, ruleContext);
-
-        SkylarkClassObject struct = (SkylarkClassObject) aspectSkylarkObject;
-        Location loc = struct.getCreationLoc();
-        for (String key : struct.getKeys()) {
-          if (key.equals("output_groups")) {
-            addOutputGroups(struct.getValue(key), loc, builder);
-          } else {
-            builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
-          }
-        }
-        ConfiguredAspect configuredAspect = builder.build();
-        SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
-        return configuredAspect;
+        return createAspect(aspectSkylarkObject, aspectDescriptor, ruleContext);
       } catch (EvalException e) {
         addAspectToStackTrace(base, e);
         ruleContext.ruleError("\n" + e.print());
@@ -107,6 +97,58 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory {
     }
   }
 
+  private ConfiguredAspect createAspect(
+      Object aspectSkylarkObject, AspectDescriptor aspectDescriptor, RuleContext ruleContext)
+      throws EvalException {
+
+    ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(aspectDescriptor, ruleContext);
+
+    if (aspectSkylarkObject instanceof Iterable) {
+      addDeclaredProviders(builder, (Iterable) aspectSkylarkObject);
+    } else {
+      SkylarkClassObject struct = (SkylarkClassObject) aspectSkylarkObject;
+      Location loc = struct.getCreationLoc();
+      for (String key : struct.getKeys()) {
+        if (key.equals("output_groups")) {
+          addOutputGroups(struct.getValue(key), loc, builder);
+        } else if (key.equals("providers")) {
+          Object value = struct.getValue(key);
+          Iterable providers =
+              SkylarkType.cast(
+                  value,
+                  Iterable.class,
+                  loc,
+                  "The value for \"providers\" should be a list of declared providers, "
+                      + "got %s instead",
+                  EvalUtils.getDataTypeName(value, false));
+          addDeclaredProviders(builder, providers);
+        } else {
+          builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
+        }
+      }
+    }
+
+    ConfiguredAspect configuredAspect = builder.build();
+    SkylarkProviderValidationUtil.checkOrphanArtifacts(ruleContext);
+    return configuredAspect;
+  }
+
+  private void addDeclaredProviders(ConfiguredAspect.Builder builder, Iterable aspectSkylarkObject)
+      throws EvalException {
+    for (Object o : aspectSkylarkObject) {
+      Location loc = skylarkAspect.getImplementation().getLocation();
+      SkylarkClassObject declaredProvider =
+          SkylarkType.cast(
+              o,
+              SkylarkClassObject.class,
+              loc,
+              "A return value of an aspect implementation function should be "
+                  + "a sequence of declared providers");
+      Location creationLoc = declaredProvider.getCreationLocOrNull();
+      builder.addSkylarkDeclaredProvider(declaredProvider, creationLoc != null ? creationLoc : loc);
+    }
+  }
+
   private static void addOutputGroups(Object value, Location loc,
       ConfiguredAspect.Builder builder)
       throws EvalException {