Remove the flag --incompatible_depset_is_not_iterable

    bazelbuild/bazel#5816

    RELNOTES: None.
    PiperOrigin-RevId: 280688754

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -329,6 +329,21 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
   // TODO(elenairina): Move option to graveyard after the flag is removed from the global blazerc.
   public boolean incompatibleDisallowLegacyJavaInfo;
 
+  @Option(
+      name = "incompatible_string_join_requires_strings",
+      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, the argument of `string.join` must be an iterable whose elements are "
+              + "strings. If set to false, elements are first converted to string. "
+              + "See https://github.com/bazelbuild/bazel/issues/7802")
+  public boolean incompatibleStringJoinRequiresStrings;
+
   @Option(
       name = "incompatible_disallow_struct_provider_syntax",
       defaultValue = "false",
@@ -631,6 +646,7 @@ public StarlarkSemantics toSkylarkSemantics() {
             .incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
             .incompatibleRestrictNamedParams(incompatibleRestrictNamedParams)
             .incompatibleRunShellCommandString(incompatibleRunShellCommandString)
+            .incompatibleStringJoinRequiresStrings(incompatibleStringJoinRequiresStrings)
             .incompatibleVisibilityPrivateAttributesAtDefinition(
                 incompatibleVisibilityPrivateAttributesAtDefinition)
             .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java

@@ -301,7 +301,8 @@ public static Object checkNotNull(Expression expr, Object obj) throws EvalExcept
     return obj;
   }
 
-  public static Collection<?> toCollection(Object o, Location loc) throws EvalException {
+  public static Collection<?> toCollection(Object o, Location loc, @Nullable StarlarkThread thread)
+      throws EvalException {
     if (o instanceof Collection) {
       return (Collection<?>) o;
     } else if (o instanceof Sequence) {
@@ -352,11 +353,12 @@ private static Collection<?> nestedSetToCollection(
     }
   }
 
-  public static Iterable<?> toIterable(Object o, Location loc) throws EvalException {
+  public static Iterable<?> toIterable(Object o, Location loc, @Nullable StarlarkThread thread)
+      throws EvalException {
     if (o instanceof Iterable) {
       return (Iterable<?>) o;
     } else if (o instanceof Map) {
-      return toCollection(o, loc);
+      return toCollection(o, loc, thread);
     } else {
       throw new EvalException(loc,
           "type '" + getDataTypeName(o) + "' is not iterable");

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java

@@ -62,10 +62,11 @@ class MethodLibrary {
               + "min([5, 6, 3]) == 3</pre>",
       extraPositionals =
           @Param(name = "args", type = Sequence.class, doc = "The elements to be checked."),
-      useLocation = true)
-  public Object min(Sequence<?> args, Location loc) throws EvalException {
+      useLocation = true,
+      useStarlarkThread = true)
+  public Object min(Sequence<?> args, Location loc, StarlarkThread thread) throws EvalException {
     try {
-      return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR.reverse(), loc);
+      return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR.reverse(), loc, thread);
     } catch (ComparisonException e) {
       throw new EvalException(loc, e);
     }
@@ -81,22 +82,25 @@ public Object min(Sequence<?> args, Location loc) throws EvalException {
               + "max([5, 6, 3]) == 6</pre>",
       extraPositionals =
           @Param(name = "args", type = Sequence.class, doc = "The elements to be checked."),
-      useLocation = true)
-  public Object max(Sequence<?> args, Location loc) throws EvalException {
+      useLocation = true,
+      useStarlarkThread = true)
+  public Object max(Sequence<?> args, Location loc, StarlarkThread thread) throws EvalException {
     try {
-      return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR, loc);
+      return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR, loc, thread);
     } catch (ComparisonException e) {
       throw new EvalException(loc, e);
     }
   }
 
   /** Returns the maximum element from this list, as determined by maxOrdering. */
-  private static Object findExtreme(Sequence<?> args, Ordering<Object> maxOrdering, Location loc)
+  private static Object findExtreme(
+      Sequence<?> args, Ordering<Object> maxOrdering, Location loc, StarlarkThread thread)
       throws EvalException {
     // 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 {
-      Iterable<?> items = (args.size() == 1) ? EvalUtils.toIterable(args.get(0), loc) : args;
+      Iterable<?> items =
+          (args.size() == 1) ? EvalUtils.toIterable(args.get(0), loc, thread) : args;
       return maxOrdering.max(items);
     } catch (NoSuchElementException ex) {
       throw new EvalException(loc, "expected at least one item", ex);
@@ -119,9 +123,10 @@ private static Object findExtreme(Sequence<?> args, Ordering<Object> maxOrdering
             // TODO(cparsons): This parameter should be positional-only.
             legacyNamed = true)
       },
-      useLocation = true)
-  public Boolean all(Object collection, Location loc) throws EvalException {
-    return !hasElementWithBooleanValue(collection, false, loc);
+      useLocation = true,
+      useStarlarkThread = true)
+  public Boolean all(Object collection, Location loc, StarlarkThread thread) throws EvalException {
+    return !hasElementWithBooleanValue(collection, false, loc, thread);
   }
 
   @SkylarkCallable(
@@ -140,14 +145,15 @@ public Boolean all(Object collection, Location loc) throws EvalException {
             // TODO(cparsons): This parameter should be positional-only.
             legacyNamed = true)
       },
-      useLocation = true)
-  public Boolean any(Object collection, Location loc) throws EvalException {
-    return hasElementWithBooleanValue(collection, true, loc);
+      useLocation = true,
+      useStarlarkThread = true)
+  public Boolean any(Object collection, Location loc, StarlarkThread thread) throws EvalException {
+    return hasElementWithBooleanValue(collection, true, loc, thread);
   }
 
-  private static boolean hasElementWithBooleanValue(Object collection, boolean value, Location loc)
-      throws EvalException {
-    Iterable<?> iterable = EvalUtils.toIterable(collection, loc);
+  private static boolean hasElementWithBooleanValue(
+      Object collection, boolean value, Location loc, StarlarkThread thread) throws EvalException {
+    Iterable<?> iterable = EvalUtils.toIterable(collection, loc, thread);
     for (Object obj : iterable) {
       if (Starlark.truth(obj) == value) {
         return true;
@@ -195,7 +201,7 @@ public StarlarkList<?> sorted(
       final StarlarkThread thread)
       throws EvalException, InterruptedException {
 
-    Object[] array = EvalUtils.toCollection(iterable, loc).toArray();
+    Object[] array = EvalUtils.toCollection(iterable, loc, thread).toArray();
     if (key == Starlark.NONE) {
       try {
         Arrays.sort(array, EvalUtils.SKYLARK_COMPARATOR);
@@ -275,7 +281,7 @@ public StarlarkList<?> reversed(Object sequence, Location loc, StarlarkThread th
       throw new EvalException(loc, "Argument to reversed() must be a sequence, not a dictionary.");
     }
     ArrayDeque<Object> tmpList = new ArrayDeque<>();
-    for (Object element : EvalUtils.toIterable(sequence, loc)) {
+    for (Object element : EvalUtils.toIterable(sequence, loc, thread)) {
       tmpList.addFirst(element);
     }
     return StarlarkList.copyOf(thread.mutability(), tmpList);
@@ -296,9 +302,10 @@ public StarlarkList<?> reversed(Object sequence, Location loc, StarlarkThread th
             // TODO(cparsons): This parameter should be positional-only.
             legacyNamed = true)
       },
-      useLocation = true)
-  public Tuple<?> tuple(Object x, Location loc) throws EvalException {
-    return Tuple.copyOf(EvalUtils.toCollection(x, loc));
+      useLocation = true,
+      useStarlarkThread = true)
+  public Tuple<?> tuple(Object x, Location loc, StarlarkThread thread) throws EvalException {
+    return Tuple.copyOf(EvalUtils.toCollection(x, loc, thread));
   }
 
   @SkylarkCallable(
@@ -319,7 +326,7 @@ public Tuple<?> tuple(Object x, Location loc) throws EvalException {
       useLocation = true,
       useStarlarkThread = true)
   public StarlarkList<?> list(Object x, Location loc, StarlarkThread thread) throws EvalException {
-    return StarlarkList.copyOf(thread.mutability(), EvalUtils.toCollection(x, loc));
+    return StarlarkList.copyOf(thread.mutability(), EvalUtils.toCollection(x, loc, thread));
   }
 
   @SkylarkCallable(
@@ -622,7 +629,7 @@ private String getIntegerPrefix(String value) {
       useLocation = true)
   public StarlarkList<?> enumerate(Object input, Integer start, Location loc, StarlarkThread thread)
       throws EvalException {
-    Collection<?> src = EvalUtils.toCollection(input, loc);
+    Collection<?> src = EvalUtils.toCollection(input, loc, thread);
     Object[] array = new Object[src.size()];
     int i = 0;
     for (Object x : src) {
@@ -1176,7 +1183,7 @@ public StarlarkList<?> zip(Sequence<?> args, Location loc, StarlarkThread thread
       throws EvalException {
     Iterator<?>[] iterators = new Iterator<?>[args.size()];
     for (int i = 0; i < args.size(); i++) {
-      iterators[i] = EvalUtils.toIterable(args.get(i), loc).iterator();
+      iterators[i] = EvalUtils.toIterable(args.get(i), loc, thread).iterator();
     }
     ArrayList<Tuple<?>> result = new ArrayList<>();
     boolean allHasNext;

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/SkylarkNestedSet.java

@@ -181,7 +181,7 @@ static SkylarkNestedSet of(SkylarkNestedSet left, Object right, Location loc)
   // TODO(adonovan): enforce that we never construct a SkylarkNestedSet with a StarlarkType
   // that represents a non-Skylark type (e.g. NestedSet<PathFragment>).
   // One way to do that is to disallow constructing StarlarkTypes for classes
-  // that would fail Starlark.valid; however remains the problem that
+  // that would fail isSkylarkAcceptable; however remains the problem that
   // Object.class means "any Starlark value" but in fact allows any Java value.
   public static <T> SkylarkNestedSet of(SkylarkType contentType, NestedSet<T> set) {
     return new SkylarkNestedSet(contentType, set, null, null);

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -190,6 +190,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleRunShellCommandString();
 
+  public abstract boolean incompatibleStringJoinRequiresStrings();
+
   public abstract boolean incompatibleVisibilityPrivateAttributesAtDefinition();
 
   public abstract boolean internalSkylarkFlagTestCanary();
@@ -271,6 +273,7 @@ public static Builder builderWithDefaults() {
           .incompatibleRemoveNativeMavenJar(true)
           .incompatibleRunShellCommandString(false)
           .incompatibleRestrictNamedParams(true)
+          .incompatibleStringJoinRequiresStrings(true)
           .incompatibleVisibilityPrivateAttributesAtDefinition(false)
           .internalSkylarkFlagTestCanary(false)
           .incompatibleDoNotSplitLinkingCmdline(true)
@@ -350,6 +353,8 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleRunShellCommandString(boolean value);
 
+    public abstract Builder incompatibleStringJoinRequiresStrings(boolean value);
+
     public abstract Builder incompatibleVisibilityPrivateAttributesAtDefinition(boolean value);
 
     public abstract Builder internalSkylarkFlagTestCanary(boolean value);

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java

@@ -155,6 +155,7 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
         "--incompatible_restrict_named_params=" + rand.nextBoolean(),
         "--incompatible_run_shell_command_string=" + rand.nextBoolean(),
+        "--incompatible_string_join_requires_strings=" + rand.nextBoolean(),
         "--incompatible_visibility_private_attributes_at_definition=" + rand.nextBoolean(),
         "--incompatible_restrict_string_escapes=" + rand.nextBoolean(),
         "--incompatible_disallow_dict_lookup_unhashable_keys=" + rand.nextBoolean(),
@@ -204,6 +205,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleRemoveNativeMavenJar(rand.nextBoolean())
         .incompatibleRestrictNamedParams(rand.nextBoolean())
         .incompatibleRunShellCommandString(rand.nextBoolean())
+        .incompatibleStringJoinRequiresStrings(rand.nextBoolean())
         .incompatibleVisibilityPrivateAttributesAtDefinition(rand.nextBoolean())
         .incompatibleRestrictStringEscapes(rand.nextBoolean())
         .incompatibleDisallowDictLookupUnhashableKeys(rand.nextBoolean())

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java

@@ -146,7 +146,7 @@ public Boolean isEmpty(String str) {
     public void value() {}
     @SkylarkCallable(name = "return_bad", documented = false)
     public Bad returnBad() {
-      return new Bad(); // not a legal Starlark value
+      return new Bad();
     }
     @SkylarkCallable(name = "struct_field", documented = false, structField = true)
     public String structField() {
@@ -520,8 +520,7 @@ static final class MockClassObject implements ClassObject, SkylarkValue {
     public Object getValue(String name) {
       switch (name) {
         case "field": return "a";
-        case "nset":
-          return NestedSetBuilder.stableOrder().build(); // not a legal Starlark value
+        case "nset": return NestedSetBuilder.stableOrder().build();
         default: return null;
       }
     }
@@ -1458,15 +1457,11 @@ public void testStructAccessType_nonClassObject() throws Exception {
   }
 
   @Test
-  public void testJavaFunctionReturnsIllegalValue() throws Exception {
-    update("mock", new Mock());
-    IllegalArgumentException e =
-        assertThrows(IllegalArgumentException.class, () -> eval("mock.return_bad()"));
-    assertThat(e)
-        .hasMessageThat()
-        .contains(
-            "cannot expose internal type to Starlark: class"
-                + " com.google.devtools.build.lib.syntax.SkylarkEvaluationTest$Bad");
+  public void testJavaFunctionReturnsMutableObject() throws Exception {
+    new SkylarkTest()
+        .update("mock", new Mock())
+        .testIfExactError(
+            "method 'return_bad' returns an object of invalid type Bad", "mock.return_bad()");
   }
 
   @Test
@@ -1506,15 +1501,10 @@ public void testSetIsNotIterable() throws Exception {
   }
 
   @Test
-  public void testFieldReturnsNestedSet() throws Exception {
-    update("mock", new MockClassObject());
-    IllegalArgumentException e =
-        assertThrows(IllegalArgumentException.class, () -> eval("mock.nset"));
-    assertThat(e)
-        .hasMessageThat()
-        .contains(
-            "cannot expose internal type to Starlark: class"
-                + " com.google.devtools.build.lib.collect.nestedset.NestedSet");
+  public void testClassObjectCannotAccessNestedSet() throws Exception {
+    new SkylarkTest()
+        .update("mock", new MockClassObject())
+        .testIfErrorContains("internal error: type 'NestedSet' is not allowed", "v = mock.nset");
   }
 
   @Test

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java

@@ -41,19 +41,26 @@ public void testConstructor() throws Exception {
   }
 
   @Test
-  public void testTuples() throws Exception {
+  public void testTuplePairs() throws Exception {
     exec(
+        // Depsets with tuple-pairs
         "s_one = depset([('1', '2'), ('3', '4')])",
         "s_two = depset(direct = [('1', '2'), ('3', '4'), ('5', '6')])",
         "s_three = depset(transitive = [s_one, s_two])",
         "s_four = depset(direct = [('1', '3')], transitive = [s_one, s_two])",
+        // Depsets with tuple-pairs and non-pair tuples are considered just tuple depsets.
         "s_five = depset(direct = [('1', '3', '5')], transitive = [s_one, s_two])",
         "s_six = depset(transitive = [s_one, s_five])",
         "s_seven = depset(direct = [('1', '3')], transitive = [s_one, s_five])",
-        "s_eight = depset(direct = [(1, 3)], transitive = [s_one, s_two])"); // note, tuple of int
-    assertThat(get("s_one").getContentType()).isEqualTo(SkylarkType.TUPLE);
-    assertThat(get("s_two").getContentType()).isEqualTo(SkylarkType.TUPLE);
-    assertThat(get("s_three").getContentType()).isEqualTo(SkylarkType.TUPLE);
+        "s_eight = depset(direct = [(1, 3)], transitive = [s_one, s_two])");
+    assertThat(get("s_one").getContentType()).isEqualTo(SkylarkType.STRING_PAIR);
+    assertThat(get("s_two").getContentType()).isEqualTo(SkylarkType.STRING_PAIR);
+    assertThat(get("s_three").getContentType()).isEqualTo(SkylarkType.STRING_PAIR);
+    assertThat(get("s_four").getContentType()).isEqualTo(SkylarkType.STRING_PAIR);
+
+    assertThat(get("s_five").getContentType()).isEqualTo(SkylarkType.TUPLE);
+    assertThat(get("s_six").getContentType()).isEqualTo(SkylarkType.TUPLE);
+    assertThat(get("s_seven").getContentType()).isEqualTo(SkylarkType.TUPLE);
     assertThat(get("s_eight").getContentType()).isEqualTo(SkylarkType.TUPLE);
 
     assertThat(get("s_four").getSet(Tuple.class))