Name the unexpected keyword argument in the bind-failure TypeError (#144)

* Raise a proper unexpected-keyword-argument TypeError on bind failure

When a method call fails to bind and one of the supplied keyword arguments
matches no parameter of any candidate overload, the generic 'No method
matches given arguments' message did not mention the keyword argument at
all (only positional argument types are echoed), leaving the actual
mistake invisible, e.g.:

    market_order(symbol, -10, as_tag="EmergencyFlatten")
    -> No method matches given arguments for market_order:
       (<class 'Symbol'>, <class 'int'>). The following overloads ...

Now such calls raise the Python-style error instead, naming the offending
kwarg and suggesting the closest parameter name when one exists:

    market_order() got an unexpected keyword argument 'as_tag'.
    Did you mean 'tag'?

When every kwarg name is valid for some overload but binding still fails,
the existing no-method-matches message is preserved.

* Tighten comments in unexpected-keyword-argument error path

* Share the Levenshtein distance helper between ClassBase and MethodBinder

Moves ClassBase's private LevenshteinDistance implementation verbatim to
Util.LevenshteinDistance and uses it from both call sites, removing the
duplicate introduced for keyword-argument suggestions.

* Extend the no-match error with the unexpected keyword argument instead of replacing it

* Never let bind-failure message construction throw

Author: jhonabreul

src/runtime/MethodBinder.cs

@@ -1017,29 +1017,48 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a
                 if (!Exceptions.ErrorOccurred())
                 {
                     var value = new StringBuilder("No method matches given arguments");
-                    // Use the snake_case name Python callers use, matching the hinted signatures below.
-                    if (methodinfo != null && methodinfo.Length > 0)
+                    try
                     {
-                        value.Append($" for {MethodSignatureFormatter.SnakeCaseName(methodinfo[0])}");
-                    }
-                    else if (list.Count > 0)
-                    {
-                        value.Append($" for {MethodSignatureFormatter.SnakeCaseName(list[0].MethodBase)}");
-                    }
+                        // Use the snake_case name Python callers use, matching the hinted signatures below.
+                        if (methodinfo != null && methodinfo.Length > 0)
+                        {
+                            value.Append($" for {MethodSignatureFormatter.SnakeCaseName(methodinfo[0])}");
+                        }
+                        else if (list.Count > 0)
+                        {
+                            value.Append($" for {MethodSignatureFormatter.SnakeCaseName(list[0].MethodBase)}");
+                        }
 
-                    value.Append(": ");
-                    AppendArgumentTypes(to: value, args);
-
-                    // List the candidate overloads so the caller can see what was
-                    // expected (e.g. that an int overload exists when a float was
-                    // passed). Applies to every "no match" case, not just numeric ones.
-                    var candidates = methodinfo != null && methodinfo.Length > 0
-                        ? methodinfo.Cast<MethodBase>()
-                        : list?.Select(m => m.MethodBase);
-                    var overloads = MethodSignatureFormatter.FormatOverloads(candidates);
-                    if (overloads.Length > 0)
+                        value.Append(": ");
+                        AppendArgumentTypes(to: value, args);
+
+                        // The argument types echo above covers positional args only; name the first
+                        // unknown kwarg (if any) so a misspelled keyword argument is visible.
+                        AppendUnexpectedKeywordArgument(value, kw, info);
+
+                        // List the candidate overloads so the caller can see what was
+                        // expected (e.g. that an int overload exists when a float was
+                        // passed). Applies to every "no match" case, not just numeric ones.
+                        var candidates = methodinfo != null && methodinfo.Length > 0
+                            ? methodinfo.Cast<MethodBase>()
+                            : list?.Select(m => m.MethodBase);
+                        var overloads = MethodSignatureFormatter.FormatOverloads(candidates);
+                        if (overloads.Length > 0)
+                        {
+                            // The kwarg hint may already end the sentence with a question mark.
+                            if (value[value.Length - 1] != '?')
+                            {
+                                value.Append('.');
+                            }
+                            value.Append(' ').Append(overloads);
+                        }
+                    }
+                    catch
                     {
-                        value.Append(". ").Append(overloads);
+                        // The details above are best-effort diagnostics over arbitrary caller
+                        // input; an exception here would escape the tp_call slot into CPython
+                        // and mask the bind failure. Raise with whatever was appended so far.
+                        Exceptions.Clear();
                     }
 
                     // After the overloads block: consumers that extract the hint from
@@ -1131,6 +1150,86 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a
             return Converter.ToPython(result, returnType);
         }
 
