gh-64490: Tweak AC implementation for **kwds (GH-139133)

Check for parameters following a var-keyword parameter in one place
instead of three, and improve the error messages for a var-keyword
parameter used with non-positional parameters and for a var-positional
or var-keyword parameter in an optional group.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Author: AA-Turner

Lib/test/test_clinic.py

@@ -379,28 +379,26 @@ def test_vararg_after_star(self):
         """
         self.expect_failure(block, err, lineno=6)
 
-    def test_double_star_after_var_keyword(self):
-        err = "Function 'my_test_func' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+    def test_parameter_after_var_keyword(self):
+        err = "parameters cannot follow var-keyword parameter: 'invalid_arg: object'"
         block = """
             /*[clinic input]
             my_test_func
 
-                pos_arg: object
                 **kwds: dict
-                **
+                invalid_arg: object
             [clinic start generated code]*/
         """
         self.expect_failure(block, err, lineno=5)
 
-    def test_var_keyword_after_star(self):
+    def test_double_star_without_name(self):
         err = "Function 'my_test_func' has an invalid parameter declaration: '**'"
         block = """
             /*[clinic input]
             my_test_func
 
                 pos_arg: object
                 **
-                **kwds: dict
             [clinic start generated code]*/
         """
         self.expect_failure(block, err, lineno=5)
@@ -2128,6 +2126,42 @@ def test_disallowed_grouping__varpos(self):
         """
         self.expect_failure(block, err, lineno=4)
 
