Browse Source

Improve generic extension methods and Linq interop (#896)

* add linq extension method tests
* correctly detect extension methods with generic type parameters
* remove code which was trying to solve generic type constraints
* small cleanup
Gökhan Kurt 4 years ago
parent
commit
50b723254f

+ 48 - 0
Jint.Tests/Runtime/ExtensionMethods/ExtensionMethodsTest.cs

@@ -1,5 +1,8 @@
 using Jint.Native;
 using Jint.Tests.Runtime.Domain;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using Xunit;
 
 namespace Jint.Tests.Runtime.ExtensionMethods
@@ -109,5 +112,50 @@ namespace Jint.Tests.Runtime.ExtensionMethods
             var propertyValue = engine.Execute("person.bogus").GetCompletionValue();
             Assert.Equal(JsValue.Undefined, propertyValue);
         }
+
+        private Engine GetLinqEngine()
+        {
+            return new Engine(opts =>
+            {
+                opts.AddExtensionMethods(typeof(Enumerable));
+            });
+        }
+
+        [Fact]
+        public void LinqExtensionMethodWithoutGenericParameter()
+        {
+            var engine = GetLinqEngine();
+            var intList = new List<int>() { 0, 1, 2, 3 };
+
+            engine.SetValue("intList", intList);
+            var intSumRes = engine.Execute("intList.Sum()").GetCompletionValue().AsNumber();
+            Assert.Equal(6, intSumRes);
+        }
+
+        [Fact]
+        public void LinqExtensionMethodWithSingleGenericParameter()
+        {
+            var engine = GetLinqEngine();
+            var stringList = new List<string>() { "working", "linq" };
+            engine.SetValue("stringList", stringList);
+
+            var stringSumRes = engine.Execute("stringList.Sum(x => x.length)").GetCompletionValue().AsNumber();
+            Assert.Equal(11, stringSumRes);
+        }
+
+        [Fact]
+        public void LinqExtensionMethodWithMultipleGenericParameters()
+        {
+            var engine = GetLinqEngine();
+            var stringList = new List<string>() { "working", "linq" };
+            engine.SetValue("stringList", stringList);
+
+            var stringRes = engine.Execute("stringList.Select((x) => x + 'a').ToArray().join()").GetCompletionValue().AsString();
+            Assert.Equal("workinga,linqa", stringRes);
+
+            // The method ambiguity resolver is not so smart to choose the Select method with the correct number of parameters
+            // Thus, the following script will not work as expected.
+            // stringList.Select((x, i) => x + i).ToArray().join()
+        }
     }
 }

+ 42 - 4
Jint/Runtime/Interop/Reflection/ExtensionMethodCache.cs

@@ -24,12 +24,18 @@ namespace Jint.Runtime.Interop.Reflection
         {
             _allExtensionMethods = extensionMethods;
         }
-		
+
         internal static ExtensionMethodCache Build(List<Type> extensionMethodContainerTypes)
         {
+            Type GetTypeDefinition(Type type)
+            {
+                return type.IsConstructedGenericType && type.GenericTypeArguments.Any(x => x.IsGenericParameter) ?
+                    type.GetGenericTypeDefinition() : type;
+            }
+
             var methodsByTarget = extensionMethodContainerTypes
                 .SelectMany(x => x.GetExtensionMethods())
-                .GroupBy(x => x.GetParameters()[0].ParameterType)
+                .GroupBy(x => GetTypeDefinition(x.GetParameters()[0].ParameterType))
                 .ToDictionary(x => x.Key, x => x.ToArray());
 
             return new ExtensionMethodCache(methodsByTarget);
@@ -37,6 +43,27 @@ namespace Jint.Runtime.Interop.Reflection
 
         public bool HasMethods => _allExtensionMethods.Count > 0;
 
+        private MethodInfo BindMethodGenericParameters(MethodInfo method)
+        {
+            if (method.IsGenericMethodDefinition && method.ContainsGenericParameters)
+            {
+                var methodGenerics = method.GetGenericArguments();
+                var parameterList = Enumerable.Repeat(typeof(object), methodGenerics.Length).ToArray();
+
+                try
+                {
+                    return method.MakeGenericMethod(parameterList);
+                }
+                catch
+                {
+                    // Generic parameter constraints failed probably.
+                    // If it does not work, let it be. We don't need to do anything.
+                }
+            }
+            return method;
+        }
+
+
         public bool TryGetExtensionMethods(Type objectType, out MethodInfo[] methods)
         {
             var methodLookup = _extensionMethods;
@@ -60,7 +87,7 @@ namespace Jint.Runtime.Interop.Reflection
                 }
             }
 
-            methods = results.ToArray();
+            methods = results.Select(BindMethodGenericParameters).ToArray();
 
             // racy, we don't care, worst case we'll catch up later
             Interlocked.CompareExchange(ref _extensionMethods, new Dictionary<Type, MethodInfo[]>(methodLookup)
@@ -83,6 +110,11 @@ namespace Jint.Runtime.Interop.Reflection
             foreach (var i in type.GetInterfaces())
             {
                 yield return i;
+
+                if (i.IsConstructedGenericType)
+                {
+                    yield return i.GetGenericTypeDefinition();
+                }
             }
 
             // return all inherited types
@@ -90,8 +122,14 @@ namespace Jint.Runtime.Interop.Reflection
             while (currentBaseType != null)
             {
                 yield return currentBaseType;
+
+                if (currentBaseType.IsConstructedGenericType)
+                {
+                    yield return currentBaseType.GetGenericTypeDefinition();
+                }
+
                 currentBaseType = currentBaseType.BaseType;
             }
         }
     }
-}
+}