Accept a single provider instance as return value from an aspect.

Previously, if a single provider instance was returned, it was silently dropped.

This makes the behavior consistent with rule implementation functions.
Fixes #5955

RELNOTES: None.
PiperOrigin-RevId: 209777202

src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java

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 12a3e98..150cfb4 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
@@ -29,6 +29,7 @@
 import com.google.devtools.build.lib.packages.InfoInterface;
 import com.google.devtools.build.lib.packages.SkylarkDefinedAspect;
 import com.google.devtools.build.lib.packages.StructImpl;
+import com.google.devtools.build.lib.packages.StructProvider;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
 import com.google.devtools.build.lib.syntax.Environment;
@@ -86,10 +87,11 @@
         if (ruleContext.hasErrors()) {
           return null;
         } else if (!(aspectSkylarkObject instanceof StructImpl)
-            && !(aspectSkylarkObject instanceof Iterable)) {
+            && !(aspectSkylarkObject instanceof Iterable)
+            && !(aspectSkylarkObject instanceof InfoInterface)) {
           ruleContext.ruleError(
-              String.format(
-                  "Aspect implementation should return a struct or a list, but got %s",
+              String.format("Aspect implementation should return a struct, a list, or a provider "
+                      + "instance, but got %s",
                   SkylarkType.typeOf(aspectSkylarkObject)));
           return null;
         }
@@ -115,25 +117,32 @@
     if (aspectSkylarkObject instanceof Iterable) {
       addDeclaredProviders(builder, (Iterable) aspectSkylarkObject);
     } else {
-      StructImpl struct = (StructImpl) aspectSkylarkObject;
-      Location loc = struct.getCreationLoc();
-      for (String field : struct.getFieldNames()) {
-        if (field.equals("output_groups")) {
-          addOutputGroups(struct.getValue(field), loc, builder);
-        } else if (field.equals("providers")) {
-          Object value = struct.getValue(field);
-          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(field, struct.getValue(field), loc);
+      // Either an old-style struct or a single declared provider (not in a list)
+      InfoInterface info = (InfoInterface) aspectSkylarkObject;
+      Location loc = info.getCreationLoc();
+      if (info.getProvider().getKey().equals(StructProvider.STRUCT.getKey())) {
+        // Old-style struct, that may contain declared providers.
+        StructImpl struct = (StructImpl) aspectSkylarkObject;
+        for (String field : struct.getFieldNames()) {
+          if (field.equals("output_groups")) {
+            addOutputGroups(struct.getValue(field), loc, builder);
+          } else if (field.equals("providers")) {
+            Object value = struct.getValue(field);
+            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(field, struct.getValue(field), loc);
+          }
         }
+      } else {
+        builder.addSkylarkDeclaredProvider(info);
       }
     }
 

src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index 248d609..c39a1fc 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -89,6 +89,30 @@
   }
 
   @Test
+  public void aspectWithSingleDeclaredProvider() throws Exception {
+    scratch.file(
+        "test/aspect.bzl",
+        "foo = provider()",
+        "def _impl(target, ctx):",
+        "   return foo()",
+        "MyAspect = aspect(implementation=_impl)");
+    scratch.file("test/BUILD", "java_library(name = 'xxx',)");
+
+    AnalysisResult analysisResult =
+        update(ImmutableList.of("test/aspect.bzl%MyAspect"), "//test:xxx");
+    assertThat(getLabelsToBuild(analysisResult)).containsExactly("//test:xxx");
+    assertThat(getAspectDescriptions(analysisResult))
+        .containsExactly("//test:aspect.bzl%MyAspect(//test:xxx)");
+    ConfiguredAspect configuredAspect = Iterables.getOnlyElement(analysisResult.getAspects())
+        .getConfiguredAspect();
+
+    SkylarkKey fooKey =
+        new SkylarkKey(Label.parseAbsolute("//test:aspect.bzl", ImmutableMap.of()), "foo");
+
+    assertThat(configuredAspect.get(fooKey).getProvider().getKey()).isEqualTo(fooKey);
+  }
+
+  @Test
   public void aspectWithDeclaredProviders() throws Exception {
     scratch.file(
         "test/aspect.bzl",
@@ -868,7 +892,8 @@
     } catch (ViewCreationFailedException e) {
       // expect to fail.
     }
-    assertContainsEvent("Aspect implementation should return a struct or a list, but got int");
+    assertContainsEvent("Aspect implementation should return a struct, a list, or a provider "
+        + "instance, but got int");
   }
 
   @Test