Pinpoint the first mismatched argument in bind-failure TypeErrors (#145)

* Pinpoint the first mismatched argument in bind-failure TypeErrors

When no overload matches a call, the TypeError now appends a diagnosis of
the first argument that fails to match the nearest overload (the one with
the most leading convertible arguments), e.g.:

  Argument mismatch: argument 3 ('asynchronous') expected bool, got str.

Keyword arguments whose values cannot convert to the matching parameter
are diagnosed too. The line is appended after the overloads hint so
consumers that extract the hint from its marker onwards keep it.

* Reuse Runtime.PyObject_GetTypeName in the bind-failure diagnosis

Drops the duplicated type-name helper in favor of the existing runtime
one, and tightens the new comments.

Author: jhonabreul

src/embed_tests/TestBindFailureDiagnosis.cs

@@ -0,0 +1,109 @@
+using NUnit.Framework;
+using Python.Runtime;
+
+namespace Python.EmbeddingTest
+{
+    /// <summary>
+    /// The bind-failure TypeError must pinpoint the first argument that fails to
+    /// match the nearest overload.
+    /// </summary>
+    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<string>();
+        }
+
+        [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."));
+        }
+    }
+}

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
             }
         }
 
+        /// <summary>
+        /// 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.
+        /// </summary>
+        private static string DiagnoseClosestOverloadMismatch(IEnumerable<MethodBase> 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<KeyValuePair<string, PyObject>> kwargs = null;
+                if (kw != null && Runtime.PyDict_Size(kw) > 0)
+                {
+                    kwargs = new List<KeyValuePair<string, PyObject>>();
+                    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<string, PyObject>(
+                                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();
+            }
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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);

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 <c>Name[Arg1, Arg2]</c>.
         /// </summary>
-        private static string FormatType(Type type)
+        internal static string FormatType(Type type)
         {
             if (type.IsByRef)
             {

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";
+        }
     }
 
 

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