Misc Skylark cleanups and small features
--
MOS_MIGRATED_REVID=88224368
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java
index fb6a04f..633f0e6 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java
@@ -84,14 +84,32 @@
// this over selectableConvert.
/**
- * Equivalent to <code>convert(x, null)</code>. Useful for converting values to types that do not
- * involve the type <code>LABEL</code> and hence do not require the label of the current package.
+ * Equivalent to {@code convert(x, what, null)}. Useful for converting values to types that do not
+ * involve the type {@code LABEL} and hence do not require the label of the current package.
*/
public final T convert(Object x, String what) throws ConversionException {
return convert(x, what, null);
}
/**
+ * Like {@code convert(x, what, label)}, but converts skylark {@code None} to java {@code null}.
+ */
+ @Nullable public final T convertOptional (Object x, String what, @Nullable Label currentRule)
+ throws ConversionException {
+ if (EvalUtils.isNullOrNone(x)) {
+ return null;
+ }
+ return convert(x, what, currentRule);
+ }
+
+ /**
+ * Like {@code convert(x, what)}, but converts skylark {@code NONE} to java {@code null}.
+ */
+ @Nullable public final T convertOptional(Object x, String what) throws ConversionException {
+ return convertOptional(x, what, null);
+ }
+
+ /**
* Variation of {@link #convert} that supports selector expressions for configurable attributes
* (i.e. "{ config1: 'value1_of_orig_type', config2: 'value2_of_orig_type; }"). If x is a
* selector expression, returns a {@link Selector} instance that contains key-mapped entries
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
index ae81f81..a5dc760 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
@@ -37,6 +37,8 @@
/**
* A class to handle all Skylark modules, to create and setup Validation and regular Environments.
*/
+// TODO(bazel-team): move that to syntax/ and
+// let each extension register itself in a static { } statement.
public class SkylarkModules {
public static final ImmutableList<Class<?>> MODULES = ImmutableList.of(
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 59aadd1..c03a9ff 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
@@ -138,11 +138,14 @@
// TODO(bazel-team): move the following few type-related functions to SkylarkType
/**
- * Compute the super-class of a class that Skylark considers as the Skylark type of its instances
+ * Return the Skylark-type of {@code c}
*
- * <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.
+ * <p>The result will be a type that Skylark understands and is either equal to {@code c}
+ * or is a supertype of it. For example, all instances of (all subclasses of) SkylarkList
+ * are considered to be SkylarkLists.
+ *
+ * <p>Skylark's type validation isn't equipped to deal with inheritance so we must tell it which
+ * of the superclasses or interfaces of {@code c} is the one that matters for type compatibility.
*
* @param c a class
* @return a super-class of c to be used in validation-time type inference.
@@ -162,7 +165,8 @@
} else if (Set.class.isAssignableFrom(c)) {
return Set.class;
} else {
- // TODO(bazel-team): also unify all ClassObject, that we print the same?
+ // TODO(bazel-team): also unify all implementations of ClassObject,
+ // that we used to all print the same as "struct"?
//
// Check if one of the superclasses or implemented interfaces has the SkylarkModule
// annotation. If yes return that class.
@@ -271,7 +275,7 @@
private static void printValueX(Object o, Appendable buffer)
throws IOException {
if (o == null) {
- throw new NullPointerException(); // None is not a build language value.
+ throw new NullPointerException(); // Java null is not a build language value.
} else if (o instanceof String || o instanceof Integer || o instanceof Double) {
buffer.append(o.toString());
@@ -619,7 +623,7 @@
}
/**
- * Returns the size of the Skylark object or -1 in case the object doesn't have a size.
+ * @return the size of the Skylark object or -1 in case the object doesn't have a size.
*/
public static int size(Object arg) {
if (arg instanceof String) {
@@ -634,4 +638,40 @@
}
return -1;
}
+
+ /** @return true if x is Java null or Skylark None */
+ public static boolean isNullOrNone(Object x) {
+ return x == null || x == Environment.NONE;
+ }
+
+ /**
+ * Build a map of kwarg arguments from a list, removing null-s or None-s.
+ *
+ * @param init a series of key, value pairs (as consecutive arguments), and optionally
+ * a lone map at the end, as in {@code optionMap(k1, v1, k2, v2, k3, v3, map)}
+ * where each key is a String, each value is an arbitrary Objet, and the map
+ * must be a {@code Map<String, Object>}.
+ * @return a {@code Map<String, Object>} that has all the specified entries,
+ * where key, value pairs appearing earlier have precedence,
+ * i.e. {@code k1, v1} may override {@code k3, v3}.
+ *
+ * Ignore any entry the key or value of which is null or None.
+ */
+ @SuppressWarnings("unchecked")
+ public static ImmutableMap<String, Object> optionMap(Object... init) {
+ ImmutableMap.Builder<String, Object> b = new ImmutableMap.Builder<>();
+ int l = init.length;
+ if (l % 2 == 1) { // If there's an odd number of argument, the last one is a Map.
+ l--;
+ b.putAll((Map<String, Object>) init[l]);
+ }
+ for (int i = l - 2; i >= 0; i -= 2) {
+ String key = (String) init[i];
+ Object value = init[i + 1];
+ if (!(isNullOrNone(key) || isNullOrNone(value))) {
+ b.put(key, value);
+ }
+ }
+ return b.build();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 76bb559..c31d434 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -470,8 +470,8 @@
// we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
Map<String, Object> kwargs = new HashMap<>();
- final Object returnValue;
- final Function function;
+ Object returnValue;
+ Function function;
if (obj != null) { // obj.func(...)
Object objValue = obj.eval(env);
// Strings, lists and dictionaries (maps) have functions that we want to use in MethodLibrary.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
index 8b0f5b0..0208cd7 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionSignature.java
@@ -344,7 +344,7 @@
}
public void type(int i) {
if (types != null && types.get(i) != null) {
- sb.append(" : ").append(types.get(i).toString());
+ sb.append(": ").append(types.get(i).toString());
}
}
public void mandatory(int i) {
@@ -355,7 +355,7 @@
public void optional(int i) {
mandatory(i);
sb.append(" = ").append((defaultValues == null)
- ? "null" : String.valueOf(defaultValues.get(j++)));
+ ? "?" : EvalUtils.prettyPrintValue(defaultValues.get(j++)));
}
};
Show show = new Show();
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 42cf163..305ccbe 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
@@ -407,10 +407,21 @@
}
/**
- * Returns a Skylark tuple containing elements.
+ * Build a Skylark tuple from a Java List
+ * @param elements a List of objects
+ * @return a Skylark tuple containing the specified List of objects as elements.
*/
public static SkylarkList tuple(List<?> elements) {
// Tuple elements do not have to have the same type.
return new SimpleSkylarkList(ImmutableList.copyOf(elements), true, SkylarkType.TOP);
}
+
+ /**
+ * Build a Skylark tuple from a variable number of arguments
+ * @param elements a variable number of arguments (or an array of objects)
+ * @return a Skylark tuple containing the specified arguments as elements.
+ */
+ public static SkylarkList tuple(Object... elements) {
+ return new SimpleSkylarkList(ImmutableList.copyOf(elements), true, SkylarkType.TOP);
+ }
}
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 d98062b..1bd5908 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
@@ -287,11 +287,6 @@
simple = TOP;
} else if (type == Empty.class) {
simple = BOTTOM;
- } else if (type == Environment.NoneType.class) {
- // For the purpose of validation-time type inference, treat NONE as being of type TOP,
- // 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 {
// Consider all classes that have the same EvalUtils.getSkylarkType() as equivalent,
// as a substitute to handling inheritance.
@@ -549,7 +544,7 @@
}
public boolean contains(Object value) {
- // This returns true a bit too much, but it looks
+ // This returns true a bit too much, not looking at the result type.
return value instanceof Function;
}
@@ -591,6 +586,17 @@
// Utility functions regarding types
+ /**
+ * For the purpose of type inference during validation,
+ * we upgrade the type for None as being Top, the type of everything,
+ * so None is compatible with anything as far as the validate method is concern.
+ *
+ * @param type a SkylarkType suitable for runtime type checking.
+ * @return the corresponding SkylarkType suitable for a type validation.
+ */
+ private static SkylarkType typeForInference(SkylarkType type) {
+ return type == NONE ? TOP : type;
+ }
/**
* Returns the stronger type of this and o if they are compatible. Stronger means that
@@ -599,12 +605,12 @@
*
* <p>If they are not compatible an EvalException is thrown.
*/
- SkylarkType infer(SkylarkType o, String name, Location thisLoc, Location originalLoc)
+ SkylarkType infer(SkylarkType other, String name, Location thisLoc, Location originalLoc)
throws EvalException {
- SkylarkType both = intersection(this, o);
+ SkylarkType both = intersection(typeForInference(this), typeForInference(other));
if (both == BOTTOM) {
throw new EvalException(thisLoc, String.format("bad %s: %s is incompatible with %s at %s",
- name, o, this, originalLoc));
+ name, other, this, originalLoc));
} else {
return both;
}
@@ -678,6 +684,24 @@
}
}
+
+ /**
+ * General purpose type-casting facility.
+ *
+ * @param value - the actual value of the parameter
+ * @param type - the expected Class for the value
+ * @param loc - the location info used in the EvalException
+ * @param format - a format String
+ * @param args - arguments to format, in case there's an exception
+ */
+ public static <T> T castOrNull(Object value, Class<T> type,
+ Location loc, String format, Object... args) throws EvalException {
+ if (value == Environment.NONE) {
+ return null;
+ }
+ return SkylarkType.<T>cast(value, type, loc, format, args);
+ }
+
/**
* General purpose type-casting facility.
*
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
index 6afb96a..cc69867 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ValidationEnvironment.java
@@ -68,6 +68,17 @@
clonable = false;
}
+ // ValidationEnvironment for a new Environment()
+ private static ImmutableMap<SkylarkType, ImmutableMap<String, SkylarkType>> globalTypes =
+ ImmutableMap.<SkylarkType, ImmutableMap<String, SkylarkType>>of(SkylarkType.GLOBAL,
+ new ImmutableMap.Builder<String, SkylarkType> ()
+ .put("False", SkylarkType.BOOL).put("True", SkylarkType.BOOL)
+ .put("None", SkylarkType.TOP).build());
+
+ public ValidationEnvironment() {
+ this(globalTypes);
+ }
+
@Override
public ValidationEnvironment clone() {
Preconditions.checkState(clonable);
