syntax: permit trailing commas in all function defs and calls (google…

…#263)

Following Python3 and Starlark-in-Java, this change causes
the parser not to reject a trailing comma, even after *args or **kwargs,
in function definitions and function calls.

Fixes google#256

doc/spec.md

@@ -991,6 +991,8 @@ Unlike Python, Starlark does not allow more than one `*args` argument in a
 call, and if a `*args` argument is present it must appear after all
 positional and named arguments.
 
+The final argument to a function call may be followed by a trailing comma.
+
 A function call completes normally after the execution of either a
 `return` statement, or of the last statement in the function body.
 The result of the function call is the value of the return statement's
@@ -1524,9 +1526,6 @@ Operand = identifier
 DotSuffix   = '.' identifier .
 CallSuffix  = '(' [Arguments [',']] ')' .
 SliceSuffix = '[' [Expression] [':' Test [':' Test]] ']' .
-
-# A CallSuffix does not allow a trailing comma
-# if the last argument is '*' Test or '**' Test.
 ```
 
 TODO: resolve position of +x, -x, and 'not x' in grammar: Operand or UnaryExpr?
@@ -2152,7 +2151,7 @@ x = ([1, 2], [3, 4], [5, 6])
 ### Function and method calls
 
 ```grammar {.good}
-CallSuffix = '(' [Arguments] ')' .
+CallSuffix = '(' [Arguments [',']] ')' .
 
 Arguments = Argument {',' Argument} .
 Argument  = Test | identifier '=' Test | '*' Test | '**' Test .
@@ -2557,6 +2556,8 @@ This is called the _keyword arguments_ parameter, and accumulates in a
 dictionary any surplus `name=value` arguments that do not match a
 prior parameter. It is conventionally named `**kwargs`.
 
+The final parameter may be followed by a trailing comma.
+
 Here are some example parameter lists:
 
 ```python
@@ -2567,6 +2568,12 @@ def f(a, b, c=1, *args): pass
 def f(a, b, c=1, *args, **kwargs): pass
 def f(**kwargs): pass
 def f(a, b, c=1, *, d=1): pass
+
+def f(
+  a,
+  *args,
+  **kwargs,
+)
 ```
 
 Execution of a `def` statement creates a new function object.  The

starlark/testdata/function.star

@@ -287,3 +287,21 @@ def e():
     f()
 
 assert.fails(e, "local variable x referenced before assignment")
+
+
+---
+# A trailing comma is allowed in any function definition or call.
+# This reduces the need to edit neighboring lines when editing defs
+# or calls splayed across multiple lines.
+
+def a(x,): pass
+def b(x, y=None, ): pass
+def c(x, y=None, *args, ): pass
+def d(x, y=None, *args, z=None, ): pass
+def e(x, y=None, *args, z=None, **kwargs, ): pass
+
+a(1,)
+b(1, y=2, )
+#c(1, *[], )
+#d(1, *[], z=None, )
+#e(1, *[], z=None, *{}, )

syntax/grammar.txt

@@ -68,9 +68,6 @@ DotSuffix   = '.' identifier .
 CallSuffix  = '(' [Arguments [',']] ')' .
 SliceSuffix = '[' [Expression] [':' Test [':' Test]] ']' .
 
-# A CallSuffix does not allow a trailing comma
-# if the last argument is '*' Test or '**' Test.
-
 Arguments = Argument {',' Argument} .
 Argument  = Test | identifier '=' Test | '*' Test | '**' Test .
 

syntax/parse.go

@@ -414,7 +414,7 @@ func (p *parser) consume(t Token) Position {
 	return p.nextToken()
 }
 
-// params = (param COMMA)* param
+// params = (param COMMA)* param COMMA?
 //        |
 //
 // param = IDENT
@@ -432,22 +432,16 @@ func (p *parser) consume(t Token) Position {
 //      *Unary{Op: STARSTAR, X: *Ident}                 **kwargs
 func (p *parser) parseParams() []Expr {
 	var params []Expr
-	stars := false
 	for p.tok != RPAREN && p.tok != COLON && p.tok != EOF {
 		if len(params) > 0 {
 			p.consume(COMMA)
 		}
 		if p.tok == RPAREN {
-			// list can end with a COMMA if there is neither * nor **
-			if stars {
-				p.in.errorf(p.in.pos, "got %#v, want parameter", p.tok)
-			}
 			break
 		}
 
 		// * or *args or **kwargs
 		if p.tok == STAR || p.tok == STARSTAR {
-			stars = true
 			op := p.tok
 			pos := p.nextToken()
 			var x Expr
@@ -730,22 +724,16 @@ func (p *parser) parseCallSuffix(fn Expr) Expr {
 // arg_list = ((arg COMMA)* arg COMMA?)?
 func (p *parser) parseArgs() []Expr {
 	var args []Expr
-	stars := false
 	for p.tok != RPAREN && p.tok != EOF {
 		if len(args) > 0 {
 			p.consume(COMMA)
 		}
 		if p.tok == RPAREN {
-			// list can end with a COMMA if there is neither * nor **
-			if stars {
-				p.in.errorf(p.in.pos, `got %#v, want argument`, p.tok)
-			}
 			break
 		}
 
 		// *args or **kwargs
 		if p.tok == STAR || p.tok == STARSTAR {
-			stars = true
 			op := p.tok
 			pos := p.nextToken()
 			x := p.parseTest()

syntax/testdata/errors.star

@@ -11,19 +11,11 @@ x = 1 +
 _ = *x ### `got '\*', want primary`
 
 ---
+# trailing comma is ok
 
-def f(a, ):  # trailing comma is ok
-  pass
-
----
-
-def f(*args, ):  ### `got '\)', want parameter`
-  pass
-
----
-
-def f(**kwargs, ): ### `got '\)', want parameter`
-  pass
+def f(a, ): pass
+def f(*args, ): pass
+def f(**kwargs, ): pass
 
 ---
 
@@ -51,16 +43,11 @@ def pass(): ### "not an identifier"
 def f : ### `got ':', want '\('`
 
 ---
+# trailing comma is ok
 
-f(a, ) # trailing comma is ok
-
----
-
-f(*args, ) ### `got '\)', want argument`
-
----
-
-f(**kwargs, ) ### `got '\)', want argument`
+f(a, )
+f(*args, )
+f(**kwargs, )
 
 ---