fix: record positional optional params and **kwargs correctly

_appmap/event.py

@@ -156,10 +156,11 @@ def __repr__(self):
 
 
 class Param:
-    __slots__ = ["name", "kind", "default", "default_class"]
+    __slots__ = ["name", "kind", "positional_only", "default", "default_class"]
 
     def __init__(self, sigp):
         self.name = sigp.name
+        self.positional_only = sigp.kind == Parameter.POSITIONAL_ONLY
         has_default = sigp.default is not Signature.empty
         if sigp.kind in (Parameter.POSITIONAL_ONLY, Parameter.POSITIONAL_OR_KEYWORD):
             self.kind = "opt" if has_default else "req"
@@ -275,28 +276,34 @@ def set_params(params, instance, args, kwargs, display_value=False):
             # we're popping the front repeatedly, lists are better at this
             args = list(args)
 
+        # Names of keyword arguments bound to named parameters. Python
+        # doesn't include them in **kwargs, so neither do we.
+        bound = set()
         for p in params:
-            if p.kind == "req":
-                # A 'req' argument can be either keyword or
-                # positional.
-                if p.name in kwargs:
+            if p.kind in ("req", "opt"):
+                # These can be either keyword or positional, unless
+                # they're positional-only.
+                if not p.positional_only and p.name in kwargs:
                     value = kwargs[p.name]
+                    bound.add(p.name)
+                elif args:
+                    value = args.pop(0)
+                elif p.kind == "opt":
+                    value = p.default
                 else:
-                    if args:
-                        value = args.pop(0)
-                    else:
-                        continue  # required argument missing
-            elif p.kind == "keyreq":
+                    continue  # required argument missing
+            elif p.kind in ("keyreq", "key"):
                 if p.name in kwargs:
                     value = kwargs[p.name]
+                    bound.add(p.name)
+                elif p.kind == "key":
+                    value = p.default
                 else:
                     continue  # required argument missing
-            elif p.kind in ("opt", "key"):
-                value = kwargs.get(p.name, p.default)
             elif p.kind == "rest":
                 value = tuple(args)
             elif p.kind == "keyrest":
-                value = kwargs
+                value = {k: v for k, v in kwargs.items() if k not in bound} if bound else kwargs
             else:
                 # If all the parameter types are handled, this
                 # shouldn't ever happen...

_appmap/test/data/params.py

@@ -22,6 +22,9 @@ def args_kwargs(self, *args, **kwargs):
     def with_defaults(self, p1=1, p2=2):
         return "%s %s" % (p1, p2)
 
+    def mixed(self, a, b=None, c="dflt", *args, d, e="e", **kwargs):
+        return "%s %s %s %s %s %s %s" % (a, b, c, args, d, e, kwargs)
+
 
 if sys.version_info >= (3, 8):
     exec(
@@ -33,5 +36,9 @@ def positional_only(self, p1, p2, /):
 def keyword_only(self, *, p1, p2):
     return '%s %s' % (p1, p2)
 C.keyword_only = keyword_only
+
+def positional_only_kwargs(self, a, b="dflt", /, **kwargs):
+    return '%s %s %s' % (a, b, kwargs)
+C.positional_only_kwargs = positional_only_kwargs
     """
     )

_appmap/test/test_params.py

@@ -321,3 +321,161 @@ def test_keywords_defaults_with_args(self, params):
         self.assert_parameter(
             evt, 1, {"class": "builtins.int", "kind": "opt", "name": "p2", "value": "4"}
         )
+
+    @pytest.mark.parametrize("params", ["with_defaults"], indirect=True)
+    def test_defaults_positional(self, params):
+        evt = params.C().with_defaults(3, 4)
+        assert len(evt.parameters) == 2
+
+        self.assert_parameter(
+            evt, 0, {"class": "builtins.int", "kind": "opt", "name": "p1", "value": "3"}
+        )
+
+        self.assert_parameter(
+            evt, 1, {"class": "builtins.int", "kind": "opt", "name": "p2", "value": "4"}
+        )
+
+    @pytest.mark.parametrize("params", ["with_defaults"], indirect=True)
+    def test_defaults_positional_and_keyword(self, params):
+        evt = params.C().with_defaults(3, p2=4)
+        assert len(evt.parameters) == 2
+
+        self.assert_parameter(
+            evt, 0, {"class": "builtins.int", "kind": "opt", "name": "p1", "value": "3"}
+        )
+
+        self.assert_parameter(
+            evt, 1, {"class": "builtins.int", "kind": "opt", "name": "p2", "value": "4"}
+        )
+
+
+class TestMixedParams(TestMethodBase):
+    """
+    C.mixed has the signature (self, a, b=None, c="dflt", *args, d, e="e", **kwargs),
+    covering every parameter kind.
+    """
+
+    def assert_values(self, evt, expected):
+        actual = {p["name"]: (p["kind"], p["value"]) for p in evt.parameters}
+        assert actual == expected
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_positional_opt(self, params):
+        evt = params.C().mixed("A", "B", "C", d="D")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "B"),
+                "c": ("opt", "C"),
+                "args": ("rest", "()"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "e"),
+                "kwargs": ("keyrest", "{}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_partial_positional_opt(self, params):
+        evt = params.C().mixed("A", "B", d="D")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "B"),
+                "c": ("opt", "dflt"),
+                "args": ("rest", "()"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "e"),
+                "kwargs": ("keyrest", "{}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_keyword_opt(self, params):
+        evt = params.C().mixed("A", c="C", d="D")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "None"),
+                "c": ("opt", "C"),
+                "args": ("rest", "()"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "e"),
+                "kwargs": ("keyrest", "{}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_rest_after_opt(self, params):
+        evt = params.C().mixed("A", "B", "C", "X", "Y", d="D")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "B"),
+                "c": ("opt", "C"),
+                "args": ("rest", "('X', 'Y')"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "e"),
+                "kwargs": ("keyrest", "{}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_keyword_only(self, params):
+        evt = params.C().mixed("A", d="D", e="E", f="F")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "None"),
+                "c": ("opt", "dflt"),
+                "args": ("rest", "()"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "E"),
+                "kwargs": ("keyrest", "{'f': 'F'}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["mixed"], indirect=True)
+    def test_keyword_req(self, params):
+        evt = params.C().mixed(a="A", d="D", f="F")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "None"),
+                "c": ("opt", "dflt"),
+                "args": ("rest", "()"),
+                "d": ("keyreq", "D"),
+                "e": ("key", "e"),
+                "kwargs": ("keyrest", "{'f': 'F'}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["positional_only_kwargs"], indirect=True)
+    def test_positional_only_name_in_kwargs(self, params):
+        # Keyword arguments named like positional-only parameters go to **kwargs
+        evt = params.C().positional_only_kwargs("A", "B", a="X", b="Y")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "B"),
+                "kwargs": ("keyrest", "{'a': 'X', 'b': 'Y'}"),
+            },
+        )
+
+    @pytest.mark.parametrize("params", ["positional_only_kwargs"], indirect=True)
+    def test_positional_only_default_name_in_kwargs(self, params):
+        evt = params.C().positional_only_kwargs("A", b="Y")
+        self.assert_values(
+            evt,
+            {
+                "a": ("req", "A"),
+                "b": ("opt", "dflt"),
+                "kwargs": ("keyrest", "{'b': 'Y'}"),
+            },
+        )