diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index a20624d2b..6d98fe671 100644 --- a/src/runtime/MethodBinder.cs +++ b/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() - : 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() + : 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(); } Exceptions.RaiseTypeError(value.ToString()); @@ -1123,6 +1142,86 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a return Converter.ToPython(result, returnType); } + /// + /// 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. + /// + 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(1) { new MethodInformation(info, true) }; + var parameterNames = new HashSet(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}'?"); + } + } + + /// + /// Closest parameter name to suggest, or null: small edit distance, or containment + /// between names of 3+ characters (e.g. 'as_tag' suggests 'tag'). + /// + private static string ClosestParameterName(string name, HashSet 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; + } + /// /// Utility class to store the information about a /// diff --git a/src/runtime/Types/ClassBase.cs b/src/runtime/Types/ClassBase.cs index 1342b6a3f..aa32662d8 100644 --- a/src/runtime/Types/ClassBase.cs +++ b/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]; - } } } diff --git a/src/runtime/Util/Util.cs b/src/runtime/Util/Util.cs index 45ee649a9..2e17911bf 100644 --- a/src/runtime/Util/Util.cs +++ b/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]; + } } } diff --git a/src/testing/methodtest.cs b/src/testing/methodtest.cs index fe49de88d..4b62c07a4 100644 --- a/src/testing/methodtest.cs +++ b/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"; diff --git a/tests/test_method.py b/tests/test_method.py index 07b5c5a34..7190f49f8 100644 --- a/tests/test_method.py +++ b/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"