Przeglądaj źródła

Fix default parameter handling under interop (#2134)

* use state for best match resolving
Marko Lahma 1 miesiąc temu
rodzic
commit
d6e1a8354f

+ 19 - 1
Jint.Tests/Runtime/ConstructorSignatureTests.cs

@@ -3,7 +3,7 @@ using Jint.Runtime.Interop;
 
 namespace Jint.Tests.Runtime;
 
-public class ConstructorSignature
+public class ConstructorSignatureTests
 {
     [Fact]
     public void OptionalConstructorParametersShouldBeSupported()
@@ -54,6 +54,17 @@ public class ConstructorSignature
         Assert.Equal(LengthUnit.Pixel, (LengthUnit) engine.Evaluate("new Length(12.3, 42).UnitValue").AsInteger());
     }
 
+    [Fact]
+    public void ShouldBeAbleToIgnoreDefaultParameters()
+    {
+        var engine = new Engine();
+        engine.SetValue("C", TypeReference.CreateTypeReference(engine, typeof(C)));
+
+        // Should not throw an error
+        engine.Evaluate("new C(1, 2)");
+        engine.Evaluate("new C(1, 2, 'context')");
+    }
+
     private class A
     {
         public A(int param1, int param2 = 5) => Result = (param1 + param2).ToString();
@@ -97,4 +108,11 @@ public class ConstructorSignature
         public float Value { get; }
         public LengthUnit UnitValue { get; }
     }
+
+    private class C
+    {
+        public C(int tileCoordsX, int tileCoordsY, string context = null)
+        {
+        }
+    }
 }

+ 4 - 3
Jint/Runtime/Interop/InteropHelper.cs

@@ -296,16 +296,17 @@ internal sealed class InteropHelper
         public int CompareTo(MethodMatch other) => Score.CompareTo(other.Score);
     }
 
-    internal static IEnumerable<MethodMatch> FindBestMatch(
+    internal static IEnumerable<MethodMatch> FindBestMatch<TState>(
         Engine engine,
         MethodDescriptor[] methods,
-        Func<MethodDescriptor, JsValue[]> argumentProvider)
+        Func<MethodDescriptor, TState, JsValue[]> argumentProvider,
+        TState state)
     {
         List<MethodMatch>? matchingByParameterCount = null;
         foreach (var method in methods)
         {
             var parameterInfos = method.Parameters;
-            var arguments = argumentProvider(method);
+            var arguments = argumentProvider(method, state);
             if (arguments.Length <= parameterInfos.Length
                 && arguments.Length >= parameterInfos.Length - method.ParameterDefaultValuesCount)
             {

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

@@ -124,7 +124,7 @@ internal sealed class MethodDescriptor
                 }
                 else if (value.IsUndefined() && methodParameter.IsOptional)
                 {
-                    // undefined is considered missing, null is consider explicit value
+                    // undefined is considered missing, null is considered explicit value
                     converted = methodParameter.DefaultValue;
                 }
                 else if (!ReflectionExtensions.TryConvertViaTypeCoercion(parameterType, valueCoercionType, value, out converted))
@@ -138,20 +138,13 @@ internal sealed class MethodDescriptor
                 parameters[i] = converted;
             }
 
-            if (Method is MethodInfo m)
+            var retVal = Method switch
             {
-                var retVal = m.Invoke(instance, parameters);
-                return JsValue.FromObject(engine, retVal);
-            }
-            else if (Method is ConstructorInfo c)
-            {
-                var retVal = c.Invoke(parameters);
-                return JsValue.FromObject(engine, retVal);
-            }
-            else
-            {
-                throw new NotSupportedException("Method is unknown type");
-            }
+                MethodInfo m => m.Invoke(instance, parameters),
+                ConstructorInfo c => c.Invoke(parameters),
+                _ => throw new NotSupportedException("Method is unknown type"),
+            };
+            return JsValue.FromObject(engine, retVal);
         }
         catch (TargetInvocationException exception)
         {

+ 17 - 14
Jint/Runtime/Interop/MethodInfoFunction.cs

@@ -143,29 +143,32 @@ internal sealed class MethodInfoFunction : Function
         return genericMethodInfo;
     }
 
