Return opaque value for native.rule() on a select() statement.

This is imperfect in many ways:

 1) the value is opaque, and not a BUILD value, so it cannot be used
 in rule arguments

 2) its representation has a pointer address, so it breaks hermeticity.

Despite its problem, we return this value because otherwise
native.rules() fails if there is any rule using a select() in the
BUILD file.

A future solution would be to convert BuildType.SelectorList back to
syntax.SelectorList. To do so, we would have to

1) recurse into the Selector contents of SelectorList, so those values
are run through skylarkifyValue too

2) get the right Class<?> value. We could probably get at that by looking at
   ((SelectorList)val).getSelectors().first().getEntries().first().getClass().

--
MOS_MIGRATED_REVID=113051612

2 files changed, 36 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 824ecc1377..45d1e0e143 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -892,10 +892,12 @@ public final class PackageFactory {
    * Returns null if we don't want to export the value.
    *
    * <p>All of the types returned are immutable. If we want, we can change this to
-   * immutable in the future, but this is the safe choice for now.o
+   * immutable in the future, but this is the safe choice for now.
    */
   @Nullable
   private static Object skylarkifyValue(Object val, Package pkg) throws NotRepresentableException {
+    // TODO(bazel-team): the location of this function is ad-hoc. Arguably, the conversion
+    // from Java native types to Skylark types should be part of the Type class hierarchy,
     if (val == null) {
       return null;
     }
@@ -909,7 +911,6 @@ public final class PackageFactory {
       return val;
     }
 
-    // Maybe we should have an interface for types so they can represent themselves to skylark?
     if (val instanceof TriState) {
       switch ((TriState) val) {
         case AUTO:
@@ -972,6 +973,22 @@ public final class PackageFactory {
       return null;
     }
 
+    if (val instanceof BuildType.SelectorList) {
+      // This is terrible:
+      //  1) this value is opaque, and not a BUILD value, so it cannot be used in rule arguments
+      //  2) its representation has a pointer address, so it breaks hermeticity.
+      //
+      // Even though this is clearly imperfect, we return this value because otherwise
+      // native.rules() fails if there is any rule using a select() in the BUILD file.
+      //
+      // To remedy this, we should return a syntax.SelectorList. To do so, we have to
+      // 1) recurse into the Selector contents of SelectorList, so those values are skylarkified too
+      // 2) get the right Class<?> value. We could probably get at that by looking at
+      //    ((SelectorList)val).getSelectors().first().getEntries().first().getClass().
+
+      return val;
+    }
+
     // We are explicit about types we don't understand so we minimize changes to existing callers
     // if we add more types that we can represent.
     throw new NotRepresentableException(
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 088a542c89..240b5878b1 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -354,6 +354,23 @@ public class SkylarkRuleContextTest extends SkylarkTestCase {
   }
 
   @Test
+  public void testGetRuleSelect() throws Exception {
+    scratch.file("test/skylark/BUILD");
+    scratch.file("test/skylark/rulestr.bzl", "def rule_dict(name):", "  return native.rule(name)");
+
+    scratch.file(
+        "test/getrule/BUILD",
+        "load('/test/skylark/rulestr', 'rule_dict')",
+        "cc_library(name ='x', ",
+        "  srcs = select({'//conditions:default': []})",
+        ")",
+        "rule_dict('x')");
+
+    // Parse the BUILD file, to make sure select() makes it out of native.rule().
+    createRuleContext("//test/getrule:x");
+  }
+
+  @Test
   public void testGetRule() throws Exception {
     scratch.file("test/skylark/BUILD");
     scratch.file(