Remove the flag --incompatible_depset_is_not_iterable

#5816

RELNOTES: None.
PiperOrigin-RevId: 280688754

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -224,21 +224,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "Use the `depset` constructor instead.")
   public boolean incompatibleDepsetUnion;
 
-  @Option(
-      name = "incompatible_depset_is_not_iterable",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
-      effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
-      metadataTags = {
-        OptionMetadataTag.INCOMPATIBLE_CHANGE,
-        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
-      },
-      help =
-          "If set to true, depset type is not iterable. For loops and functions expecting an "
-              + "iterable will reject depset objects. Use the `.to_list` method to explicitly "
-              + "convert to a list.")
-  public boolean incompatibleDepsetIsNotIterable;
-
   @Option(
       name = "incompatible_disable_target_provider_fields",
       defaultValue = "false",
@@ -640,7 +625,6 @@ public StarlarkSemantics toSkylarkSemantics() {
             .experimentalStarlarkUnusedInputsList(experimentalStarlarkUnusedInputsList)
             .experimentalCcSharedLibrary(experimentalCcSharedLibrary)
             .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
-            .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
             .incompatibleDepsetUnion(incompatibleDepsetUnion)
             .incompatibleDisableTargetProviderFields(incompatibleDisableTargetProviderFields)
             .incompatibleDisableThirdPartyLicenseChecking(

src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java

@@ -324,17 +324,16 @@ public static Collection<?> toCollection(Object o, Location loc, @Nullable Starl
       } catch (ComparisonException e) {
         throw new EvalException(loc, e);
       }
-    } else if (o instanceof SkylarkNestedSet) {
-      return nestedSetToCollection((SkylarkNestedSet) o, loc, thread);
     } else {
       throw new EvalException(loc,
           "type '" + getDataTypeName(o) + "' is not a collection");
     }
   }
 
+  // TODO(laurentlb): Get rid of this function.
   private static Collection<?> nestedSetToCollection(
       SkylarkNestedSet set, Location loc, @Nullable StarlarkThread thread) throws EvalException {
-    if (thread != null && thread.getSemantics().incompatibleDepsetIsNotIterable()) {
+    if (thread != null) {
       throw new EvalException(
           loc,
           "type 'depset' is not iterable. Use the `to_list()` method to get a list. Use "
@@ -356,9 +355,7 @@ private static Collection<?> nestedSetToCollection(
 
   public static Iterable<?> toIterable(Object o, Location loc, @Nullable StarlarkThread thread)
       throws EvalException {
-    if (o instanceof SkylarkNestedSet) {
-      return nestedSetToCollection((SkylarkNestedSet) o, loc, thread);
-    } else if (o instanceof Iterable) {
+    if (o instanceof Iterable) {
       return (Iterable<?>) o;
     } else if (o instanceof Map) {
       return toCollection(o, loc, thread);
@@ -961,16 +958,7 @@ private static Object leftShift(Object x, Object y, Location location) throws Ev
   /** Implements 'x in y'. */
   private static boolean in(Object x, Object y, StarlarkThread thread, Location location)
       throws EvalException {
-    if (thread.getSemantics().incompatibleDepsetIsNotIterable() && y instanceof SkylarkNestedSet) {
-      throw new EvalException(
-          location,
-          "argument of type '"
-              + getDataTypeName(y)
-              + "' is not iterable. "
-              + "in operator only works on lists, tuples, dicts and strings. "
-              + "Use --incompatible_depset_is_not_iterable=false to temporarily disable "
-              + "this check.");
-    } else if (y instanceof SkylarkQueryable) {
+    if (y instanceof SkylarkQueryable) {
       return ((SkylarkQueryable) y).containsKey(x, location, thread);
     } else if (y instanceof String) {
       if (x instanceof String) {

src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java

@@ -348,16 +348,6 @@ public Integer len(Object x, Location loc, StarlarkThread thread) throws EvalExc
       return ((Map<?, ?>) x).size();
     } else if (x instanceof Sequence) {
       return ((Sequence<?>) x).size();
-    } else if (x instanceof SkylarkNestedSet) {
-      if (thread.getSemantics().incompatibleDepsetIsNotIterable()) {
-        throw new EvalException(
-            loc,
-            EvalUtils.getDataTypeName(x)
-                + " is not iterable. You may use `len(<depset>.to_list())` instead. Use "
-                + "--incompatible_depset_is_not_iterable=false to temporarily disable this "
-                + "check.");
-      }
-      return ((SkylarkNestedSet) x).toCollection().size();
     } else if (x instanceof Iterable) {
       // Iterables.size() checks if x is a Collection so it's efficient in that sense.
       return Iterables.size((Iterable<?>) x);

src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java

@@ -44,48 +44,49 @@
  * <p>TODO(bazel-team): Decide whether this restriction is still useful.
  */
 @SkylarkModule(
-  name = "depset",
-  category = SkylarkModuleCategory.BUILTIN,
-  doc =
-      "<p>A specialized data structure that supports efficient merge operations and has a defined "
-          + "traversal order. Commonly used for accumulating data from transitive dependencies in "
-          + "rules and aspects. For more information see <a href=\"../depsets.md\">here</a>."
-          + "<p>"
-          + "Depsets are not implemented as hash sets and do not support fast membership tests. If "
-          + "you need a general set datatype, you can simulate one using a dictionary where all "
-          + "keys map to <code>True</code>."
-          + "<p>"
-          + "Depsets are immutable. They should be created using their "
-          + "<a href=\"globals.html#depset\">constructor function</a> and merged or augmented with "
-          + "other depsets via the <code>transitive</code> argument. There are other deprecated "
-          + "methods (<code>|</code> and <code>+</code> operators, <code>union</code> method) that "
-          + "will eventually go away."
-          + "<p>"
-          + "The <code>order</code> parameter determines the kind of traversal that is done to "
-          + "convert the depset to an iterable. There are four possible values:"
-          + "<ul>"
-          + "<li><code>\"default\"</code> (formerly <code>\"stable\"</code>): Order is unspecified "
-          + "(but deterministic).</li>"
-          + "<li><code>\"postorder\"</code> (formerly <code>\"compile\"</code>): A left-to-right "
-          + "post-ordering. Precisely, this recursively traverses all children leftmost-first, "
-          + "then the direct elements leftmost-first.</li>"
-          + "<li><code>\"preorder\"</code> (formerly <code>\"naive_link\"</code>): A left-to-right "
-          + "pre-ordering. Precisely, this traverses the direct elements leftmost-first, then "
-          + "recursively traverses the children leftmost-first.</li>"
-          + "<li><code>\"topological\"</code> (formerly <code>\"link\"</code>): A topological "
-          + "ordering from the root down to the leaves. There is no left-to-right guarantee.</li>"
-          + "</ul>"
-          + "<p>"
-          + "Two depsets may only be merged if either both depsets have the same order, or one of "
-          + "them has <code>\"default\"</code> order. In the latter case the resulting depset's "
-          + "order will be the same as the other's order."
-          + "<p>"
-          + "Depsets may contain duplicate values but these will be suppressed when iterating "
-          + "(using <code>to_list()</code>). Duplicates may interfere with the ordering semantics."
-)
+    name = "depset",
+    category = SkylarkModuleCategory.BUILTIN,
+    doc =
+        "<p>A specialized data structure that supports efficient merge operations and has a"
+            + " defined traversal order. Commonly used for accumulating data from transitive"
+            + " dependencies in rules and aspects. For more information see <a"
+            + " href=\"../depsets.md\">here</a>."
+            + " <p>Depsets are not implemented as hash sets and do"
+            + " not support fast membership tests. If you need a general set datatype, you can"
+            + " simulate one using a dictionary where all keys map to <code>True</code>."
+            + "<p>Depsets are immutable. They should be created using their <a"
+            + " href=\"globals.html#depset\">constructor function</a> and merged or augmented with"
+            + " other depsets via the <code>transitive</code> argument. There are other deprecated"
+            + " methods (<code>|</code> and <code>+</code> operators, <code>union</code> method)"
+            + " that will eventually go away."
+            + "<p>The <code>order</code> parameter determines the"
+            + " kind of traversal that is done to convert the depset to an iterable. There are"
+            + " four possible values:"
+            + "<ul><li><code>\"default\"</code> (formerly"
+            + " <code>\"stable\"</code>): Order is unspecified (but"
+            + " deterministic).</li>"
+            + "<li><code>\"postorder\"</code> (formerly"
+            + " <code>\"compile\"</code>): A left-to-right post-ordering. Precisely, this"
+            + " recursively traverses all children leftmost-first, then the direct elements"
+            + " leftmost-first.</li>"
+            + "<li><code>\"preorder\"</code> (formerly"
+            + " <code>\"naive_link\"</code>): A left-to-right pre-ordering. Precisely, this"
+            + " traverses the direct elements leftmost-first, then recursively traverses the"
+            + " children leftmost-first.</li>"
+            + "<li><code>\"topological\"</code> (formerly"
+            + " <code>\"link\"</code>): A topological ordering from the root down to the leaves."
+            + " There is no left-to-right guarantee.</li>"
+            + "</ul>"
+            + "<p>Two depsets may only be merged if"
+            + " either both depsets have the same order, or one of them has"
+            + " <code>\"default\"</code> order. In the latter case the resulting depset's order"
+            + " will be the same as the other's order."
+            + "<p>Depsets may contain duplicate values but"
+            + " these will be suppressed when iterating (using <code>to_list()</code>). Duplicates"
+            + " may interfere with the ordering semantics.")
 @Immutable
 @AutoCodec
-public final class SkylarkNestedSet implements SkylarkValue, SkylarkQueryable {
+public final class SkylarkNestedSet implements SkylarkValue {
   private final SkylarkType contentType;
   private final NestedSet<?> set;
   @Nullable private final ImmutableList<Object> items;
@@ -397,11 +398,6 @@ public void repr(SkylarkPrinter printer) {
     printer.append(")");
   }
 
-  @Override
-  public final boolean containsKey(Object key, Location loc) throws EvalException {
-    return set.toList().contains(key);
-  }
-
   @SkylarkCallable(
       name = "union",
       doc =

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -152,8 +152,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleBzlDisallowLoadAfterStatement();
 
-  public abstract boolean incompatibleDepsetIsNotIterable();
-
   public abstract boolean incompatibleDepsetUnion();
 
   public abstract boolean incompatibleDisableTargetProviderFields();
@@ -256,7 +254,6 @@ public static Builder builderWithDefaults() {
           .experimentalStarlarkUnusedInputsList(true)
           .experimentalCcSharedLibrary(false)
           .incompatibleBzlDisallowLoadAfterStatement(true)
-          .incompatibleDepsetIsNotIterable(true)
           .incompatibleDepsetUnion(true)
           .incompatibleDisableTargetProviderFields(false)
           .incompatibleDisableThirdPartyLicenseChecking(true)
@@ -318,8 +315,6 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value);
 
-    public abstract Builder incompatibleDepsetIsNotIterable(boolean value);
-
     public abstract Builder incompatibleDepsetUnion(boolean value);
 
     public abstract Builder incompatibleDisableTargetProviderFields(boolean value);

src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -135,7 +135,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--experimental_cc_shared_library=" + rand.nextBoolean(),
         "--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
         "--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(),
-        "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
         "--incompatible_depset_union=" + rand.nextBoolean(),
         "--incompatible_disable_target_provider_fields=" + rand.nextBoolean(),
         "--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
@@ -186,7 +185,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .experimentalCcSharedLibrary(rand.nextBoolean())
         .incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
         .incompatibleDepsetForLibrariesToLinkGetter(rand.nextBoolean())
-        .incompatibleDepsetIsNotIterable(rand.nextBoolean())
         .incompatibleDepsetUnion(rand.nextBoolean())
         .incompatibleDisableTargetProviderFields(rand.nextBoolean())
         .incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -1480,14 +1480,6 @@ public void testClassObjectAccess() throws Exception {
         .testLookup("v", "a");
   }
 
-  @Test
-  public void testInSetDeprecated() throws Exception {
-    new SkylarkTest("--incompatible_depset_is_not_iterable=false")
-        .testExpression("'b' in depset(['a', 'b'])", Boolean.TRUE)
-        .testExpression("'c' in depset(['a', 'b'])", Boolean.FALSE)
-        .testExpression("1 in depset(['a', 'b'])", Boolean.FALSE);
-  }
-
   @Test
   public void testUnionSet() throws Exception {
     new SkylarkTest("--incompatible_depset_union=false")
@@ -1498,28 +1490,16 @@ public void testUnionSet() throws Exception {
 
   @Test
   public void testSetIsNotIterable() throws Exception {
-    new SkylarkTest("--incompatible_depset_is_not_iterable=true")
-        .testIfErrorContains("not iterable", "list(depset(['a', 'b']))")
+    new SkylarkTest()
+        .testIfErrorContains("not a collection", "list(depset(['a', 'b']))")
         .testIfErrorContains("not iterable", "max(depset([1, 2, 3]))")
         .testIfErrorContains("not iterable", "1 in depset([1, 2, 3])")
-        .testIfErrorContains("not iterable", "sorted(depset(['a', 'b']))")
-        .testIfErrorContains("not iterable", "tuple(depset(['a', 'b']))")
+        .testIfErrorContains("not a collection", "sorted(depset(['a', 'b']))")
+        .testIfErrorContains("not a collection", "tuple(depset(['a', 'b']))")
         .testIfErrorContains("not iterable", "[x for x in depset()]")
         .testIfErrorContains("not iterable", "len(depset(['a']))");
   }
 
-  @Test
-  public void testSetIsIterable() throws Exception {
-    new SkylarkTest("--incompatible_depset_is_not_iterable=false")
-        .testExpression("str(list(depset(['a', 'b'])))", "[\"a\", \"b\"]")
-        .testExpression("max(depset([1, 2, 3]))", 3)
-        .testExpression("1 in depset([1, 2, 3])", true)
-        .testExpression("str(sorted(depset(['b', 'a'])))", "[\"a\", \"b\"]")
-        .testExpression("str(tuple(depset(['a', 'b'])))", "(\"a\", \"b\")")
-        .testExpression("str([x for x in depset()])", "[]")
-        .testExpression("len(depset(['a']))", 1);
-  }
-
   @Test
   public void testClassObjectCannotAccessNestedSet() throws Exception {
     new SkylarkTest()

src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java

@@ -591,25 +591,19 @@ public SkylarkNestedSet merge(SkylarkNestedSet[] sets) throws Exception {
   @Test
   public void testDepthExceedsLimitDuringIteration() throws Exception {
     NestedSet.setApplicationDepthLimit(2000);
-    new SkylarkTest("--incompatible_depset_is_not_iterable=false")
+    new SkylarkTest()
         .setUp(
             "def create_depset(depth):",
             "  x = depset([0])",
             "  for i in range(1, depth):",
             "    x = depset([i], transitive = [x])",
-            "  for element in x:",
+            "  for element in x.to_list():",
             "    str(x)",
             "  return None")
         .testEval("create_depset(1000)", "None")
         .testIfErrorContains("depset exceeded maximum depth 2000", "create_depset(3000)");
   }
 
-  @Test
-  public void testListComprehensionsWithNestedSet() throws Exception {
-    new SkylarkTest("--incompatible_depset_is_not_iterable=false")
-        .testEval("[x + x for x in depset([1, 2, 3])]", "[2, 4, 6]");
-  }
-
   private interface MergeStrategy {
     SkylarkNestedSet merge(SkylarkNestedSet[] sets) throws Exception;
   }