Remove flag --incompatible_string_is_not_iterable

#5830

RELNOTES: None.
PiperOrigin-RevId: 234784611

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

@@ -494,20 +494,6 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
               + "https://github.com/bazelbuild/bazel/issues/6611")
   public boolean incompatibleStricArgumentOrdering;
 
-  @Option(
-      name = "incompatible_string_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, iterating over a string will throw an error. String indexing and `len` "
-              + "are still allowed.")
-  public boolean incompatibleStringIsNotIterable;
-
   /** Used in an integration test to confirm that flags are visible to the interpreter. */
   @Option(
     name = "internal_skylark_flag_test_canary",
@@ -572,7 +558,6 @@ public StarlarkSemantics toSkylarkSemantics() {
         .incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
         .incompatibleRequireFeatureConfigurationForPic(requireFeatureConfigurationForPic)
         .incompatibleStricArgumentOrdering(incompatibleStricArgumentOrdering)
-        .incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
         .incompatibleUseToolchainProvidersInJavaCommon(
             incompatibleUseToolchainProvidersInJavaCommon)
         .internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)

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

@@ -354,11 +354,7 @@ private static Collection<?> nestedSetToCollection(
 
   public static Iterable<?> toIterable(Object o, Location loc, @Nullable Environment env)
       throws EvalException {
-    if (o instanceof String) {
-      // 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, loc, env);
-    } else if (o instanceof SkylarkNestedSet) {
+    if (o instanceof SkylarkNestedSet) {
       return nestedSetToCollection((SkylarkNestedSet) o, loc, env);
     } else if (o instanceof Iterable) {
       return (Iterable<?>) o;
@@ -410,22 +406,6 @@ public static void unlock(Object object, Location loc) {
     }
   }
 
-  private static ImmutableList<String> split(String value, Location loc, @Nullable Environment env)
-      throws EvalException {
-    if (env != null && env.getSemantics().incompatibleStringIsNotIterable()) {
-      throw new EvalException(
-          loc,
-          "type 'string' is not iterable. You may still use `len` and string indexing. Use "
-              + "--incompatible_string_is_not_iterable=false to temporarily disable this check.");
-    }
-
-    ImmutableList.Builder<String> builder = ImmutableList.builderWithExpectedSize(value.length());
-    for (char c : value.toCharArray()) {
-      builder.add(String.valueOf(c));
-    }
-    return builder.build();
-  }
-
   // The following functions for indexing and slicing match the behavior of Python.
 
   /**

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

@@ -183,8 +183,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleStricArgumentOrdering();
 
-  public abstract boolean incompatibleStringIsNotIterable();
-
   public abstract boolean internalSkylarkFlagTestCanary();
 
   public abstract boolean incompatibleUseToolchainProvidersInJavaCommon();
@@ -239,7 +237,6 @@ public static Builder builderWithDefaults() {
           .incompatibleRemoveNativeMavenJar(false)
           .incompatibleRequireFeatureConfigurationForPic(true)
           .incompatibleStricArgumentOrdering(true)
-          .incompatibleStringIsNotIterable(true)
           .internalSkylarkFlagTestCanary(false)
           .build();
 
@@ -316,8 +313,6 @@ public abstract static class Builder {
 
     public abstract Builder incompatibleStricArgumentOrdering(boolean value);
 
-    public abstract Builder incompatibleStringIsNotIterable(boolean value);
-
     public abstract Builder incompatibleUseToolchainProvidersInJavaCommon(boolean value);
 
     public abstract Builder internalSkylarkFlagTestCanary(boolean value);

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

@@ -158,7 +158,6 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
         "--incompatible_require_feature_configuration_for_pic=" + rand.nextBoolean(),
         "--incompatible_strict_argument_ordering=" + rand.nextBoolean(),
-        "--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
         "--incompatible_use_toolchain_providers_in_java_common=" + rand.nextBoolean(),
         "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
   }
@@ -206,7 +205,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleRemoveNativeMavenJar(rand.nextBoolean())
         .incompatibleRequireFeatureConfigurationForPic(rand.nextBoolean())
         .incompatibleStricArgumentOrdering(rand.nextBoolean())
-        .incompatibleStringIsNotIterable(rand.nextBoolean())
         .incompatibleUseToolchainProvidersInJavaCommon(rand.nextBoolean())
         .internalSkylarkFlagTestCanary(rand.nextBoolean())
         .build();
@@ -219,4 +217,3 @@ private static StarlarkSemanticsOptions parseOptions(String... args) throws Exce
     return parser.getOptions(StarlarkSemanticsOptions.class);
   }
 }
-

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

@@ -47,16 +47,6 @@ private static SkylarkDict<Object, Object> makeDict(Environment env) {
     return SkylarkDict.of(env, 1, 1, 2, 2);
   }
 
-  @Test
-  public void testEmptyStringToIterable() throws Exception {
-    assertThat(EvalUtils.toIterable("", null, null)).isEmpty();
-  }
-
-  @Test
-  public void testStringToIterable() throws Exception {
-    assertThat(EvalUtils.toIterable("abc", null, null)).hasSize(3);
-  }
-
   /** MockClassA */
   @SkylarkModule(name = "MockClassA", doc = "MockClassA")
   public static class MockClassA {

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

@@ -506,16 +506,9 @@ public void testListComprehensionFailsOnNonSequence() throws Exception {
     newTest().testIfErrorContains("type 'int' is not iterable", "[x + 1 for x in 123]");
   }
 
-  @Test
-  public void testListComprehensionOnString() throws Exception {
-    newTest("--incompatible_string_is_not_iterable=false")
-        .testExactOrder("[x for x in 'abc']", "a", "b", "c");
-  }
-
   @Test
   public void testListComprehensionOnStringIsForbidden() throws Exception {
-    newTest("--incompatible_string_is_not_iterable=true")
-        .testIfErrorContains("type 'string' is not iterable", "[x for x in 'abc']");
+    newTest().testIfErrorContains("type 'string' is not iterable", "[x for x in 'abc']");
   }
 
   @Test

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

@@ -687,19 +687,6 @@ public void testForOnList() throws Exception {
         "s = foo()").testLookup("s", "hello world");
   }
 
-  @Test
-  public void testForOnString() throws Exception {
-    new SkylarkTest("--incompatible_string_is_not_iterable=false")
-        .setUp(
-            "def foo():",
-            "  s = []",
-            "  for i in 'abc':",
-            "    s = s + [i]",
-            "  return s",
-            "s = foo()")
-        .testExactOrder("s", "a", "b", "c");
-  }
-
   @Test
   public void testForAssignmentList() throws Exception {
     new SkylarkTest().setUp("def foo():",
@@ -840,6 +827,14 @@ public void testForNotIterable() throws Exception {
             "func()\n");
   }
 
+  @Test
+  public void testForStringNotIterable() throws Exception {
+    new SkylarkTest()
+        .update("mock", new Mock())
+        .testIfErrorContains(
+            "type 'string' is not iterable", "def func():", "  for i in 'abc': a = i", "func()\n");
+  }
+
   @Test
   public void testForOnDictionary() throws Exception {
     new SkylarkTest().setUp("def foo():",