Expose ObjcProtoAspect to Skylark.

RELNOTES: None.
PiperOrigin-RevId: 179737025

4 files changed, 138 insertions, 28 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java
index e0399b11c1..119701661b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspectTest.java
@@ -205,6 +205,52 @@ public final class ObjcProtoAspectTest extends ObjcRuleTestCase {
                 + "/x/_proto_filters/objc_proto_2/generated_filter_file.pbascii");
   }
 
+  @Test
+  public void testObjcProtoAspectPropagatesProviderThroughSkylarkRule() throws Exception {
+    scratch.file("test_skylark/BUILD");
+    scratch.file(
+        "test_skylark/top_level_stub.bzl",
+        "def top_level_stub_impl(ctx):",
+        "  deps = hasattr(ctx.attr.deps[0], 'ObjcProto')",
+        "  return struct(dep = ctx.attr.deps[0])",
+        "top_level_stub = rule(",
+        "    top_level_stub_impl,",
+        "    attrs = {",
+        "        'deps': attr.label_list(",
+        "             aspects=[apple_common.objc_proto_aspect],",
+        "        ),",
+        "    },",
+        "    fragments = ['apple'],",
+        ")");
+
+    scratch.file(
+        "x/BUILD",
+        "proto_library(",
+        "  name = 'protos',",
+        "  srcs = ['data.proto'],",
+        ")",
+        "objc_proto_library(",
+        "  name = 'x',",
+        "  deps = [':protos'],",
+        "  portable_proto_filters = ['data_filter.pbascii'],",
+        ")");
+
+    scratch.file(
+        "bin/BUILD",
+        "load('//test_skylark:top_level_stub.bzl', 'top_level_stub')",
+        "top_level_stub(",
+        "  name = 'link_target',",
+        "  deps = ['//x:x'],",
+        ")");
+
+    ConfiguredTarget topTarget = getConfiguredTarget("//bin:link_target");
+
+    ConfiguredTarget depTarget = (ConfiguredTarget) topTarget.get("dep");
+    ObjcProtoProvider objcProtoProvider = depTarget.get(ObjcProtoProvider.SKYLARK_CONSTRUCTOR);
+
+    assertThat(objcProtoProvider).isNotNull();
+  }
+
   private ConfiguredTarget getObjcProtoAspectConfiguredTarget(String label) throws Exception {
     scratch.file(
         "bin/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/BUILD b/src/test/java/com/google/devtools/build/lib/skylark/BUILD
index 525f9be2f6..ca3ef36b21 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skylark/BUILD
@@ -63,6 +63,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
+        "//src/main/java/com/google/devtools/build/lib/rules/objc",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index 1ad16997e0..a5353e1df2 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -35,28 +35,38 @@ import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.Info;
 import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
+import com.google.devtools.build.lib.packages.util.MockObjcSupport;
+import com.google.devtools.build.lib.packages.util.MockProtoSupport;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
 import com.google.devtools.build.lib.rules.java.Jvm;
+import com.google.devtools.build.lib.rules.objc.ObjcProtoProvider;
 import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.syntax.SkylarkList;
 import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import java.util.Arrays;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/**
- * Tests for Skylark aspects
- */
+/** Tests for Skylark aspects */
 @RunWith(JUnit4.class)
-public class SkylarkAspectsTest extends AnalysisTestCase {
+public class SkylarkDefinedAspectsTest extends AnalysisTestCase {
   protected boolean keepGoing() {
     return false;
   }
 
   private static final String LINE_SEPARATOR = System.lineSeparator();
 
+  @Before
+  public final void initializeToolsConfigMock() throws Exception {
+    // Required for tests including the objc_library rule.
+    MockObjcSupport.setup(mockToolsConfig);
+    // Required for tests including the proto_library rule.
+    MockProtoSupport.setup(mockToolsConfig);
+  }
+
   @Test
   public void simpleAspect() throws Exception {
     scratch.file(
@@ -1165,7 +1175,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     }
     assertContainsEvent(
         "Extension file not found. Unable to load file '//test:aspect.bzl': "
-        + "file doesn't exist or isn't a file");
+            + "file doesn't exist or isn't a file");
   }
 
   @Test
@@ -1182,8 +1192,8 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     }
     assertContainsEvent(
         "Every .bzl file must have a corresponding package, but 'foo' does not have one. "
-        + "Please create a BUILD file in the same or any parent directory. "
-        + "Note that this BUILD file does not need to do anything except exist.");
+            + "Please create a BUILD file in the same or any parent directory. "
+            + "Note that this BUILD file does not need to do anything except exist.");
   }
 
   @Test
@@ -1214,9 +1224,9 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     } catch (Exception e) {
       // expect to fail.
     }
-    assertContainsEvent(//"ERROR /workspace/test/aspect.bzl:9:11: "
+    assertContainsEvent( // "ERROR /workspace/test/aspect.bzl:9:11: "
         "Aspect //test:aspect.bzl%MyAspectUncovered requires rule my_rule to specify attribute "
-        + "'my_attr' with type string.");
+            + "'my_attr' with type string.");
   }
 
   @Test
