Account for integer indexers when searching for IndexerAccessor (#1662)

sebastienros · Oct 28, 2023 · 2ae0dc2 · 2ae0dc2
1 parent 0242ac5
commit 2ae0dc2
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 14 deletions.
diff --git a/Jint.Tests/Runtime/InteropTests.cs b/Jint.Tests/Runtime/InteropTests.cs
@@ -3297,6 +3297,7 @@ public interface ICountable<out T>
         public interface IStringCollection : IIndexer<string>, ICountable<string>
         {
             string this[string name] { get; }
+            string this[int index] { get; }
         }
 
         public class Strings : IStringCollection
@@ -3306,14 +3307,8 @@ public Strings(string[] strings)
             {
                 _strings = strings;
             }
-            public string this[string name]
-            {
-                get
-                {
-                    return int.TryParse(name, out var index) ? _strings[index] : _strings.FirstOrDefault(x => x.Contains(name));
-                }
-            }
 
+            public string this[string name] => null;
             public string this[int index] => _strings[index];
             public int Count => _strings.Length;
         }
@@ -3332,6 +3327,15 @@ public void AccessingInterfaceShouldContainExtendedInterfaces()
             Assert.Equal(3, result);
         }
 
+        [Fact]
+        public void IntegerIndexerIfPreferredOverStringIndexerWhenFound()
+        {
+            var engine = new Engine();
+            engine.SetValue("Utils", new Utils());
+            var result = engine.Evaluate("const strings = Utils.GetStrings(); strings[2];");
+            Assert.Equal("c", result);
+        }
+
         [Fact]
         public void CanDestructureInteropTargetMethod()
         {

diff --git a/Jint/Runtime/Interop/Reflection/IndexerAccessor.cs b/Jint/Runtime/Interop/Reflection/IndexerAccessor.cs
@@ -37,18 +37,25 @@ internal static bool TryFindIndexer(
             [NotNullWhen(true)] out IndexerAccessor? indexerAccessor,
             [NotNullWhen(true)] out PropertyInfo? indexer)
         {
+            indexerAccessor = null;
+            indexer = null;
             var paramTypeArray = new Type[1];
 
+            // integer keys can be ambiguous as we only know string keys
+            int? integerKey = null;
+
+            if (int.TryParse(propertyName, out var intKeyTemp))
+            {
+                integerKey = intKeyTemp;
+            }
+
             IndexerAccessor? ComposeIndexerFactory(PropertyInfo candidate, Type paramType)
             {
                 object? key = null;
                 // int key is quite common case
-                if (paramType == typeof(int))
+                if (paramType == typeof(int) && integerKey is not null)
                 {
-                    if (int.TryParse(propertyName, out var intValue))
-                    {
-                        key = intValue;
-                    }
+                    key = integerKey;
                 }
                 else
                 {
@@ -89,6 +96,7 @@ internal static bool TryFindIndexer(
             }
 
             // try to find first indexer having either public getter or setter with matching argument type
+            PropertyInfo? fallbackIndexer = null;
             foreach (var candidate in targetType.GetProperties())
             {
                 if (!filter(candidate))
@@ -108,12 +116,30 @@ internal static bool TryFindIndexer(
                     indexerAccessor = ComposeIndexerFactory(candidate, paramType);
                     if (indexerAccessor != null)
                     {
-                        indexer = candidate;
-                        return true;
+                        if (paramType != typeof(string) ||  integerKey is null)
+                        {
+                            // exact match, we don't need to check for integer key
+                            indexer = candidate;
+                            return true;
+                        }
+
+                        if (fallbackIndexer is null)
+                        {
+                            // our fallback
+                            fallbackIndexer = candidate;
+                        }
                     }
                 }
             }
 
+            if (fallbackIndexer is not null)
+            {
+                indexer = fallbackIndexer;
+                // just to keep compiler happy, we know we have a value
+                indexerAccessor = indexerAccessor ?? new IndexerAccessor(indexer, null, null!);
+                return true;
+            }
+
             indexerAccessor = default;
             indexer = default;
             return false;