Restore Skylark "support" for inheritance

Classes of the same EvalUtils.getSkylarkType are once again the same for the
type inferencer.

Also, for debugging purposes, only print SkylarkClassObject as struct, not all classes that implement ClassObject yet are considered disjoint by the type engine.

--
MOS_MIGRATED_REVID=87933890

3 files changed, 25 insertions, 17 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java b/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
index 3b1cccfb71..1c3b3e29b8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ClassObject.java
@@ -53,6 +53,7 @@ public interface ClassObject {
   /**
    * An implementation class of ClassObject for structs created in Skylark code.
    */
+  // TODO(bazel-team): maybe move the SkylarkModule annotation to the ClassObject interface?
   @Immutable
   @SkylarkModule(name = "struct",
       doc = "A special language element to support structs (i.e. simple value objects). "
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index f5dd75368b..15eaaa20c5 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -137,8 +137,14 @@ public abstract class EvalUtils {
 
   // TODO(bazel-team): move the following few type-related functions to SkylarkType
   /**
-   * Returns the Skylark equivalent type of the parameter. Note that the Skylark
-   * language doesn't have inheritance.
+   * Compute the super-class of a class that Skylark considers as the Skylark type of its instances
+   *
+   * <p>Skylark type validation isn't otherwise equipped to deal with inheritance, so we must tell
+   * it which is the super-class or interface that matters for Skylark type compatibility.
+   * e.g. instances of all subclasses of SkylarkList are considered as being of type SkylarkList.
+   *
+   * @param c a class
+   * @return a super-class of c to be used in validation-time type inference.
    */
   public static Class<?> getSkylarkType(Class<?> c) {
     if (ImmutableList.class.isAssignableFrom(c)) {
@@ -155,6 +161,8 @@ public abstract class EvalUtils {
     } else if (Set.class.isAssignableFrom(c)) {
       return Set.class;
     } else {
+      // TODO(bazel-team): also unify all ClassObject, that we print the same?
+      //
       // Check if one of the superclasses or implemented interfaces has the SkylarkModule
       // annotation. If yes return that class.
       Class<?> parent = getParentWithSkylarkModule(c);
@@ -222,7 +230,7 @@ public abstract class EvalUtils {
       return "FilesetEntry";
     } else if (NestedSet.class.isAssignableFrom(c) || SkylarkNestedSet.class.isAssignableFrom(c)) {
       return "set";
-    } else if (ClassObject.class.isAssignableFrom(c)) {
+    } else if (ClassObject.SkylarkClassObject.class.isAssignableFrom(c)) {
       return "struct";
     } else if (SkylarkList.class.isAssignableFrom(c)) {
       // TODO(bazel-team): Refactor the class hierarchy so we can distinguish list and tuple types.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
index d259898c37..d98062b032 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkType.java
@@ -139,10 +139,6 @@ public abstract class SkylarkType {
     return intersection(this, other).equals(other);
   }
 
-  public boolean includes(Class<?> other) {
-    return includes(Simple.of(other));
-  }
-
   public SkylarkType getArgType() {
     return TOP;
   }
@@ -152,18 +148,18 @@ public abstract class SkylarkType {
   // Notable types
 
   /** A singleton for the TOP type, that at analysis time means that any type is possible. */
-  public static final Top TOP = new Top();
+  public static final Simple TOP = new Top();
 
   /** UNKNOWN, an alias for the TOP type, for backward compatibility */
-  public static final Top UNKNOWN = TOP;
+  public static final Simple UNKNOWN = TOP;
 
   /** A singleton for the ANY type, that at run time means that any object is possible. */
   // NB: right now, it has the same representation as TOP or UNKNOWN,
   // but means something subtly different.
-  public static final SkylarkType ANY = Simple.of(Object.class);
+  public static final Simple ANY = TOP;
 
   /** A singleton for the BOTTOM type, that contains no element */
-  public static final Bottom BOTTOM = new Bottom();
+  public static final Simple BOTTOM = new Bottom();
 
   /** NONE, the Unit type, isomorphic to Void, except its unique element prints as None */
   // Note that we currently consider at validation time that None is in every type,
@@ -189,7 +185,7 @@ public abstract class SkylarkType {
   public static final Simple BOOL = Simple.of(Boolean.class);
 
   /** The STRUCT type, for all Struct's */
-  public static final Simple STRUCT = Simple.of(ClassObject.class);
+  public static final Simple STRUCT = Simple.of(ClassObject.SkylarkClassObject.class);
 
   /** The FUNCTION type, that contains all functions, otherwise dynamically typed at call-time */
   public static final SkylarkFunctionType FUNCTION = new SkylarkFunctionType("unknown", TOP);
@@ -296,10 +292,15 @@ public abstract class SkylarkType {
         // i.e. None like null in Java is in every type.
         // TODO(bazel-team): Should we have .contains also always return true for NONE?
         simple = TOP;
-      } else if (ClassObject.class != type && ClassObject.class.isAssignableFrom(type)) {
-        simple = of(ClassObject.class);
       } else {
-        simple = new Simple(type);
+        // Consider all classes that have the same EvalUtils.getSkylarkType() as equivalent,
+        // as a substitute to handling inheritance.
+        Class<?> skylarkType = EvalUtils.getSkylarkType(type);
+        if (skylarkType != type) {
+          simple = Simple.of(skylarkType);
+        } else {
+          simple = new Simple(type);
+        }
       }
       simpleCache.put(type, simple);
       return simple;
@@ -616,8 +617,6 @@ public abstract class SkylarkType {
       return of(LIST, ((SkylarkList) value).getContentType());
     } else if (value instanceof SkylarkNestedSet) {
       return of(SET, ((SkylarkNestedSet) value).getContentType());
-    } else if (value instanceof ClassObject) {
-      return STRUCT;
     } else {
       return Simple.of(value.getClass());
     }