diff options
14 files changed, 184 insertions, 80 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java index 39265a6875..9f5b9d97c5 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java +++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java @@ -151,7 +151,7 @@ public final class NestedSet<E> implements Iterable<E> { } /** - * Returns true if the set is empty. + * Returns true if the set is empty. Runs in O(1) time (i.e. does not flatten the set). */ public boolean isEmpty() { return children == EMPTY_CHILDREN; diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java index 6b31fd8761..b077b013cf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkFileType.java @@ -19,6 +19,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.EvalUtils; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; @@ -53,8 +55,16 @@ public class SkylarkFileType { + "<a href=\"File.html\"><code>File</code></a>s that match the FileType. The parameter " + "must be a <a href=\"set.html\"><code>set</code></a> or a " + "<a href=\"list.html\"><code>list</code></a>.") - public List<Artifact> filter(Iterable<Artifact> files) { - return ImmutableList.copyOf(FileType.filter(files, fileType)); + // toIterablesStrict() will ensure the parameter is a SkylarkNestedSet or a java Iterable + // (including SkylarkList). If it fails, the error location information will be inserted by the + // Skylark interface framework. If there's a dynamic type error on a non-Artifact element, the + // error will also be handled by the Skylark interface framework. + @SuppressWarnings("unchecked") + public List<Artifact> filter(Object filesUnchecked) throws EvalException { + return ImmutableList.copyOf( + FileType.filter( + (Iterable<Artifact>) EvalUtils.toIterableStrict(filesUnchecked, null), + fileType)); } @VisibleForTesting diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java index f200cf5406..c58334d450 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java @@ -58,6 +58,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; @@ -650,30 +651,43 @@ public class SkylarkRuleImplementationFunctions { }; + /** + * Ensures the given {@link Map} has keys that have {@link Label} type and values that have either + * {@link Iterable} or {@link SkylarkNestedSet} type, and raises {@link EvalException} otherwise. + * Returns a corresponding map where any sets are replaced by iterables. + */ // TODO(bazel-team): find a better way to typecheck this argument. @SuppressWarnings("unchecked") private static Map<Label, Iterable<Artifact>> checkLabelDict( Map<?, ?> labelDict, Location loc) throws EvalException { + Map<Label, Iterable<Artifact>> convertedMap = new HashMap<Label, Iterable<Artifact>>(); for (Map.Entry<?, ?> entry : labelDict.entrySet()) { Object key = entry.getKey(); if (!(key instanceof Label)) { throw new EvalException( loc, Printer.format("invalid key %r in 'label_dict'", key)); } + ImmutableList.Builder<Artifact> files = ImmutableList.builder(); Object val = entry.getValue(); - if (!(val instanceof Iterable)) { + Iterable<?> valIter; + try { + valIter = EvalUtils.toIterableStrict(val, loc); + } catch (EvalException ex) { + // EvalException is thrown only if the type is wrong. throw new EvalException( - loc, Printer.format("invalid value %r in 'label_dict'", val)); + loc, Printer.format("invalid value %r in 'label_dict': " + ex, val)); } - for (Object file : (Iterable) val) { + for (Object file : valIter) { if (!(file instanceof Artifact)) { throw new EvalException( loc, Printer.format("invalid value %r in 'label_dict'", val)); } + files.add((Artifact) file); } + convertedMap.put((Label) key, files.build()); } - return (Map<Label, Iterable<Artifact>>) labelDict; + return convertedMap; } /** suffix of script to be used in case the command is too long to fit on a single line */ @@ -682,12 +696,11 @@ public class SkylarkRuleImplementationFunctions { @SkylarkSignature( name = "resolve_command", doc = - "Experimental." - + "Returns a tuple (inputs, command, input_manifests) of the list of resolved inputs, " - + "the argv list for the resolved command, and " - + "the dict mapping locations to runfiles required to run the command, " - + "all of them suitable for passing as the same-named arguments of the ctx.action " - + "method.", + "<i>(Experimental)</i> " + + "Returns a tuple <code>(inputs, command, input_manifests)</code> of the list of " + + "resolved inputs, the argv list for the resolved command, and the dict mapping " + + "locations to runfiles required to run the command, all of them suitable for passing " + + "as the same-named arguments of the <code>ctx.action</code> method.", objectType = SkylarkRuleContext.class, returnType = Tuple.class, parameters = { diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java index 7e3b443119..ee8a37c35f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProviderSkylarkConverters.java @@ -94,7 +94,7 @@ public class ObjcProviderSkylarkConverters { @Override public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, javaKey.getType(), javaKey.getSkylarkKeyName()); - return (SkylarkNestedSet) skylarkValue; + return ((SkylarkNestedSet) skylarkValue).toCollection(); } } @@ -118,7 +118,7 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<PathFragment> result = NestedSetBuilder.stableOrder(); - for (String path : (Iterable<String>) skylarkValue) { + for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) { result.add(new PathFragment(path)); } return result.build(); @@ -145,7 +145,7 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, String.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<SdkFramework> result = NestedSetBuilder.stableOrder(); - for (String path : (Iterable<String>) skylarkValue) { + for (String path : ((SkylarkNestedSet) skylarkValue).toCollection(String.class)) { result.add(new SdkFramework(path)); } return result.build(); @@ -178,7 +178,8 @@ public class ObjcProviderSkylarkConverters { public Iterable<?> valueForJava(Key<?> javaKey, Object skylarkValue) { validateTypes(skylarkValue, SkylarkClassObject.class, javaKey.getSkylarkKeyName()); NestedSetBuilder<BundleableFile> result = NestedSetBuilder.stableOrder(); - for (SkylarkClassObject struct : (Iterable<SkylarkClassObject>) skylarkValue) { + for (SkylarkClassObject struct : + ((SkylarkNestedSet) skylarkValue).toCollection(SkylarkClassObject.class)) { Artifact artifact; String path; try { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java index 799f850c8a..284483616f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java @@ -145,7 +145,7 @@ public class BazelLibrary { + "the input depset as well as all additional elements.", parameters = { @Param(name = "input", type = SkylarkNestedSet.class, doc = "The input depset."), - @Param(name = "new_elements", type = Iterable.class, doc = "The elements to be added.") + @Param(name = "new_elements", type = Object.class, doc = "The elements to be added.") }, useLocation = true ) @@ -153,8 +153,11 @@ public class BazelLibrary { new BuiltinFunction("union") { @SuppressWarnings("unused") public SkylarkNestedSet invoke( - SkylarkNestedSet input, Iterable<Object> newElements, Location loc) + SkylarkNestedSet input, Object newElements, Location loc) throws EvalException { + // newElements' type is Object because of the polymorphism on unioning two + // SkylarkNestedSets versus a set and another kind of iterable. + // Can't use EvalUtils#toIterable since that would discard this information. return new SkylarkNestedSet(input, newElements, loc); } }; 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 83d2b9f3a4..b3d7bc0ad6 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 @@ -353,6 +353,9 @@ public final class EvalUtils { // This is not as efficient as special casing String in for and dict and list comprehension // statements. However this is a more unified way. return split((String) o); + } else if (o instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) o).toCollection(); } else if (o instanceof Iterable) { return (Iterable<?>) o; } else if (o instanceof Map) { @@ -363,6 +366,35 @@ public final class EvalUtils { } } + /** + * Given an {@link Iterable}, returns it as-is. Given a {@link SkylarkNestedSet}, returns its + * contents as an iterable. Throws {@link EvalException} for any other value. + * + * <p>This is a kludge for the change that made {@code SkylarkNestedSet} not implement {@code + * Iterable}. It is different from {@link #toIterable} in its behavior for strings and other types + * that are not strictly Java-iterable. + * + * @throws EvalException if {@code o} is not an iterable or set + * @deprecated avoid writing APIs that implicitly treat depsets as iterables. It encourages + * unnecessary flattening of depsets. + * + * <p>TODO(bazel-team): Remove this if/when implicit iteration over {@code SkylarkNestedSet} is no + * longer supported. + */ + @Deprecated + public static Iterable<?> toIterableStrict(Object o, Location loc) throws EvalException { + if (o instanceof Iterable) { + return (Iterable<?>) o; + } else if (o instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) o).toCollection(); + } else { + throw new EvalException(loc, + "expected Iterable or depset, but got '" + getDataTypeName(o) + "' (strings and maps " + + "are not allowed here)"); + } + } + public static void lock(Object object, Location loc) { if (object instanceof SkylarkMutable) { ((SkylarkMutable) object).lock(loc); @@ -392,7 +424,10 @@ public final class EvalUtils { } else if (arg instanceof Map) { return ((Map<?, ?>) arg).size(); } else if (arg instanceof SkylarkList) { - return ((SkylarkList) arg).size(); + return ((SkylarkList<?>) arg).size(); + } else if (arg instanceof SkylarkNestedSet) { + // TODO(bazel-team): Add a deprecation warning: don't implicitly flatten depsets. + return ((SkylarkNestedSet) arg).toCollection().size(); } else if (arg instanceof Iterable) { // Iterables.size() checks if arg is a Collection so it's efficient in that sense. return Iterables.size((Iterable<?>) arg); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java index 47ca735a2a..3a0a176d28 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java @@ -1024,23 +1024,16 @@ public class MethodLibrary { */ private static Object findExtreme(SkylarkList<?> args, Ordering<Object> maxOrdering, Location loc) throws EvalException { - // Args can either be a list of elements or a list whose first element is a non-empty iterable - // of elements. + // Args can either be a list of items to compare, or a singleton list whose element is an + // iterable of items to compare. In either case, there must be at least one item to compare. try { - return maxOrdering.max(getIterable(args, loc)); + Iterable<?> items = (args.size() == 1) ? EvalUtils.toIterable(args.get(0), loc) : args; + return maxOrdering.max(items); } catch (NoSuchElementException ex) { - throw new EvalException(loc, "expected at least one argument"); + throw new EvalException(loc, "expected at least one item"); } } - /** - * This method returns the first element of the list, if that particular element is an - * Iterable<?>. Otherwise, it will return the entire list. - */ - private static Iterable<?> getIterable(SkylarkList<?> list, Location loc) throws EvalException { - return (list.size() == 1) ? EvalUtils.toIterable(list.get(0), loc) : list; - } - @SkylarkSignature( name = "all", returnType = Boolean.class, 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 530efc0994..3edfb99b31 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 @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.util.Preconditions; import java.util.ArrayList; import java.util.Collection; -import java.util.Iterator; import java.util.List; import javax.annotation.Nullable; @@ -101,7 +100,7 @@ import javax.annotation.Nullable; + "nested depsets." ) @Immutable -public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, SkylarkQueryable { +public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable { private final SkylarkType contentType; private final NestedSet<?> set; @@ -151,7 +150,7 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S throw new EvalException( loc, String.format( - "cannot add value of type '%s' to a depset", EvalUtils.getDataTypeName(item))); + "cannot union value of type '%s' to a depset", EvalUtils.getDataTypeName(item))); } this.contentType = Preconditions.checkNotNull(contentType, "type cannot be null"); @@ -252,10 +251,23 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S } } + private void checkHasContentType(Class<?> type) { + // Empty sets should be SkylarkType.TOP anyway. + if (!set.isEmpty()) { + Preconditions.checkArgument( + contentType.canBeCastTo(type), + "Expected a depset of '%s' but got a depset of '%s'", + EvalUtils.getDataTypeNameFromClass(type), contentType); + } + } + /** * Returns the embedded {@link NestedSet}, while asserting that its elements all have the given * type. * + * <p>If you do not specifically need the {@code NestedSet} and you are going to flatten it + * anyway, prefer {@link #toCollection} to make your intent clear. + * * @param type a {@link Class} representing the expected type of the contents * @return the {@code NestedSet}, with the appropriate generic type * @throws IllegalArgumentException if the type does not accurately describe all elements @@ -263,17 +275,32 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S // The precondition ensures generic type safety. @SuppressWarnings("unchecked") public <T> NestedSet<T> getSet(Class<T> type) { - // Empty sets don't need have to have a type since they don't have items - if (set.isEmpty()) { - return (NestedSet<T>) set; - } - Preconditions.checkArgument( - contentType.canBeCastTo(type), - "Expected a depset of '%s' but got a depset of '%s'", - EvalUtils.getDataTypeNameFromClass(type), contentType); + checkHasContentType(type); return (NestedSet<T>) set; } + /** + * Returns the contents of the set as a {@link Collection}. + */ + public Collection<Object> toCollection() { + // Do not remove <Object>: workaround for Java 7 type inference. + return ImmutableList.<Object>copyOf(set.toCollection()); + } + + /** + * Returns the contents of the set as a {@link Collection}, asserting that the set type is + * compatible with {@code T}. + * + * @param type a {@link Class} representing the expected type of the contents + * @throws IllegalArgumentException if the type does not accurately describe all elements + */ + // The precondition ensures generic type safety. + @SuppressWarnings("unchecked") + public <T> Collection<T> toCollection(Class<T> type) { + checkHasContentType(type); + return (Collection<T>) toCollection(); + } + @SkylarkCallable( name = "to_list", doc = "Returns a frozen list of the elements, without duplicates, in the depset's traversal " @@ -282,18 +309,6 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S return new MutableList<Object>(set, null); } - // For some reason this cast is unsafe in Java - @SuppressWarnings("unchecked") - @Override - public Iterator<Object> iterator() { - return (Iterator<Object>) set.iterator(); - } - - public Collection<Object> toCollection() { - // Do not remove <Object>: workaround for Java 7 type inference. - return ImmutableList.<Object>copyOf(set.toCollection()); - } - public boolean isEmpty() { return set.isEmpty(); } @@ -319,7 +334,7 @@ public final class SkylarkNestedSet implements Iterable<Object>, SkylarkValue, S @Override public void write(Appendable buffer, char quotationMark) { Printer.append(buffer, "set("); - Printer.printList(buffer, this, "[", ", ", "]", null, quotationMark); + Printer.printList(buffer, set, "[", ", ", "]", null, quotationMark); Order order = getOrder(); if (order != Order.STABLE_ORDER) { Printer.append(buffer, ", order = \"" + order.getSkylarkName() + "\""); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Type.java b/src/main/java/com/google/devtools/build/lib/syntax/Type.java index e2cc62fcbb..dbbfc79c42 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Type.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Type.java @@ -530,11 +530,13 @@ public abstract class Type<T> { @Override public List<ElemT> convert(Object x, Object what, Object context) throws ConversionException { - if (!(x instanceof Iterable<?>)) { + Iterable<?> iterable; + try { + iterable = EvalUtils.toIterableStrict(x, null); + } catch (EvalException ex) { throw new ConversionException(this, x, what); } int index = 0; - Iterable<?> iterable = (Iterable<?>) x; List<ElemT> result = new ArrayList<>(Iterables.size(iterable)); ListConversionContext conversionContext = new ListConversionContext(what); for (Object elem : iterable) { diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java index 47b4a98f41..c17cd69f12 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java @@ -213,7 +213,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(names).isInstanceOf(SkylarkNestedSet.class); assertThat( transform( - (SkylarkNestedSet) names, + ((SkylarkNestedSet) names).toCollection(), new Function<Object, String>() { @Nullable @Override @@ -225,7 +225,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { .containsExactly("//test:xxx", "//test:yyy"); Object ruleKinds = skylarkProviders.getValue("rule_kinds"); assertThat(ruleKinds).isInstanceOf(SkylarkNestedSet.class); - assertThat((SkylarkNestedSet) ruleKinds).containsExactly("java_library"); + assertThat(((SkylarkNestedSet) ruleKinds).toCollection()).containsExactly("java_library"); } @Test @@ -274,7 +274,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(names).isInstanceOf(SkylarkNestedSet.class); assertThat( transform( - (SkylarkNestedSet) names, + ((SkylarkNestedSet) names).toCollection(), new Function<Object, String>() { @Nullable @Override @@ -409,7 +409,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(names).isInstanceOf(SkylarkNestedSet.class); assertThat( transform( - (SkylarkNestedSet) names, + ((SkylarkNestedSet) names).toCollection(), new Function<Object, String>() { @Nullable @Override @@ -1442,7 +1442,7 @@ public class SkylarkAspectsTest extends AnalysisTestCase { assertThat(names).isInstanceOf(SkylarkNestedSet.class); assertThat( transform( - (SkylarkNestedSet) names, + ((SkylarkNestedSet) names).toCollection(), new Function<Object, String>() { @Nullable @Override diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java index 65b175c015..59daa250ff 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvalUtilsTest.java @@ -19,6 +19,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; @@ -53,6 +56,18 @@ public class EvalUtilsTest extends EvaluationTestCase { assertThat(EvalUtils.toIterable("abc", null)).hasSize(3); } + @Test + public void testSize() throws Exception { + assertThat(EvalUtils.size("abc")).isEqualTo(3); + assertThat(EvalUtils.size(ImmutableMap.of(1, 2, 3, 4))).isEqualTo(2); + assertThat(EvalUtils.size(SkylarkList.Tuple.of(1, 2, 3))).isEqualTo(3); + SkylarkNestedSet set = SkylarkNestedSet.of( + Object.class, + NestedSetBuilder.stableOrder().add(1).add(2).add(3).build()); + assertThat(EvalUtils.size(set)).isEqualTo(3); + assertThat(EvalUtils.size(ImmutableList.of(1, 2, 3))).isEqualTo(3); + } + /** MockClassA */ @SkylarkModule(name = "MockClassA", doc = "MockClassA") public static class MockClassA { diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java index e0c6c99c24..ed91471964 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java @@ -42,7 +42,7 @@ public class MethodLibraryTest extends EvaluationTestCase { public void testMinWithInvalidArgs() throws Exception { new SkylarkTest() .testIfExactError("type 'int' is not iterable", "min(1)") - .testIfExactError("expected at least one argument", "min([])"); + .testIfExactError("expected at least one item", "min([])"); } @Test @@ -98,7 +98,7 @@ public class MethodLibraryTest extends EvaluationTestCase { public void testMaxWithInvalidArgs() throws Exception { new BothModesTest() .testIfExactError("type 'int' is not iterable", "max(1)") - .testIfExactError("expected at least one argument", "max([])"); + .testIfExactError("expected at least one item", "max([])"); } @Test @@ -440,9 +440,8 @@ public class MethodLibraryTest extends EvaluationTestCase { public void testBuiltinFunctionErrorMessage() throws Exception { new BothModesTest() .testIfErrorContains( - "method depset.union(new_elements: Iterable) is not applicable for arguments (string): " - + "'new_elements' is 'string', but should be 'Iterable'", - "depset([]).union('a')") + "substring \"z\" not found in \"abc\"", + "'abc'.index('z')") .testIfErrorContains( "method string.startswith(sub: string, start: int, end: int or NoneType) is not " + "applicable for arguments (int, int, NoneType): 'sub' is 'int', " diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java index 6471192af5..ad4519132c 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java @@ -62,6 +62,19 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { } @Test + public void testToCollection() throws Exception { + eval("s = depset(['a', 'b'])"); + assertThat(get("s").toCollection(String.class)).containsExactly("a", "b").inOrder(); + assertThat(get("s").toCollection(Object.class)).containsExactly("a", "b").inOrder(); + assertThat(get("s").toCollection()).containsExactly("a", "b").inOrder(); + try { + get("s").toCollection(Integer.class); + Assert.fail("toCollection() with wrong type should have raised IllegalArgumentException"); + } catch (IllegalArgumentException expected) { + } + } + + @Test public void testOrder() throws Exception { eval("s = depset(['a', 'b'], order='postorder')"); assertThat(get("s").getSet(String.class).getOrder()).isEqualTo(Order.COMPILE_ORDER); @@ -94,13 +107,9 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { @Test public void testBadGenericType() throws Exception { - new BothModesTest() - .testIfExactError( - "cannot add an item of type 'int' to a depset of 'string'", - "depset(['a', 5])") - .testIfExactError( - "cannot add value of type 'string' to a depset", - "depset() + 'a'"); + new BothModesTest().testIfExactError( + "cannot add an item of type 'int' to a depset of 'string'", + "depset(['a', 5])"); } @Test @@ -130,7 +139,8 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { private void assertContainsInOrder(String statement, Object... expectedElements) throws Exception { - new BothModesTest().testCollection(statement, expectedElements); + assertThat(((SkylarkNestedSet) eval(statement)).toCollection()) + .containsExactly(expectedElements); } @Test @@ -158,12 +168,10 @@ public class SkylarkNestedSetTest extends EvaluationTestCase { public void testUnionWithNonsequence() throws Exception { new BothModesTest() .testIfExactError( - "method depset.union(new_elements: Iterable) is not applicable for arguments (int): " - + "'new_elements' is 'int', but should be 'Iterable'", + "cannot union value of type 'int' to a depset", "depset([]).union(5)") .testIfExactError( - "method depset.union(new_elements: Iterable) is not applicable for arguments (string): " - + "'new_elements' is 'string', but should be 'Iterable'", + "cannot union value of type 'string' to a depset", "depset(['a']).union('b')"); } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java index a5a850e4ec..a0e36fc7bd 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java @@ -25,6 +25,8 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.License; import com.google.devtools.build.lib.packages.TriState; @@ -326,6 +328,14 @@ public class TypeTest { } @Test + public void testListDepsetConversion() throws Exception { + Object input = SkylarkNestedSet.of( + String.class, + NestedSetBuilder.create(Order.STABLE_ORDER, "a", "b", "c")); + Type.STRING_LIST.convert(input, null); + } + + @Test public void testLabelList() throws Exception { Object input = Arrays.asList("//foo:bar", ":wiz"); List<Label> converted = |