Make incompatible_disallow_toplevel_if_statement default to true.

RELNOTES: Top-level `if` statements are now forbidden.
PiperOrigin-RevId: 165469101

site/docs/skylark/backward-compatibility.md

@@ -112,7 +112,7 @@ value has a single declaration. This restriction is consistent with the idea
 that global values cannot be redefined.
 
 *   Flag: `--incompatible_disallow_toplevel_if_statement`
-*   Default: `false`
+*   Default: `true`
 
 
 ## Comprehensions variables

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

@@ -135,7 +135,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
 
   @Option(
     name = "incompatible_disallow_toplevel_if_statement",
-    defaultValue = "false",
+    defaultValue = "true",
     category = "incompatible changes",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
     effectTags = {OptionEffectTag.UNKNOWN},

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

@@ -1329,8 +1329,8 @@ public void testDictionaryClear() throws Exception {
     new SkylarkTest()
         .testEval(
             "d = {1: 'foo', 2: 'bar', 3: 'baz'}\n"
-                + "if len(d) != 3: fail('clear 1')\n"
-                + "if d.clear() != None: fail('clear 2')\n"
+                + "len(d) == 3 or fail('clear 1')\n"
+                + "d.clear() == None or fail('clear 2')\n"
                 + "d",
             "{}");
   }
@@ -1341,12 +1341,12 @@ public void testDictionaryPop() throws Exception {
         .testIfErrorContains(
             "KeyError: 1",
             "d = {1: 'foo', 2: 'bar', 3: 'baz'}\n"
-                + "if len(d) != 3: fail('pop 1')\n"
-                + "if d.pop(2) != 'bar': fail('pop 2')\n"
-                + "if d.pop(3, 'quux') != 'baz': fail('pop 3a')\n"
-                + "if d.pop(3, 'quux') != 'quux': fail('pop 3b')\n"
-                + "if d.pop(1) != 'foo': fail('pop 1')\n"
-                + "if d != {}: fail('pop 0')\n"
+                + "len(d) == 3 or fail('pop 1')\n"
+                + "d.pop(2) == 'bar' or fail('pop 2')\n"
+                + "d.pop(3, 'quux') == 'baz' or fail('pop 3a')\n"
+                + "d.pop(3, 'quux') == 'quux' or fail('pop 3b')\n"
+                + "d.pop(1) == 'foo' or fail('pop 1')\n"
+                + "d == {} or fail('pop 0')\n"
                 + "d.pop(1)");
   }
 
@@ -1356,11 +1356,11 @@ public void testDictionaryPopItem() throws Exception {
         .testIfErrorContains(
             "popitem(): dictionary is empty",
             "d = {2: 'bar', 3: 'baz', 1: 'foo'}\n"
-                + "if len(d) != 3: fail('popitem 0')\n"
-                + "if d.popitem() != (2, 'bar'): fail('popitem 2')\n"
-                + "if d.popitem() != (3, 'baz'): fail('popitem 3')\n"
-                + "if d.popitem() != (1, 'foo'): fail('popitem 1')\n"
-                + "if d != {}: fail('popitem 4')\n"
+                + "len(d) == 3 or fail('popitem 0')\n"
+                + "d.popitem() == (2, 'bar') or fail('popitem 2')\n"
+                + "d.popitem() == (3, 'baz') or fail('popitem 3')\n"
+                + "d.popitem() == (1, 'foo') or fail('popitem 1')\n"
+                + "d == {} or fail('popitem 4')\n"
                 + "d.popitem()");
   }
 
@@ -1379,11 +1379,11 @@ public void testDictionarySetDefault() throws Exception {
     new SkylarkTest()
         .testEval(
             "d = {2: 'bar', 1: 'foo'}\n"
-                + "if len(d) != 2: fail('setdefault 0')\n"
-                + "if d.setdefault(1, 'a') != 'foo': fail('setdefault 1')\n"
-                + "if d.setdefault(2) != 'bar': fail('setdefault 2')\n"
-                + "if d.setdefault(3) != None: fail('setdefault 3')\n"
-                + "if d.setdefault(4, 'b') != 'b': fail('setdefault 4')\n"
+                + "len(d) == 2 or fail('setdefault 0')\n"
+                + "d.setdefault(1, 'a') == 'foo' or fail('setdefault 1')\n"
+                + "d.setdefault(2) == 'bar' or fail('setdefault 2')\n"
+                + "d.setdefault(3) == None or fail('setdefault 3')\n"
+                + "d.setdefault(4, 'b') == 'b' or fail('setdefault 4')\n"
                 + "d",
             "{1: 'foo', 2: 'bar', 3: None, 4: 'b'}");
   }

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

@@ -1192,14 +1192,6 @@ public void testAssignmentToListInDictSideEffect() throws Exception {
         "d[0].append(3)").testLookup("l", MutableList.of(null, 1, 2, 3));
   }
 
