incompatible_disallow_keyword_only_args now defaults to true

RELNOTES[INC]:
  Keyword-only syntax in a function definition is now forbidden
  e.g. `def foo(a, *, b)` or `def foo(a, *b, c)`

PiperOrigin-RevId: 167741296

3 files changed, 10 insertions, 40 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
index 24e7c71f32..98dfb297c0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
@@ -74,7 +74,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
 
   @Option(
     name = "incompatible_disallow_keyword_only_args",
-    defaultValue = "false",
+    defaultValue = "true",
     category = "incompatible changes",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
     effectTags = {OptionEffectTag.UNKNOWN},
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
index 4450b67eb3..575b03f662 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/BaseFunctionTest.java
@@ -134,19 +134,22 @@ public class BaseFunctionTest extends EvaluationTestCase {
   @SuppressWarnings("unchecked")
   @Test
   public void testKwParam() throws Exception {
-    eval("def foo(a, b, c=3, d=4, *args, e, f, g=7, h=8, **kwargs):\n"
-        + "  return (a, b, c, d, e, f, g, h, args, kwargs)\n"
-        + "v1 = foo(1, 2, e=5, f=6)\n"
-        + "v2 = foo(1, *['x', 'y', 'z', 't'], h=9, e=5, f=6, i=0)\n"
+    eval("def foo(a, b, c=3, d=4, g=7, h=8, *args, **kwargs):\n"
+        + "  return (a, b, c, d, g, h, args, kwargs)\n"
+        + "v1 = foo(1, 2)\n"
+        + "v2 = foo(1, *['x', 'y', 'z', 't'], h=9, i=0)\n"
+        + "v3 = foo(1, *[2, 3, 4, 5, 6, 7, 8], i=0)\n"
         + "def bar(**kwargs):\n"
         + "  return kwargs\n"
         + "b1 = bar(name='foo', type='jpg', version=42)\n"
         + "b2 = bar()\n");
 
     assertThat(Printer.repr(lookup("v1")))
-        .isEqualTo("(1, 2, 3, 4, 5, 6, 7, 8, (), {})");
+        .isEqualTo("(1, 2, 3, 4, 7, 8, (), {})");
     assertThat(Printer.repr(lookup("v2")))
-        .isEqualTo("(1, \"x\", \"y\", \"z\", 5, 6, 7, 9, (\"t\",), {\"i\": 0})");
+        .isEqualTo("(1, \"x\", \"y\", \"z\", \"t\", 9, (), {\"i\": 0})");
+    assertThat(Printer.repr(lookup("v3")))
+        .isEqualTo("(1, 2, 3, 4, 5, 6, (7, 8), {\"i\": 0})");
 
     // NB: the conversion to a TreeMap below ensures the keys are sorted.
     assertThat(Printer.repr(
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
index cf70b0e72d..76aebfb19f 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/FunctionTest.java
@@ -299,21 +299,6 @@ public class FunctionTest extends EvaluationTestCase {
   }
 
   @Test
-  public void testKwargs() throws Exception {
-    eval("def foo(a, b = 'b', *, c, d = 'd'):",
-      "  return a + b + c + d",
-      "args = {'a': 'x', 'c': 'z'}",
-      "v1 = foo(**args)",
-      "v2 = foo('x', c = 'c', d = 'e', **{'b': 'y'})",
-      "v3 = foo(c = 'z', a = 'x', **{'b': 'y', 'd': 'f'})");
-    assertThat(lookup("v1")).isEqualTo("xbzd");
-    assertThat(lookup("v2")).isEqualTo("xyce");
-    assertThat(lookup("v3")).isEqualTo("xyzf");
-    UserDefinedFunction foo = (UserDefinedFunction) lookup("foo");
-    assertThat(foo.toString()).isEqualTo("foo(a, b = \"b\", *, c, d = \"d\")");
-  }
-
-  @Test
   public void testKeywordOnlyIsForbidden() throws Exception {
     env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_keyword_only_args=true");
     checkEvalErrorContains("forbidden", "def foo(a, b, *, c): return a + b + c");
@@ -387,24 +372,6 @@ public class FunctionTest extends EvaluationTestCase {
   }
 
   @Test
-  public void testStarParam() throws Exception {
-    eval("def f(name, value = '1', *rest, mandatory, optional = '2'):",
-        "  r = name + value + mandatory + optional + '|'",
-        "  for x in rest: r += x",
-        "  return r",
-        "v1 = f('a', 'b', mandatory = 'z')",
-        "v2 = f('a', 'b', 'c', 'd', mandatory = 'z')",
-        "v3 = f('a', *['b', 'c', 'd'], mandatory = 'y', optional = 'z')",
-        "v4 = f(*['a'], **{'value': 'b', 'mandatory': 'c'})",
-        "v5 = f('a', 'b', 'c', *['d', 'e'], mandatory = 'f', **{'optional': 'g'})\n");
-    assertThat(lookup("v1")).isEqualTo("abz2|");
-    assertThat(lookup("v2")).isEqualTo("abz2|cd");
-    assertThat(lookup("v3")).isEqualTo("abyz|cd");
-    assertThat(lookup("v4")).isEqualTo("abc2|");
-    assertThat(lookup("v5")).isEqualTo("abfg|cde");
-  }
-
-  @Test
   public void testIncompatibleStarParam() throws Exception {
     env = newEnvironmentWithSkylarkOptions("--incompatible_disallow_keyword_only_args=true");
     eval("def f(name, value = '1', optional = '2', *rest):",