+    private readonly record struct MethodResolverState(Engine Engine, JsValue This, JsCallArguments Arguments);
+
     protected internal override JsValue Call(JsValue thisObject, JsCallArguments jsArguments)
     {
-        JsValue[] ArgumentProvider(MethodDescriptor method)
+        static JsCallArguments ArgumentProvider(MethodDescriptor method, MethodResolverState state)
         {
             if (method.IsExtensionMethod)
             {
-                var jsArgumentsTemp = new JsValue[1 + jsArguments.Length];
-                jsArgumentsTemp[0] = thisObject;
-                Array.Copy(jsArguments, 0, jsArgumentsTemp, 1, jsArguments.Length);
+                var jsArgumentsTemp = new JsValue[1 + state.Arguments.Length];
+                jsArgumentsTemp[0] = state.This;
+                Array.Copy(state.Arguments, 0, jsArgumentsTemp, 1, state.Arguments.Length);
                 return method.HasParams
-                    ? ProcessParamsArrays(jsArgumentsTemp, method)
+                    ? ProcessParamsArrays(state.Engine, method, jsArgumentsTemp)
                     : jsArgumentsTemp;
             }
 
             return method.HasParams
-                ? ProcessParamsArrays(jsArguments, method)
-                : jsArguments;
+                ? ProcessParamsArrays(state.Engine, method, state.Arguments)
+                : state.Arguments;
         }
 
         var converter = Engine.TypeConverter;
         var thisObj = thisObject.ToObject() ?? _target;
         object?[]? parameters = null;
-        foreach (var (method, arguments, _) in InteropHelper.FindBestMatch(_engine, _methods, ArgumentProvider))
+        var state = new MethodResolverState(_engine, thisObject, jsArguments);
+        foreach (var (method, arguments, _) in InteropHelper.FindBestMatch(_engine, _methods, ArgumentProvider, state))
         {
             var methodParameters = method.Parameters;
             if (parameters == null || parameters.Length != methodParameters.Length)
@@ -259,7 +262,7 @@ internal sealed class MethodInfoFunction : Function
     /// <summary>
     /// Reduces a flat list of parameters to a params array, if needed
     /// </summary>
-    private JsValue[] ProcessParamsArrays(JsCallArguments arguments, MethodDescriptor methodInfo)
+    private static JsCallArguments ProcessParamsArrays(Engine engine, MethodDescriptor methodInfo, JsCallArguments arguments)
     {
         var parameters = methodInfo.Parameters;
 
@@ -276,15 +279,15 @@ internal sealed class MethodInfoFunction : Function
             return arguments;
         }
 
-        var jsArray = new JsArray(_engine, argsToTransform);
-        var newArgumentsCollection = new JsValue[nonParamsArgumentsCount + 1];
+        var array = new JsArray(engine, argsToTransform);
+        var newArguments = new JsValue[nonParamsArgumentsCount + 1];
         for (var j = 0; j < nonParamsArgumentsCount; ++j)
         {
-            newArgumentsCollection[j] = arguments[j];
+            newArguments[j] = arguments[j];
         }
 
-        newArgumentsCollection[nonParamsArgumentsCount] = jsArray;
-        return newArgumentsCollection;
+        newArguments[nonParamsArgumentsCount] = array;
+        return newArguments;
     }
 
     public override string ToString()

+ 9 - 5
Jint/Runtime/Interop/TypeReference.cs

@@ -56,6 +56,8 @@ public sealed class TypeReference : Constructor, IObjectWrapper
         return Construct(arguments, Undefined);
     }
 
+    private readonly record struct MethodResolverState(Engine Engine, JsCallArguments Arguments);
+
     public override ObjectInstance Construct(JsCallArguments arguments, JsValue newTarget)
     {
         static ObjectInstance ObjectCreator(Engine engine, Realm realm, ObjectCreateState state)
@@ -80,8 +82,10 @@ public sealed class TypeReference : Constructor, IObjectWrapper
                     referenceType,
                     t => MethodDescriptor.Build(t.GetConstructors(BindingFlags.Public | BindingFlags.Instance)));
 
-                var argumentProvider = new Func<MethodDescriptor, JsValue[]>(method =>
+                Func<MethodDescriptor, MethodResolverState, JsCallArguments> argumentProvider = static (method, state) =>
                 {
+                    var engine = state.Engine;
+                    var arguments = state.Arguments;
                     var parameters = method.Parameters;
 
                     if (parameters.Length == 0)
@@ -141,8 +145,7 @@ public sealed class TypeReference : Constructor, IObjectWrapper
                     if (parameters.Length > arguments.Length)
                     {
                         // all missing ones must be optional
-                        int start = parameters.Length - arguments.Length;
-                        for (var i = start; i < parameters.Length; i++)
+                        for (var i = arguments.Length; i < parameters.Length; i++)
                         {
                             if (!parameters[i].IsOptional)
                             {
@@ -175,9 +178,10 @@ public sealed class TypeReference : Constructor, IObjectWrapper
                     }
 
                     return arguments;
-                });
+                };
 
-                foreach (var (method, methodArguments, _) in InteropHelper.FindBestMatch(engine, constructors, argumentProvider))
+                var resolverState = new MethodResolverState(engine, arguments);
+                foreach (var (method, methodArguments, _) in InteropHelper.FindBestMatch(engine, constructors, argumentProvider, resolverState))
                 {
                     var retVal = method.Call(engine, null, methodArguments);
                     result = TypeConverter.ToObject(realm, retVal);

+ 3 - 1
Jint/Runtime/Interpreter/Expressions/JintBinaryExpression.cs

@@ -40,6 +40,8 @@ internal abstract class JintBinaryExpression : JintExpression
         _initialized = true;
     }
 
+    private readonly record struct MethodResolverState(JsCallArguments Arguments);
+
     internal static bool TryOperatorOverloading(
         EvaluationContext context,
         JsValue leftValue,
@@ -65,7 +67,7 @@ internal abstract class JintBinaryExpression : JintExpression
                 var methods = leftMethods.Concat(rightMethods).Where(x => string.Equals(x.Name, clrName, StringComparison.Ordinal) && x.GetParameters().Length == 2);
                 var methodDescriptors = MethodDescriptor.Build(methods.ToArray());
 
-                return InteropHelper.FindBestMatch(context.Engine, methodDescriptors, _ => arguments).FirstOrDefault().Method;
+                return InteropHelper.FindBestMatch(context.Engine, methodDescriptors, static (_, state) => state.Arguments, new MethodResolverState(arguments)).FirstOrDefault().Method;
             });
 
             if (method != null)