+        /// <summary>
+        /// Appends "Got an unexpected keyword argument" to the no-match message when a kwarg
+        /// name is accepted by no candidate overload, with a "Did you mean" suggestion when a
+        /// similar parameter name exists. Appends nothing when every kwarg name is valid.
+        /// </summary>
+        private void AppendUnexpectedKeywordArgument(StringBuilder to, BorrowedReference kw, MethodBase info)
+        {
+            var kwCount = kw == null ? 0 : (int)Runtime.PyDict_Size(kw);
+            if (kwCount <= 0)
+            {
+                return;
+            }
+
+            // Same candidate set Bind considered; ParameterNames are already in the caller's convention.
+            var methods = info == null
+                ? GetMethods()
+                : new List<MethodInformation>(1) { new MethodInformation(info, true) };
+            var parameterNames = new HashSet<string>(StringComparer.Ordinal);
+            foreach (var method in methods)
+            {
+                foreach (var parameterName in method.ParameterNames)
+                {
+                    parameterNames.Add(parameterName);
+                }
+            }
+
+            // Report the first unknown kwarg in call order, like CPython does.
+            string unexpectedName = null;
+            using (var keyList = Runtime.PyDict_Keys(kw))
+            {
+                for (var i = 0; i < kwCount && unexpectedName == null; i++)
+                {
+                    var name = Runtime.GetManagedString(Runtime.PyList_GetItem(keyList.Borrow(), i));
+                    if (name != null && !parameterNames.Contains(name))
+                    {
+                        unexpectedName = name;
+                    }
+                }
+            }
+
+            if (unexpectedName == null)
+            {
+                return;
+            }
+
+            to.Append($". Got an unexpected keyword argument '{unexpectedName}'");
+            var suggestion = ClosestParameterName(unexpectedName, parameterNames);
+            if (suggestion != null)
+            {
+                to.Append($". Did you mean '{suggestion}'?");
+            }
+        }
+
+        /// <summary>
+        /// Closest parameter name to suggest, or null: small edit distance, or containment
+        /// between names of 3+ characters (e.g. 'as_tag' suggests 'tag').
+        /// </summary>
+        private static string ClosestParameterName(string name, HashSet<string> parameterNames)
+        {
+            const int MinContainmentLength = 3;
+            var threshold = Math.Max(2, name.Length / 3);
+            string best = null;
+            var bestDistance = int.MaxValue;
+            foreach (var candidate in parameterNames)
+            {
+                var distance = Util.LevenshteinDistance(name, candidate);
+                var related = distance <= threshold
+                    || (candidate.Length >= MinContainmentLength && name.Length >= MinContainmentLength
+                        && (candidate.IndexOf(name, StringComparison.OrdinalIgnoreCase) >= 0
+                            || name.IndexOf(candidate, StringComparison.OrdinalIgnoreCase) >= 0));
+                if (related && (distance < bestDistance
+                    || (distance == bestDistance && string.CompareOrdinal(candidate, best) < 0)))
+                {
+                    bestDistance = distance;
+                    best = candidate;
+                }
+            }
+            return best;
+        }
+
         /// <summary>
         /// Utility class to store the information about a <see cref="MethodBase"/>
         /// </summary>

src/runtime/Types/ClassBase.cs

@@ -845,7 +845,7 @@ private static string ComputeSimilarMemberNames(Type type, string name)
             var scored = new List<(string Name, int Distance, SuggestionKind Kind)>();
             foreach (var candidate in GetCandidateMemberNames(type))
             {
-                var distance = LevenshteinDistance(name, candidate.Key);
+                var distance = Util.LevenshteinDistance(name, candidate.Key);
                 var related = distance <= threshold
                     || candidate.Key.IndexOf(name, StringComparison.OrdinalIgnoreCase) >= 0
                     || name.IndexOf(candidate.Key, StringComparison.OrdinalIgnoreCase) >= 0;
@@ -895,30 +895,5 @@ private static (string Name, SuggestionKind Kind) ToSnakeCaseMemberName(MemberIn
             };
         }
 
-        private static int LevenshteinDistance(string a, string b)
-        {
-            a = a.ToLowerInvariant();
-            b = b.ToLowerInvariant();
-            var n = a.Length;
-            var m = b.Length;
-            if (n == 0) return m;
-            if (m == 0) return n;
-
-            var prev = new int[m + 1];
-            var curr = new int[m + 1];
-            for (var j = 0; j <= m; j++) prev[j] = j;
-
-            for (var i = 1; i <= n; i++)
-            {
-                curr[0] = i;
-                for (var j = 1; j <= m; j++)
-                {
-                    var cost = a[i - 1] == b[j - 1] ? 0 : 1;
-                    curr[j] = Math.Min(Math.Min(curr[j - 1] + 1, prev[j] + 1), prev[j - 1] + cost);
-                }
-                (prev, curr) = (curr, prev);
-            }
-            return prev[m];
-        }
     }
 }

