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
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);
}
}
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
