diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index 043bd95a2..05b463f40 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/src/pya/pya/pyaCallables.cc @@ -193,10 +193,33 @@ num_args (const gsi::MethodBase *m) return int (m->end_arguments () - m->begin_arguments ()); } +std::set +invalid_kwnames (const gsi::MethodBase *meth, PyObject *kwargs) +{ + pya::PythonRef keys (PyDict_Keys (kwargs)); + + std::set valid_names; + for (gsi::MethodBase::argument_iterator a = meth->begin_arguments (); a != meth->end_arguments (); ++a) { + valid_names.insert (a->spec ()->name ()); + } + + std::set invalid_names; + for (int i = int (PyList_Size (keys.get ())); i > 0; ) { + --i; + std::string k = python2c (PyList_GetItem (keys.get (), i)); + if (valid_names.find (k) == valid_names.end ()) { + invalid_names.insert (k); + } + } + + return invalid_names; +} + static bool compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs, std::string *why_not = 0) { int nargs = num_args (m); + int nkwargs = kwargs == NULL ? 0 : int (PyDict_Size (kwargs)); if (argc > nargs) { if (why_not) { @@ -205,7 +228,7 @@ compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs, std: return false; } else if (argc == nargs) { // no more arguments to consider - if (kwargs != NULL && PyDict_Size (kwargs) > 0) { + if (nkwargs > 0) { if (why_not) { *why_not = tl::to_string (tr ("all arguments given, but additional keyword arguments specified")); } @@ -235,7 +258,20 @@ compatible_with_args (const gsi::MethodBase *m, int argc, PyObject *kwargs, std: ++argc; } - return true; + if (kwargs_taken != nkwargs) { + if (why_not) { + std::set invalid_names = invalid_kwnames (m, kwargs); + if (invalid_names.size () > 1) { + std::string names_str = tl::join (invalid_names.begin (), invalid_names.end (), ", "); + *why_not = tl::to_string (tr ("unknown keyword parameters: ")) + names_str; + } else if (invalid_names.size () == 1) { + *why_not = tl::to_string (tr ("unknown keyword parameter: ")) + *invalid_names.begin (); + } + } + return false; + } else { + return true; + } } else { @@ -746,23 +782,7 @@ push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, PyObject *args if (kwargs_taken != nkwargs) { // check if there are any left-over keyword parameters with unknown names - - pya::PythonRef keys (PyDict_Keys (kwargs)); - - std::set valid_names; - for (gsi::MethodBase::argument_iterator a = meth->begin_arguments (); a != meth->end_arguments (); ++a) { - valid_names.insert (a->spec ()->name ()); - } - - std::set invalid_names; - for (int i = int (PyList_Size (keys.get ())); i > 0; ) { - --i; - std::string k = python2c (PyList_GetItem (keys.get (), i)); - if (valid_names.find (k) == valid_names.end ()) { - invalid_names.insert (k); - } - } - + std::set invalid_names = invalid_kwnames (meth, kwargs); if (invalid_names.size () > 1) { std::string names_str = tl::join (invalid_names.begin (), invalid_names.end (), ", "); throw tl::Exception (tl::to_string (tr ("Unknown keyword parameters: ")) + names_str); diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index dee16050b..e6ab2a357 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -182,6 +182,27 @@ get_kwarg (const gsi::ArgType &atype, VALUE kwargs) } } +static int get_kwargs_keys (VALUE key, VALUE, VALUE arg) +{ + std::set *names = reinterpret_cast *> (arg); + names->insert (ruby2c (rba_safe_obj_as_string (key))); + + return ST_CONTINUE; +} + +static std::set +invalid_kwnames (const gsi::MethodBase *meth, VALUE kwargs) +{ + std::set invalid_names; + rb_hash_foreach (kwargs, (int (*)(...)) &get_kwargs_keys, (VALUE) &invalid_names); + + for (gsi::MethodBase::argument_iterator a = meth->begin_arguments (); a != meth->end_arguments (); ++a) { + invalid_names.erase (a->spec ()->name ()); + } + + return invalid_names; +} + // ------------------------------------------------------------------- // The lookup table for the method overload resolution @@ -325,10 +346,12 @@ public: } private: + static bool compatible_with_args (const gsi::MethodBase *m, int argc, VALUE kwargs, std::string *why_not = 0) { int nargs = num_args (m); + int nkwargs = kwargs == Qnil ? 0 : RHASH_SIZE (kwargs); if (argc > nargs) { if (why_not) { @@ -337,7 +360,7 @@ private: return false; } else if (argc == nargs) { // no more arguments to consider - if (kwargs != Qnil && RHASH_SIZE (kwargs) > 0) { + if (nkwargs > 0) { if (why_not) { *why_not = tl::to_string (tr ("all arguments given, but additional keyword arguments specified")); } @@ -367,7 +390,20 @@ private: ++argc; } - return true; + if (kwargs_taken != nkwargs) { + if (why_not) { + std::set invalid_names = invalid_kwnames (m, kwargs); + if (invalid_names.size () > 1) { + std::string names_str = tl::join (invalid_names.begin (), invalid_names.end (), ", "); + *why_not = tl::to_string (tr ("unknown keyword parameters: ")) + names_str; + } else if (invalid_names.size () == 1) { + *why_not = tl::to_string (tr ("unknown keyword parameter: ")) + *invalid_names.begin (); + } + } + return false; + } else { + return true; + } } else { @@ -1063,14 +1099,6 @@ static gsi::ArgType create_void_type () static gsi::ArgType s_void_type = create_void_type (); -static int get_kwargs_keys (VALUE key, VALUE, VALUE arg) -{ - std::set *names = reinterpret_cast *> (arg); - names->insert (ruby2c (rba_safe_obj_as_string (key))); - - return ST_CONTINUE; -} - void push_args (gsi::SerialArgs &arglist, const gsi::MethodBase *meth, VALUE *argv, int argc, VALUE kwargs, tl::Heap &heap) { diff --git a/testdata/python/basic.py b/testdata/python/basic.py index c24e2d477..2c43d39cd 100644 --- a/testdata/python/basic.py +++ b/testdata/python/basic.py @@ -3261,14 +3261,14 @@ class BasicTest(unittest.TestCase): bb.d4(1, "a", 2.0, xxx=17) except Exception as ex: m = str(ex) - self.assertEqual(m, "Unknown keyword parameter: xxx in BB.d4") + self.assertEqual(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [unknown keyword parameter: xxx]\n in BB.d4") m = "" try: bb.d4(a=1, b="a", c=2.0, xxx=17) except Exception as ex: m = str(ex) - self.assertEqual(m, "Unknown keyword parameter: xxx in BB.d4") + self.assertEqual(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [unknown keyword parameter: xxx]\n in BB.d4") self.assertEqual(bb.d4(1, "a", 2.0), "1,a,2,100,nil") self.assertEqual(bb.d4(1, "a", 2.0, e=42), "1,a,2,100,42") diff --git a/testdata/ruby/basic_testcore.rb b/testdata/ruby/basic_testcore.rb index 4464012af..b706f9ee7 100644 --- a/testdata/ruby/basic_testcore.rb +++ b/testdata/ruby/basic_testcore.rb @@ -3181,7 +3181,7 @@ class Basic_TestClass < TestBase rescue => ex m = ex.to_s end - assert_equal(m, "Unknown keyword parameter: xxx in BB::d4") + assert_equal(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [unknown keyword parameter: xxx]\n in BB::d4") m = "" begin @@ -3189,7 +3189,7 @@ class Basic_TestClass < TestBase rescue => ex m = ex.to_s end - assert_equal(m, "Unknown keyword parameter: xxx in BB::d4") + assert_equal(m, "Can't match arguments. Variants are:\n string d4(int a, string b, double c, B3::E d = E3A, variant e = nil) [unknown keyword parameter: xxx]\n in BB::d4") assert_equal(bb.d4(1, "a", 2.0), "1,a,2,100,nil") assert_equal(bb.d4(1, "a", 2.0, e: 42), "1,a,2,100,42")