Make strings not iterable

Fixes #5830

RELNOTES: Flip flag --incompatible_string_is_not_iterable (#5830)
PiperOrigin-RevId: 225981394

site/docs/skylark/backward-compatibility.md

@@ -185,7 +185,7 @@ for i in range(len(my_string)):
 ```
 
 *   Flag: `--incompatible_string_is_not_iterable`
-*   Default: `false`
+*   Default: `true`
 *   Tracking issue: [#5830](https://github.com/bazelbuild/bazel/issues/5830)
 
 

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

@@ -492,7 +492,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
 
   @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 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
       },
       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. */

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

@@ -230,7 +230,7 @@ public static Builder builderWithDefaults() {
           .incompatibleRemoveNativeHttpArchive(true)
           .incompatibleRemoveNativeMavenJar(false)
           .incompatibleStricArgumentOrdering(false)
-          .incompatibleStringIsNotIterable(false)
+          .incompatibleStringIsNotIterable(true)
           .internalSkylarkFlagTestCanary(false)
           .build();
 

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

@@ -314,18 +314,12 @@ public void testListComprehensions() throws Exception {
 
   @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

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

@@ -693,12 +693,15 @@ public void testForOnList() throws Exception {
 
   @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

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({1 : None}), True)
 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)

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(5, 2, 3), 2)
 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])

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

src/test/skylark/testdata/string_misc.sky

@@ -123,14 +123,6 @@ assert_eq('012345678'[:], "012345678")
 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)