Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 109 additions & 0 deletions src/embed_tests/TestBindFailureDiagnosis.cs
Original file line number Diff line number Diff line change
@@ -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."));
}
}
}
230 changes: 230 additions & 0 deletions src/runtime/MethodBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/MethodSignatureFormatter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
10 changes: 10 additions & 0 deletions src/testing/methodtest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
}


Expand Down
16 changes: 16 additions & 0 deletions tests/test_method.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading