Tweaks off of upcoming changes to SkylarkType.

--
MOS_MIGRATED_REVID=87154772

6 files changed, 44 insertions, 12 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 22d6af008c..ae79a17103 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -266,7 +266,7 @@ public final class RuleConfiguredTargetBuilder {
   /**
    * Populates the configuration specific mnemonicToExtraActionMap
    * based on all action_listers selected by the user (via the blaze option
-   * --experimental_action_listener=<target>).
+   * {@code --experimental_action_listener=<target>}).
    */
   private Multimap<String, ExtraActionSpec> computeMnemonicsToExtraActionMap() {
     // We copy the multimap here every time. This could be expensive.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractFunction.java
index f444c231dc..b7e3b84a1e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractFunction.java
@@ -40,6 +40,10 @@ public abstract class AbstractFunction implements Function {
     return null;
   }
 
+  @Override public String toString() {
+    return name;
+  }
+
   /**
    * Abstract implementation of Function that accepts no parameters.
    */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index d54e8e8856..d7c142713e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -268,10 +268,12 @@ public final class BinaryOperatorExpression extends Expression {
       }
     } // endswitch
 
+    // NB: this message format is identical to that used by CPython 2.7.6 or 3.4.0,
+    // though python raises a TypeError.
+    // For more details, we'll hopefully have usable stack traces at some point.
     throw new EvalException(getLocation(),
-        "unsupported operand types for '" + operator + "': '"
-        + EvalUtils.getDataTypeName(lval) + "' and '"
-        + EvalUtils.getDataTypeName(rval) + "'");
+        String.format("unsupported operand type(s) for %s: '%s' and '%s'",
+            operator, EvalUtils.getDataTypeName(lval), EvalUtils.getDataTypeName(rval)));
   }
 
   @Override
@@ -404,9 +406,12 @@ public final class BinaryOperatorExpression extends Expression {
       }
     } // endswitch
 
+    // NB: this message format is identical to that used by CPython 2.7.6 or 3.4.0,
+    // though python raises a TypeError at runtime, whereas we issue an EvalException a bit earlier.
     if (ltype != SkylarkType.UNKNOWN && rtype != SkylarkType.UNKNOWN) {
       throw new EvalException(getLocation(),
-          "unsupported operand types for '" + operator + "': '" + lname + "' and '" + rname + "'");
+          String.format("unsupported operand type(s) for %s: '%s' and '%s'",
+              operator, lname, rname));
     }
     return SkylarkType.UNKNOWN;
   }
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 57aec07e56..02c10a26e1 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
@@ -139,6 +139,7 @@ public abstract class EvalUtils {
     return null;
   }
 
+  // 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.
@@ -168,16 +169,32 @@ public abstract class EvalUtils {
     return c;
   }
 
-  // TODO(bazel-team): shouldn't we agree on Datatype vs DataType in the two methods below???
   /**
    * Returns a pretty name for the datatype of object 'o' in the Build language.
    */
   public static String getDataTypeName(Object o) {
-    Preconditions.checkNotNull(o);
-    if (o instanceof SkylarkList) {
-      return ((SkylarkList) o).isTuple() ? "tuple" : "list";
+    return getDataTypeName(o, false);
+  }
+
+  /**
+   * Returns a pretty name for the datatype of object {@code object} in Skylark
+   * or the BUILD language, with full details if the {@code full} boolean is true.
+   */
+  public static String getDataTypeName(Object object, boolean full) {
+    Preconditions.checkNotNull(object);
+    if (object instanceof SkylarkList) {
+      SkylarkList list = (SkylarkList) object;
+      if (list.isTuple()) {
+        return "tuple";
+      } else {
+        return "list" + (full ? " of " + list.getGenericType() + "s" : "");
+      }
+    } else if (object instanceof SkylarkNestedSet) {
+      SkylarkNestedSet set = (SkylarkNestedSet) object;
+      return "set" + (full ? " of " + set.getGenericType() + "s" : "");
+    } else {
+      return getDataTypeNameFromClass(object.getClass());
     }
-    return getDataTypeNameFromClass(o.getClass());
   }
 
   /**
@@ -280,6 +297,11 @@ public abstract class EvalUtils {
       buffer.append(": ");
       prettyPrintValue(entry.getValue(), buffer);
 
+    } else if (o instanceof SkylarkNestedSet) {
+      Iterable<?> seq = (Iterable<?>) o;
+      // TODO(bazel-team): preserve the order= flag when not the default "stable".
+      printList(seq, "set([", ", ", "])", buffer);
+
     } else if (o instanceof Function) {
       Function func = (Function) o;
       buffer.append("<function " + func.getName() + ">");
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
index 89c26dad1b..5e00f057ee 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
@@ -40,7 +40,8 @@ import java.util.List;
         + "List elements have to be of the same type, <code>[1, 2, \"c\"]</code> results in an "
         + "error. Lists - just like everything - are immutable, therefore <code>x[1] = \"a\""
         + "</code> is not supported.")
-public abstract class SkylarkList implements Iterable<Object> {
+  // TODO(bazel-team): should we instead implements List<Object> like ImmutableList does?
+  public abstract class SkylarkList implements Iterable<Object> {
 
   private final boolean tuple;
   private final Class<?> genericType;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
index 5b9929892f..44729cf23e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java
@@ -194,6 +194,6 @@ public final class SkylarkNestedSet implements Iterable<Object> {
 
   @Override
   public String toString() {
-    return "set(" + set.toCollection() + ")";
+    return EvalUtils.prettyPrintValue(this);
   }
 }