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);