From eb92e5f2d1f000577d1cbfde49a1aa9b7bcd685a Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 25 May 2024 18:43:44 +0200 Subject: [PATCH] Better error messages on argument mismatch for Ruby too. --- src/rba/rba/rba.cc | 39 ++++++++++++++++++++++++++------- testdata/ruby/basic_testcore.rb | 22 +++++++++---------- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/rba/rba/rba.cc b/src/rba/rba/rba.cc index f05031a2c..dee16050b 100644 --- a/src/rba/rba/rba.cc +++ b/src/rba/rba/rba.cc @@ -326,18 +326,29 @@ public: private: static bool - compatible_with_args (const gsi::MethodBase *m, int argc, VALUE kwargs) + compatible_with_args (const gsi::MethodBase *m, int argc, VALUE kwargs, std::string *why_not = 0) { int nargs = num_args (m); - if (argc >= nargs) { + if (argc > nargs) { + if (why_not) { + *why_not = tl::sprintf (tl::to_string (tr ("%d argument(s) expected, but %d given")), nargs, argc); + } + return false; + } else if (argc == nargs) { // no more arguments to consider - return argc == nargs && (kwargs == Qnil || RHASH_SIZE (kwargs) == 0); + if (kwargs != Qnil && RHASH_SIZE (kwargs) > 0) { + if (why_not) { + *why_not = tl::to_string (tr ("all arguments given, but additional keyword arguments specified")); + } + return false; + } else { + return true; + } } if (kwargs != Qnil) { - int nkwargs = int (RHASH_SIZE (kwargs)); int kwargs_taken = 0; while (argc < nargs) { @@ -345,6 +356,9 @@ private: VALUE rb_arg = rb_hash_lookup2 (kwargs, ID2SYM (rb_intern (atype.spec ()->name ().c_str ())), Qnil); if (rb_arg == Qnil) { if (! atype.spec ()->has_default ()) { + if (why_not) { + *why_not = tl::sprintf (tl::to_string (tr ("no argument specified for '%s' (neither positional or keyword)")), atype.spec ()->name ()); + } return false; } } else { @@ -353,14 +367,20 @@ private: ++argc; } - // matches if all keyword arguments are taken - return kwargs_taken == nkwargs; + return true; } else { while (argc < nargs) { const gsi::ArgType &atype = m->begin_arguments () [argc]; if (! atype.spec ()->has_default ()) { + if (why_not) { + if (argc < nargs - 1 && ! m->begin_arguments () [argc + 1].spec ()->has_default ()) { + *why_not = tl::sprintf (tl::to_string (tr ("no value given for argument #%d and following")), argc + 1); + } else { + *why_not = tl::sprintf (tl::to_string (tr ("no value given for argument #%d")), argc + 1); + } + } return false; } ++argc; @@ -375,8 +395,11 @@ private: describe_overload (const gsi::MethodBase *m, int argc, VALUE kwargs) { std::string res = m->to_string (); - if (compatible_with_args (m, argc, kwargs)) { + std::string why_not; + if (compatible_with_args (m, argc, kwargs, &why_not)) { res += " " + tl::to_string (tr ("[match candidate]")); + } else if (! why_not.empty ()) { + res += " [" + why_not + "]"; } return res; } @@ -1043,7 +1066,7 @@ 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 (key)); + names->insert (ruby2c (rba_safe_obj_as_string (key))); return ST_CONTINUE; } diff --git a/testdata/ruby/basic_testcore.rb b/testdata/ruby/basic_testcore.rb index 60aeff65d..4464012af 100644 --- a/testdata/ruby/basic_testcore.rb +++ b/testdata/ruby/basic_testcore.rb @@ -3165,7 +3165,7 @@ class Basic_TestClass < TestBase rescue => ex m = ex.to_s end - 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) [no value given for argument #1 and following]\n 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) [no value given for argument #1 and following]\n in BB::d4") m = "" begin @@ -3173,30 +3173,30 @@ class Basic_TestClass < TestBase rescue => ex m = ex.to_s end - 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) [no value given for argument #3]\n 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) [no value given for argument #3]\n in BB::d4") m = "" begin - bb.d4(1, "a", 2.0, xxx=17) + bb.d4(1, "a", 2.0, xxx: 17) rescue => ex m = ex.to_s end - assert_equal(m, "Unknown keyword parameter: xxx in BB.d4") + assert_equal(m, "Unknown keyword parameter: xxx in BB::d4") m = "" begin - bb.d4(a=1, b="a", c=2.0, xxx=17) + bb.d4(a: 1, b: "a", c: 2.0, xxx: 17) rescue => ex m = ex.to_s end - assert_equal(m, "Unknown keyword parameter: xxx in BB.d4") + assert_equal(m, "Unknown keyword parameter: xxx 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") - assert_equal(bb.d4(1, "a", c=2.0, e=42), "1,a,2,100,42") - assert_equal(bb.d4(c=2.0, a=1, b="a", e=42), "1,a,2,100,42") - assert_equal(bb.d4(1, "a", 2.0, d=RBA::BB::E::E3B), "1,a,2,101,nil") - assert_equal(bb.d4(1, "a", d=RBA::BB::E::E3B, c=2.5), "1,a,2.5,101,nil") + assert_equal(bb.d4(1, "a", 2.0, e: 42), "1,a,2,100,42") + assert_equal(bb.d4(1, "a", c=2.0, e: 42), "1,a,2,100,42") + assert_equal(bb.d4(c: 2.0, a: 1, b: "a", e: 42), "1,a,2,100,42") + assert_equal(bb.d4(1, "a", 2.0, d: RBA::BB::E::E3B), "1,a,2,101,nil") + assert_equal(bb.d4(1, "a", d: RBA::BB::E::E3B, c: 2.5), "1,a,2.5,101,nil") assert_equal(bb.d4(1, "a", 2.0, RBA::BB::E::E3B, 42), "1,a,2,101,42") end