Refactor SkylarkNestedSet to not implement Iterable

This is not intended to be a user-visible semantic change, aside from error messages.

This is to help avoid unintentional flattening of depsets, and to narrow down the number of call sites where this can occur, to help us print warning/deprecation messages.

EvalUtils#toIterable will now return an ImmutableList in place of SkylarkNestedSet. This should be ok since the caller shouldn't be relying on the result being a Skylark-safe type. Code that takes Iterable because it accepts either a list or set, can instead be changed to take Object and use EvalUtils#toIterableStrict for validation.

Note that NestedSet still implements Iterable, so native code can still easily and accidentally flatten sets.

--
PiperOrigin-RevId: 145044023
MOS_MIGRATED_REVID=145044023

1 files changed, 12 insertions, 2 deletions

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