starlark: disallow keyword argument after *args (google#317)

f(*args, k=v) is a now an error. Use f(k=v, *args).

Arguments are evaluated left-to-right, as in Python2 (but not Python3).
See bazelbuild/starlark#13 for spec change.

resolve/resolve.go

@@ -741,7 +741,9 @@ func (r *resolver) expr(e syntax.Expr) {
 				// k=v
 				n++
 				if seenKwargs {
-					r.errorf(pos, "argument may not follow **kwargs")
+					r.errorf(pos, "keyword argument may not follow **kwargs")
+				} else if seenVarargs {
+					r.errorf(pos, "keyword argument may not follow *args")
 				}
 				x := binop.X.(*syntax.Ident)
 				if seenName[x.Name] {
@@ -757,9 +759,9 @@ func (r *resolver) expr(e syntax.Expr) {
 				// positional argument
 				p++
 				if seenVarargs {
-					r.errorf(pos, "argument may not follow *args")
+					r.errorf(pos, "positional argument may not follow *args")
 				} else if seenKwargs {
-					r.errorf(pos, "argument may not follow **kwargs")
+					r.errorf(pos, "positional argument may not follow **kwargs")
 				} else if len(seenName) > 0 {
 					r.errorf(pos, "positional argument may not follow named")
 				}

resolve/testdata/resolve.star

@@ -289,12 +289,12 @@ f(**{}, *[]) ### `\*args may not follow \*\*kwargs`
 f(**{}, **{}) ### `multiple \*\*kwargs not allowed`
 
 ---
-# Only keyword arguments may follow *args in a call.
+# Only **kwargs may follow *args in a call.
 def f(*args, **kwargs):
   pass
 
-f(*[], 1) ### `argument may not follow \*args`
-f(*[], a=1) # ok
+f(*[], 1) ### `positional argument may not follow \*args`
+f(*[], a=1) ### `keyword argument may not follow \*args`
 f(*[], *[]) ### `multiple \*args not allowed`
 f(*[], **{}) # ok
 

starlark/eval_test.go

@@ -297,6 +297,7 @@ def j(a, b=42, *args, c, d=123, e, **kwargs):
 		t.Fatal(err)
 	}
 
+	// All errors are dynamic; see resolver for static errors.
 	for _, test := range []struct{ src, want string }{
 		// a()
 		{`a()`, `None`},
@@ -352,8 +353,6 @@ def j(a, b=42, *args, c, d=123, e, **kwargs):
 		{`f(0, b=1)`, `(0, 1, (), {})`},
 		{`f(0, a=1)`, `function f got multiple values for parameter "a"`},
 		{`f(0, b=1, c=2)`, `(0, 1, (), {"c": 2})`},
-		{`f(0, 1, x=2, *[3, 4], y=5, **dict(z=6))`, // github.com/google/skylark/issues/135
-			`(0, 1, (3, 4), {"x": 2, "y": 5, "z": 6})`},
 
 		// g(a, b=42, *args, c=123, **kwargs)
 		{`g()`, `function g missing 1 argument (a)`},
@@ -365,8 +364,6 @@ def j(a, b=42, *args, c, d=123, e, **kwargs):
 		{`g(0, b=1)`, `(0, 1, (), 123, {})`},
 		{`g(0, a=1)`, `function g got multiple values for parameter "a"`},
 		{`g(0, b=1, c=2, d=3)`, `(0, 1, (), 2, {"d": 3})`},
-		{`g(0, 1, x=2, *[3, 4], y=5, **dict(z=6))`,
-			`(0, 1, (3, 4), 123, {"x": 2, "y": 5, "z": 6})`},
 
 		// h(a, b=42, *, c=123, **kwargs)
 		{`h()`, `function h missing 1 argument (a)`},
@@ -379,7 +376,6 @@ def j(a, b=42, *args, c, d=123, e, **kwargs):
 		{`h(0, b=1, c=2)`, `(0, 1, 2, {})`},
 		{`h(0, b=1, d=2)`, `(0, 1, 123, {"d": 2})`},
 		{`h(0, b=1, c=2, d=3)`, `(0, 1, 2, {"d": 3})`},
-		{`h(0, b=1, c=2, d=3)`, `(0, 1, 2, {"d": 3})`},
 
 		// i(a, b=42, *, c, d=123, e, **kwargs)
 		{`i()`, `function i missing 3 arguments (a, c, e)`},

starlark/testdata/function.star

@@ -223,20 +223,20 @@ load("assert.star", "assert")
 r = []
 
 def id(x):
-       r.append(x)
-       return x
+  r.append(x)
+  return x
 
 def f(*args, **kwargs):
   return (args, kwargs)
 
-y = f(id(1), id(2), x=id(3), *[id(4)], y=id(5), **dict(z=id(6)))
-assert.eq(y, ((1, 2, 4), dict(x=3, y=5, z=6)))
+y = f(id(1), id(2), x=id(3), *[id(4)], **dict(z=id(5)))
+assert.eq(y, ((1, 2, 4), dict(x=3, z=5)))
 
-# This matches Python2, but not Starlark-in-Java:
+# This matches Python2 and Starlark-in-Java, but not Python3 [1 2 4 3 6].
 # *args and *kwargs are evaluated last.
-# See github.com/bazelbuild/starlark#13 for pending spec change.
-assert.eq(r, [1, 2, 3, 5, 4, 6])
-
+# (Python[23] also allows keyword arguments after *args.)
+# See github.com/bazelbuild/starlark#13 for spec change.
+assert.eq(r, [1, 2, 3, 4, 5])
 
 ---
 # option:nesteddef option:recursion