diff --git a/src/embed_tests/TestBindFailureDiagnosis.cs b/src/embed_tests/TestBindFailureDiagnosis.cs new file mode 100644 index 000000000..1251e52d2 --- /dev/null +++ b/src/embed_tests/TestBindFailureDiagnosis.cs @@ -0,0 +1,109 @@ +using NUnit.Framework; +using Python.Runtime; + +namespace Python.EmbeddingTest +{ + /// + /// The bind-failure TypeError must pinpoint the first argument that fails to + /// match the nearest overload. + /// + public class TestBindFailureDiagnosis + { + public class OrdersTarget + { + public string PlaceOrder(string symbol, decimal quantity, bool asynchronous = false, string tag = "", int depth = 0) => "decimal"; + public string PlaceOrder(string symbol, int quantity, bool asynchronous = false, string tag = "", int depth = 0) => "int"; + } + + public class SingleOverloadTarget + { + public int Compute(int periods) => periods; + } + + [OneTimeSetUp] + public void SetUp() + { + PythonEngine.Initialize(); + } + + [OneTimeTearDown] + public void Dispose() + { + PythonEngine.Shutdown(); + } + + private static string TypeErrorMessageOf(string call) + { + using var _ = Py.GIL(); + var module = PyModule.FromString("TestBindFailureDiagnosis_" + TestContext.CurrentContext.Test.Name, $@" +from clr import AddReference +AddReference(""Python.EmbeddingTest"") +AddReference(""System"") + +from Python.EmbeddingTest import * + +def get_error(): + target = TestBindFailureDiagnosis.OrdersTarget() + single = TestBindFailureDiagnosis.SingleOverloadTarget() + try: + {call} + except TypeError as e: + return str(e) + return None +"); + using var result = module.GetAttr("get_error").Invoke(); + Assert.IsFalse(result.IsNone(), "expected the call to raise a TypeError"); + return result.As(); + } + + [Test] + public void PinpointsFirstMismatchedPositionalArgument() + { + var message = TypeErrorMessageOf("target.place_order('SPY', -10, 'exit signal')"); + + Assert.That(message, Does.StartWith("No method matches given arguments for place_order: ")); + Assert.That(message, Does.Contain("The following overloads are available:")); + Assert.That(message, Does.Contain("Argument mismatch: argument 3 ('asynchronous') expected bool, got str.")); + } + + [Test] + public void PinpointsMismatchedKeywordArgument() + { + var message = TypeErrorMessageOf("target.place_order('SPY', 10, tag=5)"); + + Assert.That(message, Does.Contain("Argument mismatch: keyword argument 'tag' expected str, got int.")); + } + + [Test] + public void PinpointsMismatchOnSingleOverloadMethods() + { + var message = TypeErrorMessageOf("single.compute('abc')"); + + Assert.That(message, Does.Contain("The expected signature is:")); + Assert.That(message, Does.Contain("Argument mismatch: argument 1 ('periods') expected int, got str.")); + } + + [Test] + public void SkipsDiagnosisWhenAllGivenArgumentsMatch() + { + // Pure arity failure: no mismatched argument to single out. + var message = TypeErrorMessageOf("single.compute(1, 2)"); + + Assert.That(message, Does.Contain("No method matches given arguments for compute")); + Assert.That(message, Does.Not.Contain("Argument mismatch:")); + } + + [Test] + public void DiagnosisSurvivesTheOverloadsHintExtraction() + { + // Lean keeps the message from the overloads marker onwards; the diagnosis + // must be inside that region to reach users. + var message = TypeErrorMessageOf("target.place_order('SPY', -10, 'exit signal')"); + + var hintStart = message.IndexOf("The following overloads are available:"); + Assert.GreaterOrEqual(hintStart, 0); + var hint = message.Substring(hintStart); + Assert.That(hint, Does.Contain("Argument mismatch: argument 3 ('asynchronous') expected bool, got str.")); + } + } +} diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index a20624d2b..9138f6ab5 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -1042,6 +1042,14 @@ internal virtual NewReference Invoke(BorrowedReference inst, BorrowedReference a value.Append(". ").Append(overloads); } + // After the overloads block: consumers that extract the hint from + // its marker onwards must keep this line too. + var mismatch = DiagnoseClosestOverloadMismatch(candidates, args, kw); + if (mismatch.Length > 0) + { + value.Append('\n').Append(mismatch); + } + Exceptions.RaiseTypeError(value.ToString()); } @@ -1216,6 +1224,228 @@ public MatchedMethod(int kwargsMatched, object?[] margs, int outs, MethodInforma } } + /// + /// One-line diagnosis of the first argument failing to match the nearest + /// overload (most leading convertible arguments), e.g. "Argument mismatch: + /// argument 3 ('asynchronous') expected bool, got str." Empty when nothing + /// conclusive (e.g. pure arity mismatch). Never throws, never leaves a + /// Python error pending. + /// + private static string DiagnoseClosestOverloadMismatch(IEnumerable candidates, BorrowedReference args, BorrowedReference kw) + { + try + { + if (candidates == null) + { + return string.Empty; + } + + var pyArgCount = args == null ? 0 : (int)Runtime.PyTuple_Size(args); + + // Strong references: the values must outlive the candidate probing. + List> kwargs = null; + if (kw != null && Runtime.PyDict_Size(kw) > 0) + { + kwargs = new List>(); + using var keyList = Runtime.PyDict_Keys(kw); + using var valueList = Runtime.PyDict_Values(kw); + var kwCount = (int)Runtime.PyList_Size(keyList.Borrow()); + for (var i = 0; i < kwCount; i++) + { + var name = Runtime.GetManagedString(Runtime.PyList_GetItem(keyList.Borrow(), i)); + if (name != null) + { + kwargs.Add(new KeyValuePair( + name, new PyObject(Runtime.PyList_GetItem(valueList.Borrow(), i)))); + } + } + } + + var bestScore = -1; + var bestMismatchIndex = -1; + ParameterInfo bestMismatchParameter = null; + string bestKwargName = null; + PyObject bestKwargValue = null; + + foreach (var method in candidates) + { + if (method == null || OperatorMethod.IsOperatorMethod(method)) + { + continue; + } + + var pi = method.GetParameters(); + var paramsArrayIndex = pi.Length > 0 && Attribute.IsDefined(pi[pi.Length - 1], typeof(ParamArrayAttribute)) + ? pi.Length - 1 + : -1; + + var score = 0; + var mismatchIndex = -1; + var limit = Math.Min(pyArgCount, pi.Length); + for (var i = 0; i < limit; i++) + { + if (i == paramsArrayIndex) + { + // Params-array element conversions aren't probed; count the tail as matched. + score = limit; + break; + } + + var op = Runtime.PyTuple_GetItem(args, i); + if (op == null) + { + Exceptions.Clear(); + break; + } + + if (!ArgumentMatchesParameter(op, pi[i])) + { + mismatchIndex = i; + break; + } + score++; + } + + string kwargName = null; + PyObject kwargValue = null; + ParameterInfo kwargParameter = null; + if (mismatchIndex == -1 && kwargs != null) + { + foreach (var pair in kwargs) + { + var parameter = pi.FirstOrDefault(p => p.Name == pair.Key || p.Name.ToSnakeCase() == pair.Key); + if (parameter == null) + { + // Unknown keyword names are not this diagnosis' job. + continue; + } + + if (ArgumentMatchesParameter(pair.Value.Reference, parameter)) + { + score++; + } + else + { + kwargName = pair.Key; + kwargValue = pair.Value; + kwargParameter = parameter; + break; + } + } + } + + if (mismatchIndex == -1 && kwargName == null) + { + // Everything given matched: nothing to pinpoint for this candidate. + continue; + } + + if (score > bestScore) + { + bestScore = score; + bestMismatchIndex = mismatchIndex; + bestKwargName = kwargName; + bestKwargValue = kwargValue; + bestMismatchParameter = mismatchIndex != -1 ? pi[mismatchIndex] : kwargParameter; + } + } + + if (bestMismatchParameter == null) + { + return string.Empty; + } + + var expected = MethodSignatureFormatter.FormatType(bestMismatchParameter.ParameterType); + var parameterName = bestMismatchParameter.Name.ToSnakeCase(); + if (bestKwargName != null) + { + return $"Argument mismatch: keyword argument '{bestKwargName}' expected {expected}, got {Runtime.PyObject_GetTypeName(bestKwargValue.Reference)}."; + } + + var mismatchedArg = Runtime.PyTuple_GetItem(args, bestMismatchIndex); + var got = mismatchedArg == null ? Util.BadStr : Runtime.PyObject_GetTypeName(mismatchedArg); + return $"Argument mismatch: argument {bestMismatchIndex + 1} ('{parameterName}') expected {expected}, got {got}."; + } + catch + { + // Best-effort hint only; never mask the original failure. + return string.Empty; + } + finally + { + // Conversion probes may have left a Python error set. + Exceptions.Clear(); + } + } + + /// + /// Mirror of the binder's per-argument acceptance rules, used to find the first + /// mismatching argument. Lenient where probing is unreliable (by-ref, generic + /// and untyped parameters) so it under-reports rather than blames the wrong + /// argument. + /// + private static bool ArgumentMatchesParameter(BorrowedReference op, ParameterInfo parameter) + { + var parameterType = parameter.ParameterType; + if (parameterType.IsByRef || parameterType.ContainsGenericParameters || parameterType == typeof(object)) + { + return true; + } + + Type clrtype = null; + using (var pyoptype = Runtime.PyObject_Type(op)) + { + Exceptions.Clear(); + if (!pyoptype.IsNull()) + { + clrtype = Converter.GetTypeByAlias(pyoptype.Borrow()); + } + } + + if (clrtype == null) + { + // Not a primitive-aliased value (e.g. a wrapped CLR object): probe the conversion. + var converted = Converter.ToManaged(op, parameterType, out _, false); + Exceptions.Clear(); + return converted; + } + + if (parameterType == clrtype) + { + return true; + } + + var pytype = Converter.GetPythonTypeByAlias(parameterType); + using (var pyoptype = Runtime.PyObject_Type(op)) + { + Exceptions.Clear(); + if (!pyoptype.IsNull() && pytype == pyoptype.Borrow()) + { + return true; + } + } + + var underlyingType = Nullable.GetUnderlyingType(parameterType) ?? parameterType; + if (Type.GetTypeCode(underlyingType) == Type.GetTypeCode(clrtype)) + { + return true; + } + + if (underlyingType == typeof(decimal) || underlyingType == typeof(double) + || (Runtime.PyFloat_Check(op) && Type.GetTypeCode(underlyingType).IsInteger() && !underlyingType.IsEnum)) + { + var converted = Converter.ToManaged(op, parameterType, out _, false); + Exceptions.Clear(); + if (converted) + { + return true; + } + } + + var opImplicit = parameterType.GetMethod("op_Implicit", new[] { clrtype }); + return opImplicit != null && opImplicit.ReturnType == parameterType; + } + protected static void AppendArgumentTypes(StringBuilder to, BorrowedReference args) { long argCount = Runtime.PyTuple_Size(args); diff --git a/src/runtime/MethodSignatureFormatter.cs b/src/runtime/MethodSignatureFormatter.cs index a382ee172..1036943de 100644 --- a/src/runtime/MethodSignatureFormatter.cs +++ b/src/runtime/MethodSignatureFormatter.cs @@ -154,7 +154,7 @@ private static bool TakesPyObject(MethodBase method) /// CLR types without a Python equivalent keep their name, with generics rendered /// as Name[Arg1, Arg2]. /// - private static string FormatType(Type type) + internal static string FormatType(Type type) { if (type.IsByRef) { diff --git a/src/testing/methodtest.cs b/src/testing/methodtest.cs index fe49de88d..94e6db1a5 100644 --- a/src/testing/methodtest.cs +++ b/src/testing/methodtest.cs @@ -729,6 +729,16 @@ public static void PointerArray(int*[] array) { } + + public static string BindDiagnosisMethod(string symbol, double quantity, bool asynchronous = false, string tag = "") + { + return "double"; + } + + public static string BindDiagnosisMethod(string symbol, int quantity, bool asynchronous = false, string tag = "") + { + return "int"; + } } diff --git a/tests/test_method.py b/tests/test_method.py index 07b5c5a34..823001159 100644 --- a/tests/test_method.py +++ b/tests/test_method.py @@ -1261,3 +1261,19 @@ def test_method_encoding(): def test_method_with_pointer_array_argument(): with pytest.raises(TypeError): MethodTest.PointerArray([0]) + + +def test_bind_failure_pinpoints_mismatched_argument(): + with pytest.raises(TypeError) as excinfo: + MethodTest.bind_diagnosis_method("SPY", -10, "exit signal") + message = str(excinfo.value) + assert message.startswith("No method matches given arguments for bind_diagnosis_method: ") + assert "The following overloads are available:" in message + assert "Argument mismatch: argument 3 ('asynchronous') expected bool, got str." in message + + +def test_bind_failure_pinpoints_mismatched_keyword_argument(): + with pytest.raises(TypeError) as excinfo: + MethodTest.bind_diagnosis_method("SPY", 10, tag=5) + message = str(excinfo.value) + assert "Argument mismatch: keyword argument 'tag' expected str, got int." in message