Browse Source

Fix constructor argument matching under interop (#1568)

Marko Lahma 2 years ago
parent
commit
dae3ec5b7c

+ 33 - 2
Jint.Tests/Runtime/ConstructorSignatureTests.cs

@@ -42,6 +42,18 @@ public class ConstructorSignature
         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();
@@ -62,8 +74,27 @@ public class ConstructorSignature
         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; }
     }
 }

+ 7 - 1
Jint/Runtime/Interop/MethodDescriptor.cs

@@ -111,7 +111,8 @@ namespace Jint.Runtime.Interop
             {
                 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;
 
@@ -119,6 +120,11 @@ namespace Jint.Runtime.Interop
                     {
                         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(

+ 2 - 2
Jint/Runtime/Interop/TypeReference.cs

@@ -131,10 +131,10 @@ namespace Jint.Runtime.Interop
                         }
 
                         // 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)
                                 {

+ 0 - 2
Jint/Runtime/Interpreter/Expressions/JintAwaitExpression.cs

@@ -1,6 +1,4 @@
 using Esprima.Ast;
-using Jint.Native;
-using Jint.Native.Object;
 using Jint.Native.Promise;
 
 namespace Jint.Runtime.Interpreter.Expressions;

+ 11 - 13
Jint/Runtime/TypeConverter.cs

@@ -1,5 +1,6 @@
 using System.Globalization;
 using System.Numerics;
+using System.Reflection;
 using System.Runtime.CompilerServices;
 using Esprima;
 using Esprima.Ast;
@@ -1142,9 +1143,8 @@ namespace Jint.Runtime
             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;
@@ -1222,12 +1222,10 @@ namespace Jint.Runtime
         /// <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)
@@ -1237,7 +1235,7 @@ namespace Jint.Runtime
 
             if (objectValue is null)
             {
-                if (!TypeIsNullable(paramType))
+                if (!parameter.IsOptional && !TypeIsNullable(paramType))
                 {
                     // this is bad
                     return -1;
@@ -1258,18 +1256,18 @@ namespace Jint.Runtime
                 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()))
@@ -1284,7 +1282,7 @@ namespace Jint.Runtime
                 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;
@@ -1315,7 +1313,7 @@ namespace Jint.Runtime
                 }
             }
 
-            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;