fix(python): unable to override methods with keyword arguments (#3695)

The callback logic in the jsii runtime library for Python was
incorrectly passing keyword arguments to Python implementations,
forwarding a struct instance as-is instead of destructuring it into a
keyword arguments mapping.

Using `inspect.signature()`, detect the presence of keyword arguments on
the callback target, and destructure the struct as relevant.

Fixes #3656



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

.all-contributorsrc

@@ -1420,6 +1420,15 @@
       "contributions": [
         "code"
       ]
+    },
+    {
+      "login": "gshpychka",
+      "name": "Glib Shpychka",
+      "avatar_url": "https://avatars.githubusercontent.com/u/23005347?v=4",
+      "profile": "https://github.com/gshpychka",
+      "contributions": [
+        "bug"
+      ]
     }
   ],
   "repoType": "github",

packages/@jsii/python-runtime/src/jsii/_kernel/__init__.py

@@ -3,7 +3,7 @@
 import itertools
 from types import FunctionType, MethodType, BuiltinFunctionType, LambdaType
 
-from typing import Any, List, Optional, Type, Union
+from typing import cast, Any, List, Optional, Type
 
 import functools
 
@@ -98,26 +98,31 @@ def _get_overides(klass: Type, obj: Any) -> List[Override]:
                 if original is not _nothing:
                     if inspect.isfunction(item) and hasattr(original, "__jsii_name__"):
                         if any(
-                            entry.method == original.__jsii_name__
+                            entry.method == cast(Any, original).__jsii_name__
                             for entry in overrides
                         ):
                             # Don't re-register an override we already discovered through a previous type
                             continue
                         overrides.append(
-                            Override(method=original.__jsii_name__, cookie=name)
+                            Override(
+                                method=cast(Any, original).__jsii_name__, cookie=name
+                            )
                         )
                         break
                     elif inspect.isdatadescriptor(item) and hasattr(
                         getattr(original, "fget", None), "__jsii_name__"
                     ):
                         if any(
-                            entry.property == original.fget.__jsii_name__
+                            entry.property == cast(Any, original).fget.__jsii_name__
                             for entry in overrides
                         ):
                             # Don't re-register an override we already discovered through a previous type
                             continue
                         overrides.append(
-                            Override(property=original.fget.__jsii_name__, cookie=name)
+                            Override(
+                                property=cast(Any, original).fget.__jsii_name__,
+                                cookie=name,
+                            )
                         )
                         break
 
@@ -209,7 +214,33 @@ def _handle_callback(kernel, callback):
         hydrated_args = [
             _recursize_dereference(kernel, a) for a in callback.invoke.args
         ]
-        return method(*hydrated_args)
+
+        # If keyword arguments are accepted, we may need to turn a struct into keywords...
+        kwargs = {}  # No keyword arguments by default
+        params = inspect.signature(method).parameters
+        params_kwargs = [
+            name
+            for (name, param) in params.items()
+            if param.kind == inspect.Parameter.KEYWORD_ONLY
+        ]
+        if len(params_kwargs) > 0:
+            params_pos_count = len(
+                [
+                    param
+                    for param in params.values()
+                    if param.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD
+                    or param.kind == inspect.Parameter.POSITIONAL_ONLY
+                ]
+            )
+            if len(hydrated_args) > params_pos_count:
+                struct = hydrated_args.pop()
+                kwargs = {
+                    name: getattr(struct, name)
+                    for name in params_kwargs
+                    if hasattr(struct, name)
+                }
+
+        return method(*hydrated_args, **kwargs)
     elif callback.get:
         obj = _reference_map.resolve_id(callback.get.objref.ref)
         return getattr(obj, callback.cookie)

packages/@jsii/python-runtime/tests/test_python.py

@@ -5,6 +5,7 @@
 from jsii.errors import JSIIError
 import jsii_calc
 from jsii_calc.module2702 import IVpc, Vpc, IBaz, Baz
+from jsii_calc.jsii3656 import OverrideMe
 
 
 class TestErrorHandling:
@@ -53,6 +54,14 @@ def baz_interface_func(b: IBaz) -> None:
         baz_interface_func(baz)
 
 
+def test_overrides_method_with_kwargs() -> None:
+    class Overridden(OverrideMe):
+        def implement_me(self, *, name: str, count: jsii.Number = None) -> bool:
+            return name == "John Doe" and count is None
+
+    assert OverrideMe.call_abstract(Overridden())
+
+
 def find_struct_bases(x):
     ret = []
     seen = set([])

packages/jsii-calc/lib/index.ts

@@ -22,3 +22,4 @@ export * as module2530 from './module2530';
 export * as module2700 from './module2700';
 
 export * as cdk16625 from './cdk16625';
+export * as jsii3656 from './jsii3656';

packages/jsii-calc/lib/jsii3656/index.ts

@@ -0,0 +1,15 @@
+export abstract class OverrideMe {
+  public static callAbstract(receiver: OverrideMe): boolean {
+    return receiver.implementMe({
+      name: 'John Doe',
+      // Decided not to pass count here...
+    });
+  }
+
+  public abstract implementMe(opts: ImplementMeOpts): boolean;
+}
+
+export interface ImplementMeOpts {
+  readonly name: string;
+  readonly count?: number;
+}

packages/jsii-calc/test/assembly.jsii

@@ -226,6 +226,13 @@
       },
       "symbolId": "lib/calculator:composition"
     },
+    "jsii-calc.jsii3656": {
+      "locationInModule": {
+        "filename": "lib/index.ts",
+        "line": 25
+      },
+      "symbolId": "lib/jsii3656/index:"
+    },
     "jsii-calc.module2530": {
       "locationInModule": {
         "filename": "lib/index.ts",
@@ -15643,6 +15650,126 @@
       "namespace": "composition.CompositeOperation",
       "symbolId": "lib/calculator:composition.CompositeOperation.CompositionStringStyle"
     },
+    "jsii-calc.jsii3656.ImplementMeOpts": {
+      "assembly": "jsii-calc",
+      "datatype": true,
+      "docs": {
+        "stability": "stable"
+      },
+      "fqn": "jsii-calc.jsii3656.ImplementMeOpts",
+      "kind": "interface",
+      "locationInModule": {
+        "filename": "lib/jsii3656/index.ts",
+        "line": 12
+      },
+      "name": "ImplementMeOpts",
+      "namespace": "jsii3656",
+      "properties": [
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "stable"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/jsii3656/index.ts",
+            "line": 13
+          },
+          "name": "name",
+          "type": {
+            "primitive": "string"
+          }
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "stable"
+          },
+          "immutable": true,
+          "locationInModule": {
+            "filename": "lib/jsii3656/index.ts",
+            "line": 14
+          },
+          "name": "count",
+          "optional": true,
+          "type": {
+            "primitive": "number"
+          }
+        }
+      ],
+      "symbolId": "lib/jsii3656/index:ImplementMeOpts"
+    },
+    "jsii-calc.jsii3656.OverrideMe": {
+      "abstract": true,
+      "assembly": "jsii-calc",
+      "docs": {
+        "stability": "stable"
+      },
+      "fqn": "jsii-calc.jsii3656.OverrideMe",
+      "initializer": {
+        "docs": {
+          "stability": "stable"
+        }
+      },
+      "kind": "class",
+      "locationInModule": {
+        "filename": "lib/jsii3656/index.ts",
+        "line": 1
+      },
+      "methods": [
+        {
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/jsii3656/index.ts",
+            "line": 2
+          },
+          "name": "callAbstract",
+          "parameters": [
+            {
+              "name": "receiver",
+              "type": {
+                "fqn": "jsii-calc.jsii3656.OverrideMe"
+              }
+            }
+          ],
+          "returns": {
+            "type": {
+              "primitive": "boolean"
+            }
+          },
+          "static": true
+        },
+        {
+          "abstract": true,
+          "docs": {
+            "stability": "stable"
+          },
+          "locationInModule": {
+            "filename": "lib/jsii3656/index.ts",
+            "line": 9
+          },
+          "name": "implementMe",
+          "parameters": [
+            {
+              "name": "opts",
+              "type": {
+                "fqn": "jsii-calc.jsii3656.ImplementMeOpts"
+              }
+            }
+          ],
+          "returns": {
+            "type": {
+              "primitive": "boolean"
+            }
+          }
+        }
+      ],
+      "name": "OverrideMe",
+      "namespace": "jsii3656",
+      "symbolId": "lib/jsii3656/index:OverrideMe"
+    },
     "jsii-calc.module2530.MyClass": {
       "assembly": "jsii-calc",
       "docs": {
@@ -17525,5 +17652,5 @@
     }
   },
   "version": "3.20.120",
-  "fingerprint": "V1EIg5zjHYsZARPmK+3FUV/k1Jp/oN+y3WOnwLEkNJ8="
+  "fingerprint": "/9fE/+p2My22F013iCie8G2N8WE1ZwPqZKkeGKYCbyI="
 }