Flip --incompatible_restrict_named_params to true.

RELNOTES: Make a number of parameters of Starlark builtin functions positional-only (as opposed to specifiable by keyword). See #8147 for details. PiperOrigin-RevId: 265995993
bazelbuild · Aug 28, 2019 · 65ecfea · 65ecfea
1 parent c91fa30
commit 65ecfea
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 25 deletions.
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
@@ -603,7 +603,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
 
   @Option(
       name = "incompatible_restrict_named_params",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {

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
@@ -277,7 +277,7 @@ public static Builder builderWithDefaults() {
           .incompatibleRemapMainRepo(false)
           .incompatibleRemoveNativeMavenJar(false)
           .incompatibleRunShellCommandString(false)
-          .incompatibleRestrictNamedParams(false)
+          .incompatibleRestrictNamedParams(true)
           .incompatibleStringJoinRequiresStrings(true)
           .internalSkylarkFlagTestCanary(false)
           .incompatibleDoNotSplitLinkingCmdline(true)

diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -1216,16 +1216,9 @@ public void testGetattrNoAttr() throws Exception {
 
   @Test
   public void testGetattr() throws Exception {
-    eval(
-        "s = struct(a='val')",
-        "x = getattr(s, 'a')",
-        "y = getattr(s, 'b', 'def')",
-        "z = getattr(s, 'b', default = 'def')",
-        "w = getattr(s, 'a', default='ignored')");
+    eval("s = struct(a='val')", "x = getattr(s, 'a')", "y = getattr(s, 'b', 'def')");
     assertThat(lookup("x")).isEqualTo("val");
     assertThat(lookup("y")).isEqualTo("def");
-    assertThat(lookup("z")).isEqualTo("def");
-    assertThat(lookup("w")).isEqualTo("val");
   }
 
   @Test

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
@@ -674,9 +674,9 @@ public void testDictKeysDuplicateKeyArgs() throws Exception {
   public void testArgBothPosKey() throws Exception {
     newTest()
         .testIfErrorContains(
-            "got multiple values for keyword argument 'old', for call to method "
-                + "replace(old, new, maxsplit = None) of 'string'",
-            "'banana'.replace('a', 'o', 3, old='a')");
+            "got multiple values for keyword argument 'base', "
+                + "for call to function int(x, base = unbound)",
+            "int('2', 3, base=3)");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -705,7 +705,7 @@ public void testTupleCoercion() throws Exception {
   // keyword, or may be None, even in places where it does not quite make sense.
   @Test
   public void testLegacyNamed() throws Exception {
-    new SkylarkTest()
+    new SkylarkTest("--incompatible_restrict_named_params=false")
         // Parameters which may be specified by keyword but are not explicitly 'named'.
         .testStatement("all(elements=[True, True])", Boolean.TRUE)
         .testStatement("any(elements=[True, False])", Boolean.TRUE)

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
@@ -1103,31 +1103,27 @@ public void testProxyMethodsObject() throws Exception {
 
   @Test
   public void testLegacyNamed() throws Exception {
-    new SkylarkTest()
+    new SkylarkTest("--incompatible_restrict_named_params=false")
         .update("mock", new Mock())
-        .setUp(
-            "b = mock.legacy_method(True, legacyNamed=True, named=True)")
+        .setUp("b = mock.legacy_method(True, legacyNamed=True, named=True)")
         .testLookup("b", "legacy_method(true, true, true)");
 
-    new SkylarkTest()
+    new SkylarkTest("--incompatible_restrict_named_params=false")
         .update("mock", new Mock())
-        .setUp(
-            "b = mock.legacy_method(True, True, named=True)")
+        .setUp("b = mock.legacy_method(True, True, named=True)")
         .testLookup("b", "legacy_method(true, true, true)");
 
     // Verify legacyNamed also works with proxy method objects.
-    new SkylarkTest()
+    new SkylarkTest("--incompatible_restrict_named_params=false")
         .update("mock", new Mock())
         .setUp(
             "m = mock.proxy_methods_object()",
             "b = m.legacy_method(True, legacyNamed=True, named=True)")
         .testLookup("b", "legacy_method(true, true, true)");
 
-    new SkylarkTest()
+    new SkylarkTest("--incompatible_restrict_named_params=false")
         .update("mock", new Mock())
-        .setUp(
-            "m = mock.proxy_methods_object()",
-            "b = m.legacy_method(True, True, named=True)")
+        .setUp("m = mock.proxy_methods_object()", "b = m.legacy_method(True, True, named=True)")
         .testLookup("b", "legacy_method(true, true, true)");
   }