src/runtime/Util/Util.cs

@@ -336,5 +336,32 @@ public static bool IsInteger(this TypeCode typeCode)
                     return false;
             }
         }
+
+        // Case-insensitive Levenshtein distance.
+        internal static int LevenshteinDistance(string a, string b)
+        {
+            a = a.ToLowerInvariant();
+            b = b.ToLowerInvariant();
+            var n = a.Length;
+            var m = b.Length;
+            if (n == 0) return m;
+            if (m == 0) return n;
+
+            var prev = new int[m + 1];
+            var curr = new int[m + 1];
+            for (var j = 0; j <= m; j++) prev[j] = j;
+
+            for (var i = 1; i <= n; i++)
+            {
+                curr[0] = i;
+                for (var j = 1; j <= m; j++)
+                {
+                    var cost = a[i - 1] == b[j - 1] ? 0 : 1;
+                    curr[j] = Math.Min(Math.Min(curr[j - 1] + 1, prev[j] + 1), prev[j - 1] + cost);
+                }
+                (prev, curr) = (curr, prev);
+            }
+            return prev[m];
+        }
     }
 }

src/testing/methodtest.cs

@@ -709,6 +709,11 @@ public static string DefaultParamsWithOverloading(int a = 5, int b = 6, int c =
             return $"{a}{b}{c}{d}XXX";
         }
 
+        public static string OrderLikeMethod(string symbol, decimal quantity, bool asynchronous = false, string tag = "", object orderProperties = null)
+        {
+            return string.Format("{0}:{1}:{2}:{3}", symbol, quantity, asynchronous, tag);
+        }
+
         public static string ParamsArrayOverloaded(int i = 1)
         {
             return "without params-array";

tests/test_method.py

@@ -1104,6 +1104,52 @@ def test_default_params():
     with pytest.raises(TypeError):
         MethodTest.DefaultParams(1,2,3,4,5)
 
+def test_unexpected_keyword_argument_with_suggestion():
+    with pytest.raises(TypeError) as excinfo:
+        MethodTest.order_like_method("SPY", 10, as_tag="EmergencyFlatten")
+    message = str(excinfo.value)
+    assert "No method matches given arguments for order_like_method" in message
+    assert "Got an unexpected keyword argument 'as_tag'" in message
+    assert "Did you mean 'tag'?" in message
+
+    # PascalCase call path: parameter names are the original ones.
+    with pytest.raises(TypeError) as excinfo:
+        MethodTest.OrderLikeMethod("SPY", 10, asTag="EmergencyFlatten")
+    message = str(excinfo.value)
+    assert "No method matches given arguments for order_like_method" in message
+    assert "Got an unexpected keyword argument 'asTag'" in message
+    assert "Did you mean 'tag'?" in message
+
+
+def test_unexpected_keyword_argument_without_suggestion():
+    with pytest.raises(TypeError) as excinfo:
+        MethodTest.order_like_method("SPY", 10, completely_unrelated_name=1)
+    message = str(excinfo.value)
+    assert "No method matches given arguments for order_like_method" in message
+    assert "Got an unexpected keyword argument " \
+           "'completely_unrelated_name'" in message
+    assert "Did you mean" not in message
+
+
+def test_unexpected_keyword_argument_reports_first_in_call_order():
+    with pytest.raises(TypeError) as excinfo:
+        MethodTest.order_like_method("SPY", 10, first_bogus=1, second_bogus=2)
+    assert "Got an unexpected keyword argument 'first_bogus'" in str(excinfo.value)
+
+
+def test_valid_keyword_arguments_still_bind():
+    res = MethodTest.order_like_method("SPY", 10, asynchronous=True, tag="mytag")
+    assert res == "SPY:10:True:mytag"
+
+
+def test_valid_keyword_argument_names_keep_no_match_message():
+    # 'd' is supplied both positionally and by name: valid names, unbindable call.
+    with pytest.raises(TypeError) as excinfo:
+        MethodTest.DefaultParams(1, 2, 3, 4, d=5)
+    message = str(excinfo.value)
+    assert "No method matches given arguments for default_params" in message
+    assert "unexpected keyword argument" not in message
+
 def test_optional_params():
     res = MethodTest.OptionalParams(1, 2, 3, 4)
     assert res == "1234"