@@ -1235,7 +1245,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
         "    implementation=_rule_impl,",
         "    attrs = { 'deps' : attr.label_list(aspects=[MyAspectMismatch]),",
         "              'my_attr' : attr.int() },",
-       ")");
+        ")");
     scratch.file("test/BUILD",
         "load('//test:aspect.bzl', 'my_rule')",
         "my_rule(name = 'xxx', my_attr = 4)");
@@ -1250,7 +1260,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     }
     assertContainsEvent(
         "Aspect //test:aspect.bzl%MyAspectMismatch requires rule my_rule to specify attribute "
-        + "'my_attr' with type string.");
+            + "'my_attr' with type string.");
   }
 
   @Test
@@ -1659,7 +1669,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
   }
 
   @Test
-  public void sharedAttributeDefintionWithAspects() throws Exception {
+  public void sharedAttributeDefinitionWithAspects() throws Exception {
     scratch.file(
         "test/aspect.bzl",
         "def _aspect_impl(target,ctx):",
@@ -2104,7 +2114,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
     }
     assertContainsEvent(
         "Aspect '//test:aspect.bzl%my_aspect', applied to '//test:xxx', "
-        + "does not provide advertised provider 'foo'");
+            + "does not provide advertised provider 'foo'");
   }
 
   @Test
@@ -2494,9 +2504,62 @@ public class SkylarkAspectsTest extends AnalysisTestCase {
             Label.parseAbsolute("//test:baz"));
   }
 
+  @Test
+  // This test verifies that aspects which are defined natively and exported for use in skylark
+  // can be referenced at the top level using the --aspects flag. For ease of testing,
+  // apple_common.objc_proto_aspect is used as an example.
+  public void testTopLevelSkylarkObjcProtoAspect() throws Exception {
+    scratch.file("test_skylark/BUILD");
+    scratch.file(
+        "test_skylark/top_level_stub.bzl",
+        "top_level_aspect = apple_common.objc_proto_aspect",
+        "",
+        "def top_level_stub_impl(ctx):",
+        "  return struct()",
+        "top_level_stub = rule(",
+        "    top_level_stub_impl,",
+        "    attrs = {",
+        "        'deps': attr.label_list(),",
+        "    },",
+        "    fragments = ['apple'],",
+        ")");
+
+    scratch.file(
+        "x/BUILD",
+        "proto_library(",
+        "  name = 'protos',",
+        "  srcs = ['data.proto'],",
+        ")",
+        "objc_proto_library(",
+        "  name = 'x',",
+        "  deps = [':protos'],",
+        "  portable_proto_filters = ['data_filter.pbascii'],",
+        ")");
+
+    scratch.file(
+        "bin/BUILD",
+        "load('//test_skylark:top_level_stub.bzl', 'top_level_stub')",
+        "top_level_stub(",
+        "  name = 'link_target',",
+        "  deps = ['//x:x'],",
+        ")");
+
+    useConfiguration(MockObjcSupport.requiredObjcCrosstoolFlags().toArray(new String[1]));
+    AnalysisResult analysisResult =
+        update(
+            ImmutableList.of("test_skylark/top_level_stub.bzl%top_level_aspect"),
+            "//bin:link_target");
+    ConfiguredAspect configuredAspect =
+        Iterables.getOnlyElement(analysisResult.getAspects()).getConfiguredAspect();
+
+    ObjcProtoProvider objcProtoProvider =
+        (ObjcProtoProvider) configuredAspect.get(ObjcProtoProvider.SKYLARK_CONSTRUCTOR.getKey());
+    assertThat(objcProtoProvider).isNotNull();
+  }
+
   /** SkylarkAspectTest with "keep going" flag */
   @RunWith(JUnit4.class)
-  public static final class WithKeepGoing extends SkylarkAspectsTest {
+  public static final class WithKeepGoing extends SkylarkDefinedAspectsTest {
     @Override
     protected FlagBuilder defaultFlags() {
       return new FlagBuilder().with(Flag.KEEP_GOING);
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index d947562206..202e28925b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -40,8 +40,8 @@ import com.google.devtools.build.lib.packages.PredicateWithMessage;
 import com.google.devtools.build.lib.packages.RequiredProviders;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
-import com.google.devtools.build.lib.packages.SkylarkAspect;
 import com.google.devtools.build.lib.packages.SkylarkAspectClass;
+import com.google.devtools.build.lib.packages.SkylarkDefinedAspect;
 import com.google.devtools.build.lib.packages.SkylarkInfo;
 import com.google.devtools.build.lib.packages.SkylarkProvider;
 import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
@@ -321,7 +321,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
             "my_aspect = aspect(implementation = _impl)",
             "a = attr.label_list(aspects = [my_aspect])");
     SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) ev.lookup("a");
-            SkylarkAspect aspect = (SkylarkAspect) ev.lookup("my_aspect");
+    SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
     assertThat(aspect).isNotNull();
     assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass());
   }
@@ -334,7 +334,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "my_aspect = aspect(implementation = _impl)",
         "a = attr.label(aspects = [my_aspect])");
     SkylarkAttr.Descriptor attr = (SkylarkAttr.Descriptor) ev.lookup("a");
