Skip to content

Commit 1f88aad

Browse files
BionicCodeBionicCode
committed
Improve type safety and error handling for indexer access
Refactored DelegateProvider and PropertyData to enhance type safety, correct parameter handling, and improve error messages for reflection-based property and indexer access. Fixed delegate caching bugs, added stricter null and type checks, and ensured correct handling of indexer parameter arrays, especially for multi-parameter indexers. These changes increase robustness and maintainability.
1 parent 8754777 commit 1f88aad

2 files changed

Lines changed: 71 additions & 27 deletions

File tree

‎src/BionicCode.Utilities.Reflection/DelegateProvider.cs‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
using System;
44
using System.Collections.Concurrent;
5-
using System.Collections.Immutable;
65
using System.Linq;
76
using System.Linq.Expressions;
87
using System.Reflection;
@@ -73,7 +72,7 @@
7372

7473
internal static class DelegateProvider
7574
{
76-
private static readonly ConcurrentDictionary<MethodDataGenericTypeVariantKey, SymbolReflectionInfoCacheKeyInternal> InvocatorKeyMap = new();
75+
private static readonly ConcurrentDictionary<MethodDataGenericTypeVariantKey, SymbolReflectionInfoCacheKey> s_invocatorKeyMap = new();
7776

7877
/// <summary>
7978
/// Gets an invocable MethodData instance for the specified method, generating a fast delegate-based invoker if
@@ -507,7 +506,7 @@ private static MethodData GetOrConstructGenericMethod(RuntimeTypeHandle targetTy
507506

508507
// Try get cached constructed invocator for the specified generic method parameters.
509508
var invocatorKeyMapKey = new MethodDataGenericTypeVariantKey(genericMethodArguments, desiredReturnTypeHandle, targetTypeHandle, targetMethodData.BasicMethodFingerprint);
510-
if (DelegateProvider.InvocatorKeyMap.TryGetValue(invocatorKeyMapKey, out SymbolReflectionInfoCacheKeyInternal symbolInfoCacheKey)
509+
if (DelegateProvider.s_invocatorKeyMap.TryGetValue(invocatorKeyMapKey, out SymbolReflectionInfoCacheKeyInternal symbolInfoCacheKey)
511510
&& SymbolReflectionInfoCache.TryGetSymbolInfoDataCacheEntry(symbolInfoCacheKey, out MethodData? cachedMethodData))
512511
{
513512
methodData = cachedMethodData!;
@@ -524,7 +523,7 @@ private static MethodData CloseOpenGenericMethodAndAddToCache(MethodData targetM
524523
{
525524
MethodData methodData;
526525
MethodData closedGenericMethodData = targetMethodData.MakeGenericMethodData(genericMethodParameters);
527-
_ = DelegateProvider.InvocatorKeyMap.TryAdd(invocatorKeyMapKey, closedGenericMethodData.CacheKey);
526+
_ = DelegateProvider.s_invocatorKeyMap.TryAdd(invocatorKeyMapKey, closedGenericMethodData.View.CacheKey);
528527
methodData = closedGenericMethodData;
529528
return methodData;
530529
}
@@ -537,6 +536,8 @@ private static MethodData CloseOpenGenericMethodAndAddToCache(MethodData targetM
537536
/// <returns>A MethodData instance representing a fast invoker for the specified event.</returns>
538537
public static MethodData GetOrCreateFastEventInvoker(EventData eventData)
539538
{
539+
ArgumentNullException.ThrowIfNull(eventData);
540+
540541
MethodData targetMethodData = eventData.EventInvokerMethodData;
541542

542543
return GetOrCreateFastMethodInvoker(targetMethodData!, TypeList.Empty);
@@ -676,7 +677,7 @@ public static ValueTypeMemberSetter<TTarget, TValue> CreateStructSetter<TTarget,
676677
valueType,
677678
nameof(TValue),
678679
fieldType,
679-
"field prpertyType"));
680+
"field propertyType"));
680681

681682
if (fieldData.IsConst)
682683
{
@@ -715,6 +716,8 @@ public static ValueTypeMemberSetter<TTarget, TValue> CreateStructSetter<TTarget,
715716
/// <exception cref="ArgumentException">Thrown if the specified property is an indexer or is write-only.</exception>
716717
public static Func<object?, object?> CreateGetter(PropertyData propertyData)
717718
{
719+
ArgumentNullException.ThrowIfNull(propertyData, nameof(propertyData));
720+
718721
if (propertyData is IPropertyDataInvoker propertyDataInvoker && propertyDataInvoker.HasGetter)
719722
{
720723
throw new InvalidOperationException("The 'PropertyData' has already the get invoker generated.");
@@ -724,7 +727,6 @@ public static ValueTypeMemberSetter<TTarget, TValue> CreateStructSetter<TTarget,
724727
propertyData.IsIndexer,
725728
nameof(propertyData),
726729
"The provided property is an indexer. Use the appropriate indexer getter creation method instead.");
727-
ArgumentNullException.ThrowIfNull(propertyData, nameof(propertyData));
728730
ArgumentExceptionAdvanced.ThrowIfFalse(
729731
propertyData.CanRead,
730732
nameof(propertyData),
@@ -976,7 +978,7 @@ public static IndexerPropertyGetter<TTarget, TValue, TIndex> CreateIndexerGetter
976978
// Use provided argument type and cast to indexer parameter type if needed.
977979
castedIndexParam = indexType != indexParameterType
978980
? Expression.Convert(indexAccess, indexParameterType)
979-
: indicesParam;
981+
: indexAccess;
980982
}
981983
catch (InvalidOperationException e)
982984
{
@@ -1001,8 +1003,8 @@ public static IndexerPropertyGetter<TTarget, TValue, TIndex> CreateIndexerGetter
10011003
try
10021004
{
10031005
body = returnType == propertyType
1004-
? propertyAccess
1005-
: Expression.Convert(propertyAccess, returnType);
1006+
? propertyAccess
1007+
: Expression.Convert(propertyAccess, returnType);
10061008
}
10071009
catch (InvalidOperationException e)
10081010
{
@@ -1086,7 +1088,7 @@ public static IndexerPropertyGetter<TTarget, TValue, TIndex> CreateIndexerGetter
10861088
// Use provided argument type and cast to indexer parameter type if needed.
10871089
castedIndexParam = indexType != indexParameterType
10881090
? Expression.Convert(indexAccess, indexParameterType)
1089-
: indicesParam;
1091+
: indexAccess;
10901092
}
10911093
catch (InvalidOperationException e)
10921094
{
@@ -1954,6 +1956,8 @@ public static IndexerPropertyGetter<TTarget, TValue, TIndex1, TIndex2, TIndex3>
19541956
$"index {i}",
19551957
e);
19561958
}
1959+
1960+
indexExpressions[i] = castedIndexParam;
19571961
}
19581962

19591963
Type declaringType = declaringTypeData.Type;
@@ -2264,8 +2268,8 @@ public static PropertySetter<object, TValue> CreateStaticSetter<TValue>(Property
22642268
Expression validationExpression = CreateIndexParameterArrayLengthMismatchExceptionExpression(propertyData, indicesParam, isGetter: false);
22652269

22662270
ParameterList propertySetMethodParameters = propertyData.PropertySetMethodParameters;
2267-
var indexExpressions = new Expression[propertySetMethodParameters.Count];
2268-
for (int i = 0; i < propertySetMethodParameters.Count; i++)
2271+
var indexExpressions = new Expression[propertySetMethodParameters.Count - 1];
2272+
for (int i = 0; i < propertySetMethodParameters.Count - 1; i++)
22692273
{
22702274
ParameterData indexParameterData = propertySetMethodParameters[i];
22712275
Type indexParameterType = indexParameterData.ParameterTypeData.Type;
@@ -2289,6 +2293,8 @@ public static PropertySetter<object, TValue> CreateStaticSetter<TValue>(Property
22892293
$"Index {i}",
22902294
e);
22912295
}
2296+
2297+
indexExpressions[i] = castedIndexParam;
22922298
}
22932299

22942300
// Access the indexer: propertyType[index0, index1, ...]
@@ -2515,23 +2521,34 @@ public static ValueTypeIndexerPropertySetter<TTarget, TValue> CreateStructIndexe
25152521
ParameterExpression indicesParam = Expression.Parameter(typeof(object[]), "indices");
25162522
ParameterExpression valueParam = Expression.Parameter(typeof(TValue), "returnType");
25172523

2518-
ImmutableArray<ParameterInfo> indexParameters = propertyData.PropertySetMethodParameters.AsParameterInfoArray();
2519-
25202524
// Validate that indices length matches the number of index parameters when not null.
25212525
// We do this inside the expression so the check happens at runtime.
2522-
var indexExpressions = new Expression[indexParameters.Length];
2523-
for (int i = 0; i < indexParameters.Length; i++)
2526+
ParameterList propertySetMethodParameters = propertyData.PropertySetMethodParameters;
2527+
var indexExpressions = new Expression[propertySetMethodParameters.Count - 1]; // -1 because the last parameter is the value parameter for the setter, not an index parameter.
2528+
for (int i = 0; i < propertySetMethodParameters.Count - 1; i++)
25242529
{
2525-
ParameterInfo indexParameterInfo = indexParameters[i];
2526-
Type parameterType = indexParameterInfo.ParameterType;
2530+
ParameterData indexParameter = propertySetMethodParameters[i];
2531+
Type parameterType = indexParameter.ParameterTypeData.Type;
25272532

25282533
// indices[i]
25292534
BinaryExpression indexAccess = Expression.ArrayIndex(
25302535
indicesParam,
25312536
Expression.Constant(i));
25322537

2533-
// (TIndexType)indices[i]
2534-
UnaryExpression convertedIndex = Expression.Convert(indexAccess, parameterType);
2538+
UnaryExpression convertedIndex;
2539+
try
2540+
{
2541+
// (TIndexType)indices[i]
2542+
convertedIndex = Expression.Convert(indexAccess, parameterType);
2543+
}
2544+
catch (InvalidOperationException e)
2545+
{
2546+
throw new ArgumentException(
2547+
$"The provided indexer argument at position '{i}' is incompatible with the property's index parameter type. Reason: A conversion to '{indexParameter.FullyQualifiedSignature}' is not natively supported.",
2548+
$"Index {i}",
2549+
e);
2550+
}
2551+
25352552
indexExpressions[i] = convertedIndex;
25362553
}
25372554

@@ -2556,9 +2573,12 @@ private static Expression CreateIndexParameterArrayLengthMismatchExceptionExpres
25562573
ParameterList indexParameters = isGetter
25572574
? propertyData.PropertyGetMethodParameters
25582575
: propertyData.PropertySetMethodParameters;
2576+
int indexParameterCount = isGetter
2577+
? indexParameters.Count
2578+
: indexParameters.Count - 1;
25592579
Expression lengthMismatch = Expression.NotEqual(
25602580
Expression.ArrayLength(indicesParam),
2561-
Expression.Constant(indexParameters.Count));
2581+
Expression.Constant(indexParameterCount));
25622582
Expression foundIndexCount = Expression.ArrayLength(indicesParam);
25632583
MethodInfo stringConcat5 = typeof(string).GetMethod(
25642584
nameof(string.Concat),

‎src/BionicCode.Utilities.Reflection/PropertyData.cs‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,18 @@ internal TValue GetIndexerValue<TTarget, TValue, TIndex1, TIndex2>(TTarget targe
360360
}
361361
else
362362
{
363+
if (target is null)
364+
{
365+
throw new ArgumentNullException(nameof(target), "Target object cannot be null for instance properties.");
366+
}
367+
368+
Type targetType = target.GetType();
369+
ArgumentExceptionAdvanced.ThrowIfNotAssignableTo(
370+
targetType,
371+
DeclaringTypeData.Type,
372+
nameof(target),
373+
$"Type mismatch. Reason: The instance type {targetType.ToFullyQualifiedSignatureName()} is not assignable to {DeclaringTypeData.FullyQualifiedSignature}");
374+
363375
IndexerPropertyGetter<TTarget, TValue, TIndex1, TIndex2> invoker = Get2DIndexerGetterInternal<TTarget, TValue, TIndex1, TIndex2>();
364376
return invoker.Invoke(target, index1, index2);
365377
}
@@ -391,6 +403,18 @@ internal TValue GetIndexerValue<TTarget, TValue, TIndex1, TIndex2, TIndex3>(TTar
391403
}
392404
else
393405
{
406+
if (target is null)
407+
{
408+
throw new ArgumentNullException(nameof(target), "Target object cannot be null for instance properties.");
409+
}
410+
411+
Type targetType = target.GetType();
412+
ArgumentExceptionAdvanced.ThrowIfNotAssignableTo(
413+
targetType,
414+
DeclaringTypeData.Type,
415+
nameof(target),
416+
$"Type mismatch. Reason: The instance type {targetType.ToFullyQualifiedSignatureName()} is not assignable to {DeclaringTypeData.FullyQualifiedSignature}");
417+
394418
IndexerPropertyGetter<TTarget, TValue, TIndex1, TIndex2, TIndex3> invoker = Get3DIndexerGetterInternal<TTarget, TValue, TIndex1, TIndex2, TIndex3>();
395419
return invoker.Invoke(target, index1, index2, index3);
396420
}
@@ -553,8 +577,8 @@ internal void SetIndexerValue(object? target, object? value, params object?[] in
553577

554578
ArgumentNullExceptionAdvanced.ThrowIfNull(indexerPropertyIndex, nameof(indexerPropertyIndex), "Indexer property index cannot be null for indexer properties.");
555579
ArgumentOutOfRangeExceptionAdvanced.ThrowIfLessThan(
556-
indexerPropertyIndex!.Length,
557-
PropertySetMethodParameters.Count,
580+
indexerPropertyIndex.Length,
581+
PropertySetMethodParameters.Count - 1, // Subtract 1 to exclude the value parameter
558582
nameof(indexerPropertyIndex),
559583
$"Indexer property index count does not match the indexer parameter count of property '{FullyQualifiedSignature}'.");
560584

@@ -623,8 +647,8 @@ internal void SetStructIndexerValue<TTarget, TValue>(ref TTarget target, TValue
623647

624648
ArgumentNullExceptionAdvanced.ThrowIfNull(indexerPropertyIndex, nameof(indexerPropertyIndex), "Indexer property index cannot be null for indexer properties.");
625649
ArgumentOutOfRangeExceptionAdvanced.ThrowIfLessThan(
626-
indexerPropertyIndex!.Length,
627-
PropertySetMethodParameters.Count,
650+
indexerPropertyIndex.Length,
651+
PropertySetMethodParameters.Count - 1, // Subtract 1 to exclude the value parameter
628652
nameof(indexerPropertyIndex),
629653
$"Indexer property index count does not match the indexer parameter count of property '{FullyQualifiedSignature}'.");
630654

@@ -634,7 +658,7 @@ internal void SetStructIndexerValue<TTarget, TValue>(ref TTarget target, TValue
634658

635659
private IndexerPropertyGetter<TTarget, TValue, TIndex> GetIndexerGetterInternal<TTarget, TValue, TIndex>()
636660
{
637-
RuntimeTypeHandle targetTypeHandle = typeof(IndexerPropertyGetter<TTarget, TIndex, TValue>).TypeHandle;
661+
RuntimeTypeHandle targetTypeHandle = typeof(IndexerPropertyGetter<TTarget, TValue, TIndex>).TypeHandle;
638662
Delegate invoker = _invokerTable.GetOrAdd(
639663
targetTypeHandle,
640664
_ => DelegateProvider.CreateIndexerGetter<TTarget, TValue, TIndex>(this));
@@ -644,7 +668,7 @@ private IndexerPropertyGetter<TTarget, TValue, TIndex> GetIndexerGetterInternal<
644668

645669
private IndexerPropertyGetter<object?, TValue, TIndex> GetStaticIndexerGetterInternal<TValue, TIndex>()
646670
{
647-
RuntimeTypeHandle targetTypeHandle = typeof(IndexerPropertyGetter<object, TValue?, TIndex>).TypeHandle;
671+
RuntimeTypeHandle targetTypeHandle = typeof(IndexerPropertyGetter<object?, TValue, TIndex>).TypeHandle;
648672
Delegate invoker = _invokerTable.GetOrAdd(
649673
targetTypeHandle,
650674
_ => DelegateProvider.CreateStaticIndexerGetter<TValue, TIndex>(this));

0 commit comments

Comments
 (0)