Fix the definition of Argument in the Skylark specification

PiperOrigin-RevId: 191717115
mmikitka · Apr 5, 2018 · 0c148f3 · 0c148f3
1 parent 832a0b1
commit 0c148f3
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 4 deletions.
diff --git a/site/docs/skylark/spec.md b/site/docs/skylark/spec.md
@@ -1635,10 +1635,10 @@ print(x)                        # 1
 ### Function and method calls
 
 ```text
-CallSuffix = '(' [Arguments] ')' .
+CallSuffix = '(' [Arguments [',']] ')' .
 
 Arguments = Argument {',' Argument} .
-Argument  = Test | identifier '=' Test | '*' identifier | '**' identifier .
+Argument  = Test | identifier '=' Test | '*' Test | '**' Test .
 ```
 
 A value `f` of type `function` may be called using the expression `f(...)`.
@@ -3161,6 +3161,7 @@ File = {Statement | newline} eof .
 Statement = DefStmt | IfStmt | ForStmt | SimpleStmt .
 
 DefStmt = 'def' identifier '(' [Parameters [',']] ')' ':' Suite .
+# NOTE: trailing comma is not permitted if the last argument is `'*' identifier` or `'**' identifier`.
 
 Parameters = Parameter {',' Parameter}.
 
@@ -3214,11 +3215,12 @@ Operand = identifier
         .
 
 DotSuffix   = '.' identifier .
-CallSuffix  = '(' [Arguments [',']] ')' .
 SliceSuffix = '[' [Expression] [':' Test [':' Test]] ']' .
+CallSuffix  = '(' [Arguments [',']] ')' .
+# NOTE: trailing comma is not permitted if the last argument is `'*' Test` or `'**' Test`.
 
 Arguments = Argument {',' Argument} .
-Argument  = Test | identifier '=' Test | '*' identifier | '**' identifier .
+Argument  = Test | identifier '=' Test | '*' Test | '**' Test .
 
 ListExpr = '[' [Expression [',']] ']' .
 ListComp = '[' Test {CompClause} ']'.

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
@@ -157,4 +157,14 @@ public void testKwParam() throws Exception {
         .isEqualTo("{\"name\": \"foo\", \"type\": \"jpg\", \"version\": 42}");
     assertThat(Printer.repr(lookup("b2"))).isEqualTo("{}");
   }
+
+  @Test
+  public void testCommaAfterArgsAndKwargs() throws Exception {
+    // Test that commas are not allowed in function definitions and calls
+    // after last *args or **kwargs expressions.
+    checkEvalErrorContains("syntax error at ')': expected identifier", "def foo(*args,): pass");
+    checkEvalErrorContains("unexpected tokens after kwarg", "def foo(**kwargs,): pass");
+    checkEvalErrorContains("syntax error at ')': expected expression", "foo(*args,)");
+    checkEvalErrorContains("unexpected tokens after kwarg", "foo(**kwargs,)");
+  }
 }