-    SkylarkAspect aspect = (SkylarkAspect) ev.lookup("my_aspect");
+    SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
     assertThat(aspect).isNotNull();
     assertThat(attr.build("xxx").getAspectClasses()).containsExactly(aspect.getAspectClass());
   }
@@ -358,7 +358,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "my_aspect = aspect(_impl,",
         "   attrs = { '_extra_deps' : attr.label(default = Label('//foo/bar:baz')) }",
         ")");
-    SkylarkAspect aspect = (SkylarkAspect) ev.lookup("my_aspect");
+    SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
     Attribute attribute = Iterables.getOnlyElement(aspect.getAttributes());
     assertThat(attribute.getName()).isEqualTo("$extra_deps");
     assertThat(attribute.getDefaultValue(null))
@@ -373,7 +373,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "my_aspect = aspect(_impl,",
         "   attrs = { 'param' : attr.string(values=['a', 'b']) }",
         ")");
-    SkylarkAspect aspect = (SkylarkAspect) ev.lookup("my_aspect");
+    SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
     Attribute attribute = Iterables.getOnlyElement(aspect.getAttributes());
     assertThat(attribute.getName()).isEqualTo("param");
   }
@@ -411,7 +411,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "   attrs = { 'param' : attr.string(values=['a', 'b']),",
         "             '_extra' : attr.label(default = Label('//foo/bar:baz')) }",
         ")");
-    SkylarkAspect aspect = (SkylarkAspect) ev.lookup("my_aspect");
+    SkylarkDefinedAspect aspect = (SkylarkDefinedAspect) ev.lookup("my_aspect");
     assertThat(aspect.getAttributes()).hasSize(2);
     assertThat(aspect.getParamAttributes()).containsExactly("param");
   }
@@ -432,7 +432,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
     scratch.file("test/BUILD", "toolchain_type(name = 'my_toolchain_type')");
     evalAndExport(
         "def _impl(ctx): pass", "a1 = aspect(_impl, toolchains=['//test:my_toolchain_type'])");
-    SkylarkAspect a = (SkylarkAspect) lookup("a1");
+    SkylarkDefinedAspect a = (SkylarkDefinedAspect) lookup("a1");
     assertThat(a.getRequiredToolchains()).containsExactly(makeLabel("//test:my_toolchain_type"));
   }
 
@@ -1327,7 +1327,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "   pass",
         "my_aspect = aspect(_impl, attr_aspects=['*'])");
 
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     assertThat(myAspect.getDefinition(AspectParameters.EMPTY).propagateAlong(
         Attribute.attr("foo", BuildType.LABEL).allowedFileTypes().build()
     )).isTrue();
@@ -1341,7 +1341,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "cc = provider()",
         "my_aspect = aspect(_impl, required_aspect_providers=['java', cc])"
     );
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     RequiredProviders requiredProviders = myAspect.getDefinition(AspectParameters.EMPTY)
         .getRequiredProvidersForAspects();
     assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue();
@@ -1367,7 +1367,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "cc = provider()",
         "my_aspect = aspect(_impl, required_aspect_providers=[['java'], [cc]])"
     );
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     RequiredProviders requiredProviders = myAspect.getDefinition(AspectParameters.EMPTY)
         .getRequiredProvidersForAspects();
     assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isTrue();
@@ -1396,7 +1396,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "   pass",
         "my_aspect = aspect(_impl, required_aspect_providers=[])"
     );
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     RequiredProviders requiredProviders = myAspect.getDefinition(AspectParameters.EMPTY)
         .getRequiredProvidersForAspects();
     assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isFalse();
@@ -1410,7 +1410,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "   pass",
         "my_aspect = aspect(_impl)"
     );
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     RequiredProviders requiredProviders = myAspect.getDefinition(AspectParameters.EMPTY)
         .getRequiredProvidersForAspects();
     assertThat(requiredProviders.isSatisfiedBy(AdvertisedProviderSet.ANY)).isFalse();
@@ -1425,7 +1425,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "y = provider()",
         "my_aspect = aspect(_impl, provides = ['x', y])"
     );
-    SkylarkAspect myAspect = (SkylarkAspect) lookup("my_aspect");
+    SkylarkDefinedAspect myAspect = (SkylarkDefinedAspect) lookup("my_aspect");
     AdvertisedProviderSet advertisedProviders = myAspect.getDefinition(AspectParameters.EMPTY)
         .getAdvertisedProviders();
     assertThat(advertisedProviders.canHaveAnyProvider()).isFalse();
@@ -1476,7 +1476,7 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "]"
     );
     SkylarkProvider p = (SkylarkProvider) lookup("p");
-    SkylarkAspect a = (SkylarkAspect) lookup("a");
+    SkylarkDefinedAspect a = (SkylarkDefinedAspect) lookup("a");
     SkylarkProvider p1 = (SkylarkProvider) lookup("p1");
     assertThat(p.getPrintableName()).isEqualTo("p");
     assertThat(p.getKey()).isEqualTo(new SkylarkProvider.SkylarkKey(FAKE_LABEL, "p"));