Browse Source

Support prioritizing indexers (#1669)

Marko Lahma 1 year ago
parent
commit
8015e77d6c

+ 3 - 14
Jint.Tests/Runtime/InteropTests.cs

@@ -1697,10 +1697,7 @@ namespace Jint.Tests.Runtime
         public void ShouldUseExplicitPropertyGetter()
         public void ShouldUseExplicitPropertyGetter()
         {
         {
             _engine.SetValue("c", new Company("ACME"));
             _engine.SetValue("c", new Company("ACME"));
-
-            RunTest(@"
-                assert(c.Name === 'ACME');
-            ");
+            Assert.Equal("ACME", _engine.Evaluate("c.Name"));
         }
         }
 
 
         [Fact]
         [Fact]
@@ -1709,21 +1706,14 @@ namespace Jint.Tests.Runtime
             var company = new Company("ACME");
             var company = new Company("ACME");
             ((ICompany) company)["Foo"] = "Bar";
             ((ICompany) company)["Foo"] = "Bar";
             _engine.SetValue("c", company);
             _engine.SetValue("c", company);
-
-            RunTest(@"
-                assert(c.Foo === 'Bar');
-            ");
+            Assert.Equal("Bar", _engine.Evaluate("c.Foo"));
         }
         }
 
 
         [Fact]
         [Fact]
         public void ShouldUseExplicitPropertySetter()
         public void ShouldUseExplicitPropertySetter()
         {
         {
             _engine.SetValue("c", new Company("ACME"));
             _engine.SetValue("c", new Company("ACME"));
-
-            RunTest(@"
-                c.Name = 'Foo';
-                assert(c.Name === 'Foo');
-            ");
+            Assert.Equal("Foo", _engine.Evaluate("c.Name = 'Foo'; c.Name;"));
         }
         }
 
 
         [Fact]
         [Fact]
@@ -3297,7 +3287,6 @@ try {
         public interface IStringCollection : IIndexer<string>, ICountable<string>
         public interface IStringCollection : IIndexer<string>, ICountable<string>
         {
         {
             string this[string name] { get; }
             string this[string name] { get; }
-            string this[int index] { get; }
         }
         }
 
 
         public class Strings : IStringCollection
         public class Strings : IStringCollection

+ 1 - 1
Jint/Runtime/Interop/Reflection/IndexerAccessor.cs

@@ -12,7 +12,7 @@ namespace Jint.Runtime.Interop.Reflection
     {
     {
         private readonly object _key;
         private readonly object _key;
 
 
-        private readonly PropertyInfo _indexer;
+        internal readonly PropertyInfo _indexer;
         private readonly MethodInfo? _getter;
         private readonly MethodInfo? _getter;
         private readonly MethodInfo? _setter;
         private readonly MethodInfo? _setter;
         private readonly MethodInfo? _containsKey;
         private readonly MethodInfo? _containsKey;

+ 89 - 50
Jint/Runtime/Interop/TypeResolver.cs

@@ -75,7 +75,7 @@ namespace Jint.Runtime.Interop
             string memberName,
             string memberName,
             bool forWrite)
             bool forWrite)
         {
         {
-            var isNumber = uint.TryParse(memberName, out _);
+            var isInteger = long.TryParse(memberName, out _);
 
 
             // we can always check indexer if there's one, and then fall back to properties if indexer returns null
             // we can always check indexer if there's one, and then fall back to properties if indexer returns null
             IndexerAccessor.TryFindIndexer(engine, type, memberName, out var indexerAccessor, out var indexer);
             IndexerAccessor.TryFindIndexer(engine, type, memberName, out var indexerAccessor, out var indexer);
@@ -83,7 +83,7 @@ namespace Jint.Runtime.Interop
             const BindingFlags BindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
             const BindingFlags BindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
 
 
             // properties and fields cannot be numbers
             // properties and fields cannot be numbers
-            if (!isNumber
+            if (!isInteger
                 && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp)
                 && TryFindMemberAccessor(engine, type, memberName, BindingFlags, indexer, out var temp)
                 && (!forWrite || temp.Writable))
                 && (!forWrite || temp.Writable))
             {
             {
@@ -95,84 +95,106 @@ namespace Jint.Runtime.Interop
                 return new DynamicObjectAccessor(type, memberName);
                 return new DynamicObjectAccessor(type, memberName);
             }
             }
 
 
-            // if no methods are found check if target implemented indexing
-            if (indexerAccessor != null)
-            {
-                return indexerAccessor;
-            }
-
-            // try to find a single explicit property implementation
-            List<PropertyInfo>? list = null;
             var typeResolverMemberNameComparer = MemberNameComparer;
             var typeResolverMemberNameComparer = MemberNameComparer;
             var typeResolverMemberNameCreator = MemberNameCreator;
             var typeResolverMemberNameCreator = MemberNameCreator;
-            foreach (var iface in type.GetInterfaces())
+
+            if (!isInteger)
             {
             {
-                foreach (var iprop in iface.GetProperties())
+                // try to find a single explicit property implementation
+                List<PropertyInfo>? list = null;
+                foreach (var iface in type.GetInterfaces())
                 {
                 {
-                    if (!Filter(engine, iprop))
+                    foreach (var iprop in iface.GetProperties())
                     {
                     {
-                        continue;
-                    }
+                        if (!Filter(engine, iprop))
+                        {
+                            continue;
+                        }
 
 
-                    if (iprop.Name == "Item" && iprop.GetIndexParameters().Length == 1)
-                    {
-                        // never take indexers, should use the actual indexer
-                        continue;
-                    }
+                        if (iprop.Name == "Item" && iprop.GetIndexParameters().Length == 1)
+                        {
+                            // never take indexers, should use the actual indexer
+                            continue;
+                        }
 
 
-                    foreach (var name in typeResolverMemberNameCreator(iprop))
-                    {
-                        if (typeResolverMemberNameComparer.Equals(name, memberName))
+                        foreach (var name in typeResolverMemberNameCreator(iprop))
                         {
                         {
-                            list ??= new List<PropertyInfo>();
-                            list.Add(iprop);
+                            if (typeResolverMemberNameComparer.Equals(name, memberName))
+                            {
+                                list ??= new List<PropertyInfo>();
+                                list.Add(iprop);
+                            }
                         }
                         }
                     }
                     }
                 }
                 }
-            }
 
 
-            if (list?.Count == 1)
-            {
-                return new PropertyAccessor(memberName, list[0]);
-            }
+                if (list?.Count == 1)
+                {
+                    return new PropertyAccessor(memberName, list[0]);
+                }
 
 
-            // try to find explicit method implementations
-            List<MethodInfo>? explicitMethods = null;
-            foreach (var iface in type.GetInterfaces())
-            {
-                foreach (var imethod in iface.GetMethods())
+                // try to find explicit method implementations
+                List<MethodInfo>? explicitMethods = null;
+                foreach (var iface in type.GetInterfaces())
                 {
                 {
-                    if (!Filter(engine, imethod))
+                    foreach (var imethod in iface.GetMethods())
                     {
                     {
-                        continue;
-                    }
+                        if (!Filter(engine, imethod))
+                        {
+                            continue;
+                        }
 
 
-                    foreach (var name in typeResolverMemberNameCreator(imethod))
-                    {
-                        if (typeResolverMemberNameComparer.Equals(name, memberName))
+                        foreach (var name in typeResolverMemberNameCreator(imethod))
                         {
                         {
-                            explicitMethods ??= new List<MethodInfo>();
-                            explicitMethods.Add(imethod);
+                            if (typeResolverMemberNameComparer.Equals(name, memberName))
+                            {
+                                explicitMethods ??= new List<MethodInfo>();
+                                explicitMethods.Add(imethod);
+                            }
                         }
                         }
                     }
                     }
                 }
                 }
+
+                if (explicitMethods?.Count > 0)
+                {
+                    return new MethodAccessor(type, memberName, MethodDescriptor.Build(explicitMethods));
+                }
             }
             }
 
 
-            if (explicitMethods?.Count > 0)
+            // if no methods are found check if target implemented indexing
+            var score = int.MaxValue;
+            if (indexerAccessor != null)
             {
             {
-                return new MethodAccessor(type, memberName, MethodDescriptor.Build(explicitMethods));
+                var parameter = indexerAccessor._indexer.GetIndexParameters()[0];
+                score = CalculateIndexerScore(parameter, isInteger);
             }
             }
 
 
-            // try to find explicit indexer implementations
-            foreach (var interfaceType in type.GetInterfaces())
+            if (score != 0)
             {
             {
-                if (IndexerAccessor.TryFindIndexer(engine, interfaceType, memberName, out var accessor, out _))
+                // try to find explicit indexer implementations that has a better score than earlier
+                foreach (var interfaceType in type.GetInterfaces())
                 {
                 {
-                    return accessor;
+                    if (IndexerAccessor.TryFindIndexer(engine, interfaceType, memberName, out var accessor, out _))
+                    {
+                        var parameter = accessor._indexer.GetIndexParameters()[0];
+                        var newScore = CalculateIndexerScore(parameter, isInteger);
+                        if (newScore < score)
+                        {
+                            // found a better one
+                            indexerAccessor = accessor;
+                            score = newScore;
+                        }
+                    }
                 }
                 }
             }
             }
 
 
-            if (engine._extensionMethods.TryGetExtensionMethods(type, out var extensionMethods))
+            // use the best indexer we were able to find
+            if (indexerAccessor != null)
+            {
+                return indexerAccessor;
+            }
+
+            if (!isInteger && engine._extensionMethods.TryGetExtensionMethods(type, out var extensionMethods))
             {
             {
                 var matches = new List<MethodInfo>();
                 var matches = new List<MethodInfo>();
                 foreach (var method in extensionMethods)
                 foreach (var method in extensionMethods)
@@ -200,6 +222,23 @@ namespace Jint.Runtime.Interop
             return ConstantValueAccessor.NullAccessor;
             return ConstantValueAccessor.NullAccessor;
         }
         }
 
 
+        private static int CalculateIndexerScore(ParameterInfo parameter, bool isInteger)
+        {
+            var paramType = parameter.ParameterType;
+
+            if (paramType == typeof(int))
+            {
+                return  isInteger ? 0 : 10;
+            }
+
+            if (paramType == typeof(string))
+            {
+                return 1;
+            }
+
+            return 5;
+        }
+
         internal bool TryFindMemberAccessor(
         internal bool TryFindMemberAccessor(
             Engine engine,
             Engine engine,
             Type type,
             Type type,