Make strings not iterable
Fixes #5830
RELNOTES: Flip flag --incompatible_string_is_not_iterable (https://github.com/bazelbuild/bazel/issues/5830)
PiperOrigin-RevId: 225981394
diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 7e611ec..fd3c1c1 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -185,7 +185,7 @@
```
* Flag: `--incompatible_string_is_not_iterable`
-* Default: `false`
+* Default: `true`
* Tracking issue: [#5830](https://github.com/bazelbuild/bazel/issues/5830)
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 008f892..6087eed 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -492,7 +492,7 @@
@Option(
name = "incompatible_string_is_not_iterable",
- defaultValue = "false",
+ defaultValue = "true",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
@@ -501,8 +501,7 @@
},
help =
"If set to true, iterating over a string will throw an error. String indexing and `len` "
- + "are still allowed."
- )
+ + "are still allowed.")
public boolean incompatibleStringIsNotIterable;
/** Used in an integration test to confirm that flags are visible to the interpreter. */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index ac3fa3a..d58fd2c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -230,7 +230,7 @@
.incompatibleRemoveNativeHttpArchive(true)
.incompatibleRemoveNativeMavenJar(false)
.incompatibleStricArgumentOrdering(false)
- .incompatibleStringIsNotIterable(false)
+ .incompatibleStringIsNotIterable(true)
.internalSkylarkFlagTestCanary(false)
.build();
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 cc6575a..e24d885 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
@@ -314,18 +314,12 @@
@Test
public void testNestedListComprehensions() throws Exception {
- newTest().testExactOrder("li = [[1, 2], [3, 4]]\n" + "[j for i in li for j in i]", 1, 2,
- 3, 4).testExactOrder("input = [['abc'], ['def', 'ghi']]\n"
- + "['%s %s' % (b, c) for a in input for b in a for c in b]",
- "abc a",
- "abc b",
- "abc c",
- "def d",
- "def e",
- "def f",
- "ghi g",
- "ghi h",
- "ghi i");
+ newTest()
+ .testExactOrder("li = [[1, 2], [3, 4]]\n" + "[j for i in li for j in i]", 1, 2, 3, 4)
+ .testExactOrder(
+ "input = [['abc'], ['def', 'ghi']]\n"
+ + "['%s %s' % (b, c) for a in input for b in a for c in b.elems()]",
+ "abc a", "abc b", "abc c", "def d", "def e", "def f", "ghi g", "ghi h", "ghi i");
}
@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 04f20d8..6895268 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
@@ -693,12 +693,15 @@
@Test
public void testForOnString() throws Exception {
- new SkylarkTest().setUp("def foo():",
- " s = []",
- " for i in 'abc':",
- " s = s + [i]",
- " return s",
- "s = foo()").testExactOrder("s", "a", "b", "c");
+ 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
diff --git a/src/test/skylark/testdata/all_any.sky b/src/test/skylark/testdata/all_any.sky
index 1b37967..4b0f59e 100644
--- a/src/test/skylark/testdata/all_any.sky
+++ b/src/test/skylark/testdata/all_any.sky
@@ -1,9 +1,8 @@
# All with empty value
-assert_eq(all(''), True)
assert_eq(all([]), True)
# All with list
-assert_eq(all('test'), True)
+assert_eq(all(['t', 'e']), True)
assert_eq(all([False]), False)
assert_eq(all([True, False]), False)
assert_eq(all([False, False]), False)
@@ -18,11 +17,9 @@
assert_eq(all({None : 1}), False)
# Any with empty value
-assert_eq(any(''), False)
assert_eq(any([]), False)
# Any with list
-assert_eq(any('test'), True)
assert_eq(any([False]), False)
assert_eq(any([0]), False)
assert_eq(any(['']), False)
diff --git a/src/test/skylark/testdata/min_max.sky b/src/test/skylark/testdata/min_max.sky
index 0349d91..02f0474 100644
--- a/src/test/skylark/testdata/min_max.sky
+++ b/src/test/skylark/testdata/min_max.sky
@@ -1,6 +1,6 @@
# min / max
-assert_eq(min("abcdefxyz"), "a")
+assert_eq(min("abcdefxyz".elems()), "a")
assert_eq(min("test", "xyz"), "test")
assert_eq(min([4, 5], [1]), [1])
@@ -15,7 +15,7 @@
assert_eq(min(1, 1, 1, 1, 1, 1), 1)
assert_eq(min([1, 1, 1, 1, 1, 1]), 1)
-assert_eq(max("abcdefxyz"), "z")
+assert_eq(max("abcdefxyz".elems()), "z")
assert_eq(max("test", "xyz"), "xyz")
assert_eq(max("test", "xyz"), "xyz")
assert_eq(max([1, 2], [5]), [5])
diff --git a/src/test/skylark/testdata/reversed.sky b/src/test/skylark/testdata/reversed.sky
index 91ce565..08beba8 100644
--- a/src/test/skylark/testdata/reversed.sky
+++ b/src/test/skylark/testdata/reversed.sky
@@ -1,10 +1,10 @@
# lists
-assert_eq(reversed(''), [])
-assert_eq(reversed('a'), ['a'])
-assert_eq(reversed('abc'), ['c', 'b', 'a'])
-assert_eq(reversed('__test '), [' ', ' ', 't', 's', 'e', 't', '_', '_'])
-assert_eq(reversed('bbb'), ['b', 'b', 'b'])
+assert_eq(reversed([]), [])
+assert_eq(reversed('a'.elems()), ['a'])
+assert_eq(reversed('abc'.elems()), ['c', 'b', 'a'])
+assert_eq(reversed('__test '.elems()), [' ', ' ', 't', 's', 'e', 't', '_', '_'])
+assert_eq(reversed('bbb'.elems()), ['b', 'b', 'b'])
---
reversed(None) ### type 'NoneType' is not iterable
diff --git a/src/test/skylark/testdata/string_misc.sky b/src/test/skylark/testdata/string_misc.sky
index 87708f9..180295d 100644
--- a/src/test/skylark/testdata/string_misc.sky
+++ b/src/test/skylark/testdata/string_misc.sky
@@ -123,14 +123,6 @@
assert_eq('012345678'[-1:2], "")
assert_eq('012345678'[4:2], "")
-# reversed
-# to be removed, strings should not be iterable
-assert_eq(reversed(''), [])
-assert_eq(reversed('a'), ['a'])
-assert_eq(reversed('abc'), ['c', 'b', 'a'])
-assert_eq(reversed('__test '), [' ', ' ', 't', 's', 'e', 't', '_', '_'])
-assert_eq(reversed('bbb'), ['b', 'b', 'b'])
-
# count
assert_eq('abc'.count('a'), 1)
assert_eq('abc'.count('b'), 1)