-  @Test
-  public void testTopLevelDict() throws Exception {
-    new SkylarkTest().setUp("if 1:",
-      "  v = 'a'",
-      "else:",
-      "  v = 'b'").testLookup("v", "a");
-  }
-
   @Test
   public void testUserFunctionKeywordArgs() throws Exception {
     new SkylarkTest().setUp("def foo(a, b, c):",

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

@@ -216,21 +216,13 @@ public void testConcatListToString() throws Exception {
 
   @Test
   public void testConcatListNotEmpty() throws Exception {
-    eval("l = [1, 2] + [3, 4]",
-        "if l:",
-        "  v = 1",
-        "else:",
-        "  v = 0");
+    eval("l = [1, 2] + [3, 4]", "v = 1 if l else 0");
     assertThat(lookup("v")).isEqualTo(1);
   }
 
   @Test
   public void testConcatListEmpty() throws Exception {
-    eval("l = [] + []",
-        "if l:",
-        "  v = 1",
-        "else:",
-        "  v = 0");
+    eval("l = [] + []", "v = 1 if l else 0");
     assertThat(lookup("v")).isEqualTo(0);
   }
 

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

@@ -174,73 +174,6 @@ public void testEmptyLiteralGenericIsSetInLaterConcatWorks() {
     parse("def func():", "  s = {}", "  s['a'] = 'b'\n");
   }
 
-  @Test
-  public void testReadOnlyWorksForSimpleBranching() {
-    parse("if 1:", "  v = 'a'", "else:", "  v = 'b'");
-  }
-
-  @Test
-  public void testReadOnlyWorksForNestedBranching() {
-    parse(
-        "if 1:",
-        "  if 0:",
-        "    v = 'a'",
-        "  else:",
-        "    v = 'b'",
-        "else:",
-        "  if 0:",
-        "    v = 'c'",
-        "  else:",
-        "    v = 'd'\n");
-  }
-
-  @Test
-  public void testReadOnlyWorksForDifferentLevelBranches() {
-    checkError("Variable v is read only", "if 1:", "  if 1:", "    v = 'a'", "  v = 'b'\n");
-  }
-
-  @Test
-  public void testReadOnlyWorksWithinSimpleBranch() {
-    checkError(
-        "Variable v is read only", "if 1:", "  v = 'a'", "else:", "  v = 'b'", "  v = 'c'\n");
-  }
-
-  @Test
-  public void testReadOnlyWorksWithinNestedBranch() {
-    checkError(
-        "Variable v is read only",
-        "if 1:",
-        "  v = 'a'",
-        "else:",
-        "  if 1:",
-        "    v = 'b'",
-        "  else:",
-        "    v = 'c'",
-        "    v = 'd'\n");
-  }
-
-  @Test
-  public void testReadOnlyWorksAfterSimpleBranch() {
-    checkError("Variable v is read only", "if 1:", "  v = 'a'", "else:", "  w = 'a'", "v = 'b'");
-  }
-
-  @Test
-  public void testReadOnlyWorksAfterNestedBranch() {
-    checkError("Variable v is read only", "if 1:", "  if 1:", "    v = 'a'", "v = 'b'");
-  }
-
-  @Test
-  public void testReadOnlyWorksAfterNestedBranch2() {
-    checkError(
-        "Variable v is read only",
-        "if 1:",
-        "  v = 'a'",
-        "else:",
-        "  if 0:",
-        "    w = 1",
-        "v = 'b'\n");
-  }
-
   @Test
   public void testModulesReadOnlyInFuncDefBody() {
     parse("def func():", "  cmd_helper = depset()");