Remove flag `--incompatible_string_is_not_iterable`
https://github.com/bazelbuild/bazel/issues/5830
RELNOTES: None.
PiperOrigin-RevId: 234784611
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index f8fa525..231480a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -494,20 +494,6 @@
+ "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 @@
.incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
.incompatibleRequireFeatureConfigurationForPic(requireFeatureConfigurationForPic)
.incompatibleStricArgumentOrdering(incompatibleStricArgumentOrdering)
- .incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
.incompatibleUseToolchainProvidersInJavaCommon(
incompatibleUseToolchainProvidersInJavaCommon)
.internalSkylarkFlagTestCanary(internalSkylarkFlagTestCanary)
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 5ca655e..38e7710 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
@@ -354,11 +354,7 @@
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 @@
}
}
- 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.
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 4c45d63..f075118 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -183,8 +183,6 @@
public abstract boolean incompatibleStricArgumentOrdering();
- public abstract boolean incompatibleStringIsNotIterable();
-
public abstract boolean internalSkylarkFlagTestCanary();
public abstract boolean incompatibleUseToolchainProvidersInJavaCommon();
@@ -239,7 +237,6 @@
.incompatibleRemoveNativeMavenJar(false)
.incompatibleRequireFeatureConfigurationForPic(true)
.incompatibleStricArgumentOrdering(true)
- .incompatibleStringIsNotIterable(true)
.internalSkylarkFlagTestCanary(false)
.build();
@@ -316,8 +313,6 @@
public abstract Builder incompatibleStricArgumentOrdering(boolean value);
- public abstract Builder incompatibleStringIsNotIterable(boolean value);
-
public abstract Builder incompatibleUseToolchainProvidersInJavaCommon(boolean value);
public abstract Builder internalSkylarkFlagTestCanary(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 06f3762..18fb41d 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -158,7 +158,6 @@
"--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 @@
.incompatibleRemoveNativeMavenJar(rand.nextBoolean())
.incompatibleRequireFeatureConfigurationForPic(rand.nextBoolean())
.incompatibleStricArgumentOrdering(rand.nextBoolean())
- .incompatibleStringIsNotIterable(rand.nextBoolean())
.incompatibleUseToolchainProvidersInJavaCommon(rand.nextBoolean())
.internalSkylarkFlagTestCanary(rand.nextBoolean())
.build();
@@ -219,4 +217,3 @@
return parser.getOptions(StarlarkSemanticsOptions.class);
}
}
-
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 eec32d1..2701457 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
@@ -47,16 +47,6 @@
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 {
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 05cbd17..6ee8f87 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -507,15 +507,8 @@
}
@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
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 3d89729..2ef84a6 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -688,19 +688,6 @@
}
@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():",
" d = ['a', 'b', 'c']",
@@ -841,6 +828,14 @@
}
@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():",
" d = {1: 'a', 2: 'b', 3: 'c'}",
