[GR-23242] Make test_keywordonlyarg.

ppisl · ppisl · commit 9bb52f0c5113 · 2020-08-20T10:51:46.000Z
PullRequest: graalpython/1218
diff --git a/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/BasicTests.java b/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/BasicTests.java
@@ -365,6 +365,11 @@ public void call20() throws Exception {
         checkScopeAndTree("def fn(name): \"in\".format(name=name)");
     }
 
+    @Test
+    public void call21() throws Exception {
+        checkSyntaxErrorMessage("f(p, k1=50, *(1,2), k1=100)", "SyntaxError: keyword argument repeated");
+    }
+
     @Test
     public void del01() throws Exception {
         checkTreeResult("del x");
diff --git a/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/FuncDefTests.java b/graalpython/com.oracle.graal.python.test/src/com/oracle/graal/python/test/parser/FuncDefTests.java
@@ -487,6 +487,16 @@ public void positionalOnlyArg47() throws Exception {
         checkSyntaxError("async def f(a, *, c, /, d, e): pass");
     }
 
+    @Test
+    public void positionalOnlyArg48() throws Exception {
+        checkSyntaxErrorMessage("def f(p1, *k1, k1=100): pass ", "SyntaxError: duplicate argument 'k1' in function definition");
+    }
+
+    @Test
+    public void positionalOnlyArg49() throws Exception {
+        checkSyntaxErrorMessage("def f(p1, *, k1, **k1): pass ", "SyntaxError: duplicate argument 'k1' in function definition");
+    }
+
     @Test
     public void issue21351() throws Exception {
         // parser test for GR-21351
diff --git a/graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_keywordonlyarg.txt b/graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_keywordonlyarg.txt
@@ -1,6 +1,8 @@
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testFunctionCall
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testKwDefaults
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testRaiseErrorFuncallWithUnexpectedKeywordArgument
+*graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testSyntaxErrorForFunctionCall
+*graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testSyntaxErrorForFunctionDefinition
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testSyntaxForManyArguments
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.testTooManyPositionalErrorMessage
 *graalpython.lib-python.3.test.test_keywordonlyarg.KeywordOnlyArgTestCase.test_default_evaluation_order
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/antlr/Python3.g4 b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/antlr/Python3.g4
@@ -632,7 +632,11 @@ kwargsparameter [ArgDefListBuilder args]
 	'**' NAME
 	{ SSTNode type = null; }
 	( ':' test { type = $test.result; } )?
-	{ args.addKwargs($NAME.text, type); }
+	{ 
+            if (args.addKwargs($NAME.text, type) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {
+                throw new PythonRecognitionException("duplicate argument '" + $NAME.text + "' in function definition", this, _input, $ctx, getCurrentToken());
+            }
+        }
 ;
 
 varargslist returns [ArgDefListBuilder result]
@@ -697,7 +701,11 @@ vsplatparameter [ArgDefListBuilder args]
 vkwargsparameter [ArgDefListBuilder args]
 :
 	'**' NAME
-	{args.addKwargs($NAME.text, null);}
+	{
+            if (args.addKwargs($NAME.text, null) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {
+                throw new PythonRecognitionException("duplicate argument '" + $NAME.text + "' in function definition", this, _input, $ctx, getCurrentToken());
+            }
+        }
 ;
 
 stmt
@@ -1691,7 +1699,11 @@ argument [ArgListBuilder args] returns [SSTNode result]
                 } 
                     '=' test 
                     { 
-                        args.addNamedArg(name, $test.result); 
+                        if (!args.hasNameArg(name)) {
+                            args.addNamedArg(name, $test.result); 
+                        } else {
+                            throw new PythonRecognitionException("keyword argument repeated", this, _input, _localctx, getCurrentToken());
+                        }
                     }
 	|
 		test {  
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/antlr/Python3Parser.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/antlr/Python3Parser.java
@@ -1787,7 +1787,11 @@ public final KwargsparameterContext kwargsparameter(ArgDefListBuilder args) thro
 				}
 			}
 
-			 args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), type); 
+			 
+			            if (args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), type) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {
+			                throw new PythonRecognitionException("duplicate argument '" + (_localctx.NAME!=null?_localctx.NAME.getText():null) + "' in function definition", this, _input, _localctx, getCurrentToken());
+			            }
+			        
 			}
 		}
 		catch (RecognitionException re) {
@@ -2321,7 +2325,11 @@ public final VkwargsparameterContext vkwargsparameter(ArgDefListBuilder args) th
 			match(POWER);
 			setState(565);
 			_localctx.NAME = match(NAME);
-			args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), null);
+
+			            if (args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), null) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {
+			                throw new PythonRecognitionException("duplicate argument '" + (_localctx.NAME!=null?_localctx.NAME.getText():null) + "' in function definition", this, _input, _localctx, getCurrentToken());
+			            }
+			        
 			}
 		}
 		catch (RecognitionException re) {
@@ -8267,7 +8275,11 @@ public final ArgumentContext argument(ArgListBuilder args) throws RecognitionExc
 				setState(1595);
 				_localctx.test = test();
 				 
-				                        args.addNamedArg(name, _localctx.test.result); 
+				                        if (!args.hasNameArg(name)) {
+				                            args.addNamedArg(name, _localctx.test.result); 
+				                        } else {
+				                            throw new PythonRecognitionException("keyword argument repeated", this, _input, _localctx, getCurrentToken());
+				                        }
 				                    
 				}
 				break;
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgDefListBuilder.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgDefListBuilder.java
@@ -129,7 +129,7 @@ public static enum AddParamResult {
     }
 
     public AddParamResult addParam(String name, SSTNode type, SSTNode defValue) {
-        if (paramNames.contains(name)) {
+        if (paramNames.contains(name) || (splatIndex != -1 && name.equals(args.get(splatIndex).name))) {
             return AddParamResult.DUPLICATED_ARGUMENT;
         }
         if (defValue == null && hasDefaultParameter() && !hasSplat()) {
@@ -175,13 +175,17 @@ public void addSplat(String name, SSTNode type) {
         splatIndex = args.size() - 1;
     }
 
-    public void addKwargs(String name, SSTNode type) {
+    public AddParamResult addKwargs(String name, SSTNode type) {
         // System.out.println("KwArg: " + name);
+        if (paramNames.contains(name) || (splatIndex != -1 && name.equals(args.get(splatIndex).name))) {
+            return AddParamResult.DUPLICATED_ARGUMENT;
+        }
         if (kwargs == null) {
             kwargs = new ArrayList<>();
         }
         kwargs.add(new Parameter(name, type));
         kwargIndex = kwargs.size() - 1;
+        return AddParamResult.OK;
     }
 
     public boolean hasDefaultParameter() {
diff --git a/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgListBuilder.java b/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/parser/sst/ArgListBuilder.java
@@ -143,6 +143,10 @@ public boolean hasNameArg() {
         return !(nameArgNodes == null || nameArgNodes.isEmpty());
     }
 
+    public boolean hasNameArg(String name) {
+        return nameArgNames != null && nameArgNames.contains(name);
+    }
+
     public void addKwArg(SSTNode value) {
         if (kwArg == null) {
             kwArg = new ArrayList<>();

Original file line number	Diff line number	Diff line change
`@@ -632,7 +632,11 @@ kwargsparameter [ArgDefListBuilder args]`
`632`	`632`	`'**' NAME`
`633`	`633`	`{ SSTNode type = null; }`
`634`	`634`	`( ':' test { type = $test.result; } )?`
`635`		`- { args.addKwargs($NAME.text, type); }`
	`635`	`+ {`
	`636`	`+ if (args.addKwargs($NAME.text, type) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {`
	`637`	`+ throw new PythonRecognitionException("duplicate argument '" + $NAME.text + "' in function definition", this, _input, $ctx, getCurrentToken());`
	`638`	`+ }`
	`639`	`+ }`
`636`	`640`	`;`
`637`	`641`
`638`	`642`	`varargslist returns [ArgDefListBuilder result]`
`@@ -697,7 +701,11 @@ vsplatparameter [ArgDefListBuilder args]`
`697`	`701`	`vkwargsparameter [ArgDefListBuilder args]`
`698`	`702`	`:`
`699`	`703`	`'**' NAME`
`700`		`- {args.addKwargs($NAME.text, null);}`
	`704`	`+ {`
	`705`	`+ if (args.addKwargs($NAME.text, null) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {`
	`706`	`+ throw new PythonRecognitionException("duplicate argument '" + $NAME.text + "' in function definition", this, _input, $ctx, getCurrentToken());`
	`707`	`+ }`
	`708`	`+ }`
`701`	`709`	`;`
`702`	`710`
`703`	`711`	`stmt`
`@@ -1691,7 +1699,11 @@ argument [ArgListBuilder args] returns [SSTNode result]`
`1691`	`1699`	`}`
`1692`	`1700`	`'=' test`
`1693`	`1701`	`{`
`1694`		`- args.addNamedArg(name, $test.result);`
	`1702`	`+ if (!args.hasNameArg(name)) {`
	`1703`	`+ args.addNamedArg(name, $test.result);`
	`1704`	`+ } else {`
	`1705`	`+ throw new PythonRecognitionException("keyword argument repeated", this, _input, _localctx, getCurrentToken());`
	`1706`	`+ }`
`1695`	`1707`	`}`
`1696`	`1708`	`\|`
`1697`	`1709`	`test {`
Original file line number	Diff line number	Diff line change
`@@ -1787,7 +1787,11 @@ public final KwargsparameterContext kwargsparameter(ArgDefListBuilder args) thro`
`1787`	`1787`	`}`
`1788`	`1788`	`}`
`1789`	`1789`
`1790`		`- args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), type);`
	`1790`	`+`
	`1791`	`+ if (args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), type) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {`
	`1792`	`+ throw new PythonRecognitionException("duplicate argument '" + (_localctx.NAME!=null?_localctx.NAME.getText():null) + "' in function definition", this, _input, _localctx, getCurrentToken());`
	`1793`	`+ }`
	`1794`	`+`
`1791`	`1795`	`}`
`1792`	`1796`	`}`
`1793`	`1797`	`catch (RecognitionException re) {`
`@@ -2321,7 +2325,11 @@ public final VkwargsparameterContext vkwargsparameter(ArgDefListBuilder args) th`
`2321`	`2325`	`match(POWER);`
`2322`	`2326`	`setState(565);`
`2323`	`2327`	`_localctx.NAME = match(NAME);`
`2324`		`- args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), null);`
	`2328`	`+`
	`2329`	`+ if (args.addKwargs((_localctx.NAME!=null?_localctx.NAME.getText():null), null) == ArgDefListBuilder.AddParamResult.DUPLICATED_ARGUMENT) {`
	`2330`	`+ throw new PythonRecognitionException("duplicate argument '" + (_localctx.NAME!=null?_localctx.NAME.getText():null) + "' in function definition", this, _input, _localctx, getCurrentToken());`
	`2331`	`+ }`
	`2332`	`+`
`2325`	`2333`	`}`
`2326`	`2334`	`}`
`2327`	`2335`	`catch (RecognitionException re) {`
`@@ -8267,7 +8275,11 @@ public final ArgumentContext argument(ArgListBuilder args) throws RecognitionExc`
`8267`	`8275`	`setState(1595);`
`8268`	`8276`	`_localctx.test = test();`
`8269`	`8277`
`8270`		`- args.addNamedArg(name, _localctx.test.result);`
	`8278`	`+ if (!args.hasNameArg(name)) {`
	`8279`	`+ args.addNamedArg(name, _localctx.test.result);`
	`8280`	`+ } else {`
	`8281`	`+ throw new PythonRecognitionException("keyword argument repeated", this, _input, _localctx, getCurrentToken());`
	`8282`	`+ }`
`8271`	`8283`
`8272`	`8284`	`}`
`8273`	`8285`	`break;`
Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ public static enum AddParamResult {`
`129`	`129`	`}`
`130`	`130`
`131`	`131`	`public AddParamResult addParam(String name, SSTNode type, SSTNode defValue) {`
`132`		`- if (paramNames.contains(name)) {`
	`132`	`+ if (paramNames.contains(name) \|\| (splatIndex != -1 && name.equals(args.get(splatIndex).name))) {`
`133`	`133`	`return AddParamResult.DUPLICATED_ARGUMENT;`
`134`	`134`	`}`
`135`	`135`	`if (defValue == null && hasDefaultParameter() && !hasSplat()) {`
`@@ -175,13 +175,17 @@ public void addSplat(String name, SSTNode type) {`
`175`	`175`	`splatIndex = args.size() - 1;`
`176`	`176`	`}`
`177`	`177`
`178`		`- public void addKwargs(String name, SSTNode type) {`
	`178`	`+ public AddParamResult addKwargs(String name, SSTNode type) {`
`179`	`179`	`// System.out.println("KwArg: " + name);`
	`180`	`+ if (paramNames.contains(name) \|\| (splatIndex != -1 && name.equals(args.get(splatIndex).name))) {`
	`181`	`+ return AddParamResult.DUPLICATED_ARGUMENT;`
	`182`	`+ }`
`180`	`183`	`if (kwargs == null) {`
`181`	`184`	`kwargs = new ArrayList<>();`
`182`	`185`	`}`
`183`	`186`	`kwargs.add(new Parameter(name, type));`
`184`	`187`	`kwargIndex = kwargs.size() - 1;`
	`188`	`+ return AddParamResult.OK;`
`185`	`189`	`}`
`186`	`190`
`187`	`191`	`public boolean hasDefaultParameter() {`