+    def test_disallowed_grouping__parameter_after_group(self):
+        # Only positional-only parameters can follow an optional group.
+        group_err = ("You cannot use optional groups ('[' and ']') unless all "
+                     "parameters are positional-only ('/')")
+        kwds_err = ("cannot use a var-keyword parameter with pos-or-keyword "
+                    "or keyword-only parameters")
+        dataset = (("""
+            module foo
+            foo.bar
+                [
+                a: int
+                b: int
+                ]
+                y: int
+        """, group_err), ("""
+            module foo
+            foo.bar
+                [
+                a: int
+                b: int
+                ]
+                *
+                y: int
+        """, group_err), ("""
+            module foo
+            foo.bar
+                [
+                a: int
+                b: int
+                ]
+                **kwds: dict
+        """, kwds_err))
+        for block, err in dataset:
+            with self.subTest(block=block):
+                self.expect_failure(block, err)
+
     def test_disallowed_grouping__must_be_position_only(self):
         dataset = ("""
             with_kwds
@@ -2140,11 +2174,6 @@ def test_disallowed_grouping__must_be_position_only(self):
                 [
                 a: object
                 ]
-        """, """
-            with_kwds
-                [
-                **kwds: dict
-                ]
         """)
         err = (
             "You cannot use optional groups ('[' and ']') unless all "
@@ -2654,38 +2683,50 @@ def test_slash_after_var_keyword(self):
         block = """
             module foo
             foo.bar
-               x: int
-               y: int
                **kwds: dict
-               z: int
                /
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = "parameters cannot follow var-keyword parameter: '/'"
         self.expect_failure(block, err)
 
     def test_star_after_var_keyword(self):
         block = """
             module foo
             foo.bar
-               x: int
-               y: int
                **kwds: dict
-               z: int
                *
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = "parameters cannot follow var-keyword parameter: '*'"
         self.expect_failure(block, err)
 
     def test_parameter_after_var_keyword(self):
         block = """
             module foo
             foo.bar
-               x: int
-               y: int
                **kwds: dict
                z: int
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = "parameters cannot follow var-keyword parameter: 'z: int'"
+        self.expect_failure(block, err)
+
+    def test_group_with_var_keyword(self):
+        block = """
+            with_kwds
+                [
+                **kwds: dict
+                ]
+        """
+        err = "A var-keyword parameter cannot be in an optional group."
+        self.expect_failure(block, err)
+
+    def test_group_with_var_positional(self):
+        block = """
+            with_varpos
+                [
+                *args: tuple
+                ]
+        """
+        err = "A var-positional parameter cannot be in an optional group."
         self.expect_failure(block, err)
 
     def test_depr_star_must_come_after_slash(self):
@@ -2777,7 +2818,7 @@ def test_parameters_no_more_than_one_vararg(self):
         self.expect_failure(block, err, lineno=3)
 
     def test_parameters_no_more_than_one_var_keyword(self):
-        err = "Encountered parameter line when not expecting parameters: **var_keyword_2: dict"
+        err = "parameters cannot follow var-keyword parameter: '**var_keyword_2: dict'"
         block = """
             module foo
             foo.bar
@@ -3587,7 +3628,8 @@ def test_var_keyword_with_pos_or_kw(self):
                x: int
                **kwds: dict
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = ("Function 'bar' cannot use a var-keyword parameter with "
+               "pos-or-keyword or keyword-only parameters.")
         self.expect_failure(block, err)
 
     def test_var_keyword_with_kw_only(self):
@@ -3600,7 +3642,8 @@ def test_var_keyword_with_kw_only(self):
                y: int
                **kwds: dict
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = ("Function 'bar' cannot use a var-keyword parameter with "
+               "pos-or-keyword or keyword-only parameters.")
         self.expect_failure(block, err)
 
     def test_var_keyword_with_pos_or_kw_and_kw_only(self):
@@ -3614,7 +3657,8 @@ def test_var_keyword_with_pos_or_kw_and_kw_only(self):
                z: int
                **kwds: dict
         """
-        err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'"
+        err = ("Function 'bar' cannot use a var-keyword parameter with "
+               "pos-or-keyword or keyword-only parameters.")
         self.expect_failure(block, err)
 
     def test_allow_negative_accepted_by_py_ssize_t_converter_only(self):

Tools/clinic/libclinic/dsl_parser.py

@@ -951,6 +951,9 @@ def state_parameter(self, line: str) -> None:
             self.deprecated_until = self.parse_version(match[1], 'until')
             line = match[2]
 
+        if not self.expecting_parameters:
+            fail(f'parameters cannot follow var-keyword parameter: {line!r}')
+
         func = self.function
         match line:
             case '*':
@@ -967,10 +970,6 @@ def state_parameter(self, line: str) -> None:
     def parse_parameter(self, line: str) -> None:
         assert self.function is not None
 
-        if not self.expecting_parameters:
-            fail('Encountered parameter line when not expecting '
-                 f'parameters: {line}')
-
         match self.parameter_state:
             case ParamState.START | ParamState.REQUIRED:
                 self.to_required()
@@ -1021,8 +1020,9 @@ def parse_parameter(self, line: str) -> None:
                 for p in self.function.parameters.values()
             )
             if has_non_positional_param:
-                fail(f"Function {self.function.name!r} has an "
-                     f"invalid parameter declaration (**kwargs?): {line!r}")
+                fail(f'Function {self.function.name!r} cannot use a var-keyword '
+                     f'parameter with pos-or-keyword or keyword-only '
+                     f'parameters.')
             is_var_keyword = True
             parameter = function_args.kwarg
         else:
@@ -1161,6 +1161,9 @@ def bad_node(self, node: ast.AST) -> None:
 
         kind: inspect._ParameterKind
         if is_vararg:
+            if self.group_stack:
+                fail("A var-positional parameter cannot be in an optional "
+                     "group.")
             if any(p.group for p in self.function.parameters.values()):
                 # With "foo([a, b], *args)" the number of arguments does not
                 # tell whether the group is passed or all arguments belong
@@ -1169,6 +1172,9 @@ def bad_node(self, node: ast.AST) -> None:
                      f"groups with a var-positional parameter.")
             kind = inspect.Parameter.VAR_POSITIONAL
         elif is_var_keyword:
+            if self.group_stack:
+                fail("A var-keyword parameter cannot be in an optional "
+                     "group.")
             kind = inspect.Parameter.VAR_KEYWORD
         elif self.keyword_only:
             kind = inspect.Parameter.KEYWORD_ONLY
@@ -1287,9 +1293,6 @@ def parse_star(self, function: Function, version: VersionTuple | None) -> None:
         The 'version' parameter signifies the future version from which
         the marker will take effect (None means it is already in effect).
         """
-        if not self.expecting_parameters:
-            fail("Encountered '*' when not expecting parameters")
-
         if version is None:
             self.check_previous_star()
             self.check_remaining_star()
@@ -1356,9 +1359,6 @@ def parse_slash(self, function: Function, version: VersionTuple | None) -> None:
         The 'version' parameter signifies the future version from which
         the marker will take effect (None means it is already in effect).
         """
-        if not self.expecting_parameters:
-            fail("Encountered '/' when not expecting parameters")
-
         if version is None:
             if self.deprecated_keyword:
                 fail(f"Function {function.name!r}: '/' must precede '/ [from ...]'")