Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix constructor argument matching under interop #1568

Merged
merged 1 commit into from
Jul 1, 2023
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
35 changes: 33 additions & 2 deletions Jint.Tests/Runtime/ConstructorSignatureTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ public void CorrectOverloadShouldBeSelected()
Assert.Equal("A-30", engine.Evaluate("new B('A', 30).Result"));
}


[Fact]
public void CanConstructWithMixedFloatAndEnumConstructorParameters()
{
var engine = new Engine();
engine.SetValue("Length", TypeReference.CreateTypeReference<Length>(engine));
Assert.Equal(12.3, engine.Evaluate("new Length(12.3).Value").AsNumber(), precision: 2);
Assert.Equal(12.3, engine.Evaluate("new Length(12.3, 0).Value").AsNumber(), precision: 2);
Assert.Equal(0, engine.Evaluate("new Length(12.3, 0).UnitValue").AsInteger());
Assert.Equal(LengthUnit.Pixel, (LengthUnit) engine.Evaluate("new Length(12.3, 42).UnitValue").AsInteger());
}

private class A
{
public A(int param1, int param2 = 5) => Result = (param1 + param2).ToString();
Expand All @@ -62,8 +74,27 @@ public B(string param1, float param2, string param3)
public B(string param1, float param2)
{
Result = string.Join("-", param1, param2.ToString(CultureInfo.InvariantCulture));
}
}

public string Result { get; }
}

private enum LengthUnit { Pixel = 42 }

private class Length
{
public Length(float value)
: this(value, LengthUnit.Pixel)
{
}

public Length(float value, LengthUnit unit)
{
Value = value;
UnitValue = unit;
}

public string Result { get;}
public float Value { get; }
public LengthUnit UnitValue { get; }
}
}
8 changes: 7 additions & 1 deletion Jint/Runtime/Interop/MethodDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,20 @@ public JsValue Call(Engine engine, object? instance, JsValue[] arguments)
{
for (var i = 0; i < arguments.Length; i++)
{
var parameterType = methodParameters[i].ParameterType;
var methodParameter = methodParameters[i];
var parameterType = methodParameter.ParameterType;
var value = arguments[i];
object? converted;

if (typeof(JsValue).IsAssignableFrom(parameterType))
{
converted = value;
}
else if (value.IsUndefined() && methodParameter.IsOptional)
{
// undefined is considered missing, null is consider explicit value
converted = methodParameter.DefaultValue;
}
else if (!ReflectionExtensions.TryConvertViaTypeCoercion(parameterType, valueCoercionType, value, out converted))
{
converted = engine.ClrTypeConverter.Convert(
Expand Down
4 changes: 2 additions & 2 deletions Jint/Runtime/Interop/TypeReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,10 @@ static ObjectInstance ObjectCreator(Engine engine, Realm realm, ObjectCreateStat
}

// optional parameters
if (parameters.Length >= arguments.Length)
if (parameters.Length > arguments.Length)
{
// all missing ones must be optional
foreach (var parameter in parameters.AsSpan(parameters.Length - arguments.Length + 1))
foreach (var parameter in parameters.AsSpan(parameters.Length - arguments.Length))
{
if (!parameter.IsOptional)
{
Expand Down
2 changes: 0 additions & 2 deletions Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using Esprima.Ast;
using Jint.Native;
using Jint.Native.Object;
using Jint.Native.Promise;

namespace Jint.Runtime.Interpreter.Expressions;
Expand Down
24 changes: 11 additions & 13 deletions Jint/Runtime/TypeConverter.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Globalization;
using System.Numerics;
using System.Reflection;
using System.Runtime.CompilerServices;
using Esprima;
using Esprima.Ast;
Expand Down Expand Up @@ -1142,9 +1143,8 @@ private static int CalculateMethodScore(Engine engine, MethodDescriptor method,
for (var i = 0; i < arguments.Length; i++)
{
var jsValue = arguments[i];
var paramType = method.Parameters[i].ParameterType;

var parameterScore = CalculateMethodParameterScore(engine, jsValue, paramType);
var parameterScore = CalculateMethodParameterScore(engine, method.Parameters[i], jsValue);
if (parameterScore < 0)
{
return parameterScore;
Expand Down Expand Up @@ -1222,12 +1222,10 @@ internal static AssignableResult IsAssignableToGenericType(Type givenType, Type
/// <summary>
/// Determines how well parameter type matches target method's type.
/// </summary>
private static int CalculateMethodParameterScore(
Engine engine,
JsValue jsValue,
Type paramType)
private static int CalculateMethodParameterScore(Engine engine, ParameterInfo parameter, JsValue parameterValue)
{
var objectValue = jsValue.ToObject();
var paramType = parameter.ParameterType;
var objectValue = parameterValue.ToObject();
var objectValueType = objectValue?.GetType();

if (objectValueType == paramType)
Expand All @@ -1237,7 +1235,7 @@ private static int CalculateMethodParameterScore(

if (objectValue is null)
{
if (!TypeIsNullable(paramType))
if (!parameter.IsOptional && !TypeIsNullable(paramType))
{
// this is bad
return -1;
Expand All @@ -1258,18 +1256,18 @@ private static int CalculateMethodParameterScore(
return 5;
}

if (paramType == typeof(int) && jsValue.IsInteger())
if (paramType == typeof(int) && parameterValue.IsInteger())
{
return 0;
}

if (paramType == typeof(float) && objectValueType == typeof(double))
{
return jsValue.IsInteger() ? 1 : 2;
return parameterValue.IsInteger() ? 1 : 2;
}

if (paramType.IsEnum &&
jsValue is JsNumber jsNumber
parameterValue is JsNumber jsNumber
&& jsNumber.IsInteger()
&& paramType.GetEnumUnderlyingType() == typeof(int)
&& Enum.IsDefined(paramType, jsNumber.AsInteger()))
Expand All @@ -1284,7 +1282,7 @@ jsValue is JsNumber jsNumber
return 1;
}

if (jsValue.IsArray() && paramType.IsArray)
if (parameterValue.IsArray() && paramType.IsArray)
{
// we have potential, TODO if we'd know JS array's internal type we could have exact match
return 2;
Expand Down Expand Up @@ -1315,7 +1313,7 @@ jsValue is JsNumber jsNumber
}
}

if (ReflectionExtensions.TryConvertViaTypeCoercion(paramType, engine.Options.Interop.ValueCoercion, jsValue, out _))
if (ReflectionExtensions.TryConvertViaTypeCoercion(paramType, engine.Options.Interop.ValueCoercion, parameterValue, out _))
{
// gray JS zone where we start to do odd things
return 10;
Expand Down