Support function calling with keyword arguments in invoke.

https://bugzilla.gnome.org/show_bug.cgi?id=625596

gi/pygi-cache.c

@@ -109,6 +109,9 @@ _pygi_callable_cache_free (PyGICallableCache *cache)
         return;
 
     g_slist_free (cache->out_args);
+    g_slist_free (cache->arg_name_list);
+    g_hash_table_destroy (cache->arg_name_hash);
+
     for (i = 0; i < cache->n_args; i++) {
         PyGIArgCache *tmp = cache->args_cache[i];
         _pygi_arg_cache_free (tmp);
@@ -1205,6 +1208,38 @@ _arg_cache_new (GITypeInfo *type_info,
     return arg_cache;
 }
 
+static void
+_arg_name_list_generate (PyGICallableCache *callable_cache)
+{
+    GSList * arg_name_list = NULL;
+
+    if (callable_cache->arg_name_hash == NULL) {
+        callable_cache->arg_name_hash = g_hash_table_new (g_str_hash, g_str_equal);
+    } else {
+        g_hash_table_remove_all (callable_cache->arg_name_hash);
+    }
+
+    for (int i=0; i < callable_cache->n_args; i++) {
+        PyGIArgCache *arg_cache = NULL;
+
+        arg_cache = callable_cache->args_cache[i];
+
+        if (arg_cache->meta_type != PYGI_META_ARG_TYPE_CHILD &&
+                (arg_cache->direction == GI_DIRECTION_IN ||
+                 arg_cache->direction == GI_DIRECTION_INOUT)) {
+
+            gpointer arg_name = (gpointer)arg_cache->arg_name;
+
+            arg_name_list = g_slist_prepend (arg_name_list, arg_name);
+            if (arg_name != NULL) {
+                g_hash_table_insert (callable_cache->arg_name_hash, arg_name, arg_name);
+            }
+        }
+    }
+
+    callable_cache->arg_name_list = g_slist_reverse (arg_name_list);
+}
+
 /* Generate the cache for the callable's arguments */
 static gboolean
 _args_cache_generate (GICallableInfo *callable_info,
@@ -1328,6 +1363,7 @@ _args_cache_generate (GICallableInfo *callable_info,
         if (arg_cache == NULL)
             goto arg_err;
 
+        arg_cache->arg_name = g_base_info_get_name ((GIBaseInfo *) arg_info);
         arg_cache->allow_none = g_arg_info_may_be_null(arg_info);
         arg_cache->is_caller_allocates = is_caller_allocates;
 
@@ -1351,6 +1387,9 @@ arg_err:
         g_base_info_unref( (GIBaseInfo *)arg_info);
         return FALSE;
     }
+
+    _arg_name_list_generate (callable_cache);
+
     return TRUE;
 }
 

gi/pygi-cache.h

@@ -82,6 +82,8 @@ typedef enum {
 
 struct _PyGIArgCache
 {
+    const gchar *arg_name;
+
     PyGIMetaArgType meta_type;
     gboolean is_pointer;
     gboolean is_caller_allocates;
@@ -150,6 +152,8 @@ struct _PyGICallableCache
     PyGIArgCache *return_cache;
     PyGIArgCache **args_cache;
     GSList *out_args;
+    GSList *arg_name_list; /* for keyword arg matching */
+    GHashTable *arg_name_hash;
 
     /* counts */
     gssize n_in_args;

gi/pygi-invoke.c

@@ -84,15 +84,156 @@ _invoke_callable (PyGIInvokeState *state,
     return TRUE;
 }
 
+static gboolean
+_check_for_unexpected_kwargs (const gchar *function_name,
+                              GHashTable  *arg_name_hash,
+                              PyObject    *py_kwargs)
+{
+    PyObject *dict_key, *dict_value;
+    Py_ssize_t dict_iter_pos = 0;
+
+    while (PyDict_Next (py_kwargs, &dict_iter_pos, &dict_key, &dict_value)) {
+        PyObject *key;
+
+#if PY_VERSION_HEX < 0x03000000
+        if (PyString_Check (dict_key)) {
+            Py_INCREF (dict_key);
+            key = dict_key;
+        } else
+#endif
+        {
+            key = PyUnicode_AsUTF8String (dict_key);
+            if (key == NULL) {
+                return FALSE;
+            }
+        }
+
+        if (g_hash_table_lookup (arg_name_hash, PyBytes_AsString(key)) == NULL) {
+            PyErr_Format (PyExc_TypeError,
+                          "%.200s() got an unexpected keyword argument '%.400s'",
+                          function_name,
+                          PyBytes_AsString (key));
+            Py_DECREF (key);
+            return FALSE;
+        }
+
+        Py_DECREF (key);
+    }
+    return TRUE;
+}
+
+/**
+ * _py_args_combine_and_check_length:
+ * @function_name: the name of the function being called. Used for error messages.
+ * @arg_name_list: a list of the string names for each argument. The length
+ *                 of this list is the number of required arguments for the
+ *                 function. If an argument has no name, NULL is put in its
+ *                 position in the list.
+ * @py_args: the tuple of positional arguments. A referece is stolen, and this
+             tuple will be either decreffed or returned as is.
+ * @py_kwargs: the dict of keyword arguments to be merged with py_args.
+ *             A reference is borrowed.
+ *
+ * Returns: The py_args and py_kwargs combined into one tuple.
+ */
+static PyObject *
+_py_args_combine_and_check_length (const gchar *function_name,
+                                   GSList      *arg_name_list,
+                                   GHashTable  *arg_name_hash,
+                                   PyObject    *py_args,
+                                   PyObject    *py_kwargs)
+{
+    PyObject *combined_py_args = NULL;
+    Py_ssize_t n_py_args, n_py_kwargs, i;
+    guint n_expected_args;
+    GSList *l;
+
+    n_py_args = PyTuple_GET_SIZE (py_args);
+    n_py_kwargs = PyDict_Size (py_kwargs);
+
+    n_expected_args = g_slist_length (arg_name_list);
+
+    if (n_py_kwargs == 0 && n_py_args == n_expected_args) {
+        return py_args;
+    }
+
+    if (n_expected_args < n_py_args) {
+        PyErr_Format (PyExc_TypeError,
+                      "%.200s() takes exactly %d %sargument%s (%zd given)",
+                      function_name,
+                      n_expected_args,
+                      n_py_kwargs > 0 ? "non-keyword " : "",
+                      n_expected_args == 1 ? "" : "s",
+                      n_py_args);
+
+        Py_DECREF (py_args);
+        return NULL;
+    }
+
+    if (!_check_for_unexpected_kwargs (function_name, arg_name_hash, py_kwargs)) {
+        Py_DECREF (py_args);
+        return NULL;
+    }
+
+    /* will hold arguments from both py_args and py_kwargs
+     * when they are combined into a single tuple */
+    combined_py_args = PyTuple_New (n_expected_args);
+
+    for (i = 0; i < n_py_args; i++) {
+        PyObject *item = PyTuple_GET_ITEM (py_args, i);
+        Py_INCREF (item);
+        PyTuple_SET_ITEM (combined_py_args, i, item);
+    }
+
+    Py_CLEAR(py_args);
+
+    for (i = 0, l = arg_name_list; i < n_expected_args && l; i++, l = l->next) {
+        PyObject *py_arg_item, *kw_arg_item = NULL;
+        const gchar *arg_name = l->data;
+
+        if (arg_name != NULL) {
+            /* NULL means this argument has no keyword name */
+            /* ex. the first argument to a method or constructor */
+            kw_arg_item = PyDict_GetItemString (py_kwargs, arg_name);
+        }
+        py_arg_item = PyTuple_GET_ITEM (combined_py_args, i);
+
+        if (kw_arg_item != NULL && py_arg_item == NULL) {
+            Py_INCREF (kw_arg_item);
+            PyTuple_SET_ITEM (combined_py_args, i, kw_arg_item);
+
+        } else if (kw_arg_item == NULL && py_arg_item == NULL) {
+            PyErr_Format (PyExc_TypeError,
+                          "%.200s() takes exactly %d %sargument%s (%zd given)",
+                          function_name,
+                          n_expected_args,
+                          n_py_kwargs > 0 ? "non-keyword " : "",
+                          n_expected_args == 1 ? "" : "s",
+                          n_py_args);
+
+            Py_DECREF (combined_py_args);
+            return NULL;
+
+        } else if (kw_arg_item != NULL && py_arg_item != NULL) {
+            PyErr_Format (PyExc_TypeError,
+                          "%.200s() got multiple values for keyword argument '%.200s'",
+                          function_name,
+                          arg_name);
+
+            Py_DECREF (combined_py_args);
+            return NULL;
+        }
+    }
+
+    return combined_py_args;
+}
+
 static inline gboolean
 _invoke_state_init_from_callable_cache (PyGIInvokeState *state,
                                         PyGICallableCache *cache,
                                         PyObject *py_args,
                                         PyObject *kwargs)
 {
-    state->py_in_args = py_args;
-    state->n_py_in_args = PySequence_Length (py_args);
-
     state->implementor_gtype = 0;
 
     /* TODO: We don't use the class parameter sent in by  the structure
@@ -135,12 +276,23 @@ _invoke_state_init_from_callable_cache (PyGIInvokeState *state,
          * code more error prone and confusing so don't do that unless profiling shows
          * significant gain
          */
-        state->py_in_args = PyTuple_GetSlice (py_args, 1, state->n_py_in_args);
-        state->n_py_in_args--;
+        state->py_in_args = PyTuple_GetSlice (py_args, 1, PyTuple_Size (py_args));
     } else {
+        state->py_in_args = py_args;
         Py_INCREF (state->py_in_args);
     }
 
+    state->py_in_args = _py_args_combine_and_check_length (cache->name,
+                                                           cache->arg_name_list,
+                                                           cache->arg_name_hash,
+                                                           state->py_in_args,
+                                                           kwargs);
+
+    if (state->py_in_args == NULL) {
+        return FALSE;
+    }
+    state->n_py_in_args = PyTuple_Size (state->py_in_args);
+
     state->args = g_slice_alloc0 (cache->n_args * sizeof (GIArgument *));
     if (state->args == NULL && cache->n_args != 0) {
         PyErr_NoMemory();

gi/types.py

@@ -40,8 +40,8 @@ if sys.version_info > (3, 0):
 
 def Function(info):
 
-    def function(*args):
-        return info.invoke(*args)
+    def function(*args, **kwargs):
+        return info.invoke(*args, **kwargs)
     function.__info__ = info
     function.__name__ = info.get_name()
     function.__module__ = info.get_namespace()
@@ -51,8 +51,8 @@ def Function(info):
 
 def NativeVFunc(info, cls):
 
-    def native_vfunc(*args):
-        return info.invoke(cls.__gtype__, *args)
+    def native_vfunc(*args, **kwargs):
+        return info.invoke(cls.__gtype__, *args, **kwargs)
     native_vfunc.__info__ = info
     native_vfunc.__name__ = info.get_name()
     native_vfunc.__module__ = info.get_namespace()
@@ -61,11 +61,11 @@ def NativeVFunc(info, cls):
 
 def Constructor(info):
 
-    def constructor(cls, *args):
+    def constructor(cls, *args, **kwargs):
         cls_name = info.get_container().get_name()
         if cls.__name__ != cls_name:
             raise TypeError('%s constructor cannot be used to create instances of a subclass' % cls_name)
-        return info.invoke(cls, *args)
+        return info.invoke(cls, *args, **kwargs)
 
     constructor.__info__ = info
     constructor.__name__ = info.get_name()

tests/test_everything.py

@@ -110,7 +110,7 @@ class TestEverything(unittest.TestCase):
             Everything.test_int8()
         except TypeError:
             (e_type, e) = sys.exc_info()[:2]
-            self.assertEquals(e.args, ("test_int8() takes exactly 1 argument(s) (0 given)",))
+            self.assertEquals(e.args, ("test_int8() takes exactly 1 argument (0 given)",))
 
     def test_gtypes(self):
         gchararray_gtype = GObject.type_from_name('gchararray')

tests/test_gi.py

@@ -1791,3 +1791,61 @@ class TestGErrorArrayInCrash(unittest.TestCase):
     # take in GArrays. See https://bugzilla.gnome.org/show_bug.cgi?id=642708
     def test_gerror_array_in_crash(self):
         self.assertRaises(GObject.GError, GIMarshallingTests.gerror_array_in, [1, 2, 3])
+
+class TestKeywordArgs(unittest.TestCase):
+    def test_calling(self):
+        kw_func = GIMarshallingTests.int_three_in_three_out
+
+        self.assertEquals(kw_func(1, 2, 3),                 (1, 2, 3))
+        self.assertEquals(kw_func(**{'a':4, 'b':5, 'c':6}), (4, 5, 6))
+        self.assertEquals(kw_func(1, **{'b':7, 'c':8}),     (1, 7, 8))
+        self.assertEquals(kw_func(1, 7, **{'c':8}),         (1, 7, 8))
+        self.assertEquals(kw_func(1, c=8, **{'b':7}),       (1, 7, 8))
+        self.assertEquals(kw_func(2, c=4, b=3),             (2, 3, 4))
+        self.assertEquals(kw_func(a=2, c=4, b=3),           (2, 3, 4))
+
+    def assertRaisesMessage(self, exception, message, func, *args, **kwargs):
+        try:
+            func(*args, **kwargs)
+        except exception:
+            (e_type, e) = sys.exc_info()[:2]
+            if message is not None:
+                self.assertEqual(str(e), message)
+        except:
+            raise
+        else:
+            msg = "%s() did not raise %s" % (func.__name__, exception.__name__)
+            raise AssertionError(msg)
+
+    def test_type_errors(self):
+        # test too few args
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 arguments (0 given)",
+                                 GIMarshallingTests.int_three_in_three_out)
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 arguments (1 given)",
+                                 GIMarshallingTests.int_three_in_three_out, 1)
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 arguments (0 given)",
+                                 GIMarshallingTests.int_three_in_three_out, *())
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 arguments (0 given)",
+                                 GIMarshallingTests.int_three_in_three_out, *(), **{})
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 non-keyword arguments (0 given)",
+                                 GIMarshallingTests.int_three_in_three_out, *(), **{'c':4})
+
+        # test too many args
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 arguments (4 given)",
+                                 GIMarshallingTests.int_three_in_three_out, *(1, 2, 3, 4))
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() takes exactly 3 non-keyword arguments (4 given)",
+                                 GIMarshallingTests.int_three_in_three_out, *(1, 2, 3, 4), c=6)
+
+        # test too many keyword args
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() got multiple values for keyword argument 'a'",
+                                 GIMarshallingTests.int_three_in_three_out, 1, 2, 3, **{'a': 4, 'b': 5})
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() got an unexpected keyword argument 'd'",
+                                 GIMarshallingTests.int_three_in_three_out, d=4)
+        self.assertRaisesMessage(TypeError, "int_three_in_three_out() got an unexpected keyword argument 'e'",
+                                 GIMarshallingTests.int_three_in_three_out, **{'e': 2})
+
+    def test_kwargs_are_not_modified(self):
+        d = {'b': 2}
+        d2 = d.copy()
+        GIMarshallingTests.int_three_in_three_out(1, c=4, **d)
+        self.assertEqual(d, d2)