Skip to content

Update PredefinedMethodsHelper #911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
{
// See http://go.microsoft.com/fwlink/?LinkId=733558
// for the documentation about the tasks.json format
"version": "0.1.0",
"version": "2.0.0",
"command": "dotnet",
"isShellCommand": true,
"args": [],
"tasks": [
{
"taskName": "build",
"args": [ ],
"isBuildCommand": true,
"showOutput": "silent",
"problemMatcher": "$msCompile"
}
]
"tasks": []
}
2 changes: 1 addition & 1 deletion src/System.Linq.Dynamic.Core/DynamicClassFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@
ilgeneratorEquals.Emit(OpCodes.Ldfld, fieldBuilders[i]);
ilgeneratorEquals.Emit(OpCodes.Callvirt, equalityComparerTEquals);

// GetHashCode();

Check warning on line 308 in src/System.Linq.Dynamic.Core/DynamicClassFactory.cs

View workflow job for this annotation

GitHub Actions / Linux: Build and Tests

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)

Check warning on line 308 in src/System.Linq.Dynamic.Core/DynamicClassFactory.cs

View workflow job for this annotation

GitHub Actions / Linux: Build and Tests

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)

Check warning on line 308 in src/System.Linq.Dynamic.Core/DynamicClassFactory.cs

View workflow job for this annotation

GitHub Actions / Linux: Build and Tests

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)

Check warning on line 308 in src/System.Linq.Dynamic.Core/DynamicClassFactory.cs

View workflow job for this annotation

GitHub Actions / Linux: Build and Tests

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
MethodInfo equalityComparerTGetHashCode = equalityComparerT.GetMethod(nameof(EqualityComparer.GetHashCode), BindingFlags.Instance | BindingFlags.Public, null, [fieldType], null)!;
ilgeneratorGetHashCode.Emit(OpCodes.Stloc_0);
ilgeneratorGetHashCode.Emit(OpCodes.Ldc_I4, -1521134295);
Expand Down Expand Up @@ -414,7 +414,7 @@
ilgeneratorToString.Emit(OpCodes.Callvirt, StringBuilderAppendString);
ilgeneratorToString.Emit(OpCodes.Pop);
ilgeneratorToString.Emit(OpCodes.Ldloc_0);
ilgeneratorToString.Emit(OpCodes.Callvirt, PredefinedMethodsHelper.ObjectToString);
ilgeneratorToString.Emit(OpCodes.Callvirt, PredefinedMethodsHelper.ObjectInstanceToString);
ilgeneratorToString.Emit(OpCodes.Ret);

EmitEqualityOperators(typeBuilder, equals);
Expand Down
8 changes: 6 additions & 2 deletions src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class ExpressionParser
private readonly ConstantExpressionHelper _constantExpressionHelper;
private readonly ITypeFinder _typeFinder;
private readonly ITypeConverterFactory _typeConverterFactory;
private readonly PredefinedMethodsHelper _predefinedMethodsHelper;
private readonly Dictionary<string, object> _internals = new();
private readonly Dictionary<string, object?> _symbols;
private readonly bool _usedForOrderBy;
Expand Down Expand Up @@ -99,6 +100,7 @@ public ExpressionParser(ParameterExpression[]? parameters, string expression, ob
_typeFinder = new TypeFinder(_parsingConfig, _keywordsHelper);
_typeConverterFactory = new TypeConverterFactory(_parsingConfig);
_constantExpressionHelper = ConstantExpressionHelperFactory.GetInstance(_parsingConfig);
_predefinedMethodsHelper = new PredefinedMethodsHelper(_parsingConfig);

if (parameters != null)
{
Expand Down Expand Up @@ -1854,7 +1856,9 @@ private Expression ParseMemberAccess(Type? type, Expression? expression, string?

case 1:
var method = (MethodInfo)methodBase!;
if (!PredefinedTypesHelper.IsPredefinedType(_parsingConfig, method.DeclaringType!) && !PredefinedMethodsHelper.IsPredefinedMethod(_parsingConfig, method))

if (!PredefinedTypesHelper.IsPredefinedType(_parsingConfig, method.DeclaringType!) &&
!_predefinedMethodsHelper.IsPredefinedMethod(type!, method.DeclaringType!, method))
{
throw ParseError(errorPos, Res.MethodIsInaccessible, id, TypeHelper.GetTypeName(method.DeclaringType!));
}
Expand Down Expand Up @@ -1934,7 +1938,7 @@ private bool TryFindPropertyOrField(Type type, string id, Expression? expression
switch (member)
{
case PropertyInfo property:
var propertyIsStatic = property?.GetGetMethod().IsStatic ?? property?.GetSetMethod().IsStatic ?? false;
var propertyIsStatic = property.GetGetMethod()?.IsStatic ?? property.GetSetMethod()?.IsStatic ?? false;
propertyOrFieldExpression = propertyIsStatic ? Expression.Property(null, property) : Expression.Property(expression, property);
return true;

Expand Down
95 changes: 80 additions & 15 deletions src/System.Linq.Dynamic.Core/Parser/PredefinedMethodsHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,92 @@

namespace System.Linq.Dynamic.Core.Parser;

internal static class PredefinedMethodsHelper
internal class PredefinedMethodsHelper
{
internal static readonly MethodInfo ObjectToString = typeof(object).GetMethod(nameof(ToString), BindingFlags.Instance | BindingFlags.Public, null, Type.EmptyTypes, null)!;
internal static readonly MethodInfo ObjectInstanceEquals = typeof(object).GetMethod(nameof(Equals), BindingFlags.Instance | BindingFlags.Public, null, [typeof(object)], null)!;
internal static readonly MethodInfo ObjectStaticEquals = typeof(object).GetMethod(nameof(Equals), BindingFlags.Static | BindingFlags.Public, null, [typeof(object), typeof(object)], null)!;
internal static readonly MethodInfo ObjectStaticReferenceEquals = typeof(object).GetMethod(nameof(ReferenceEquals), BindingFlags.Static | BindingFlags.Public, null, [typeof(object), typeof(object)], null)!;
private const BindingFlags PublicInstance = BindingFlags.Public | BindingFlags.Instance;
private const BindingFlags PublicStatic = BindingFlags.Public | BindingFlags.Static;

internal static readonly MethodInfo ObjectInstanceToString = typeof(object).GetMethod(nameof(ToString), PublicInstance, null, Type.EmptyTypes, null)!;
internal static readonly MethodInfo ObjectInstanceEquals = typeof(object).GetMethod(nameof(Equals), PublicInstance, null, [typeof(object)], null)!;
internal static readonly MethodInfo ObjectStaticEquals = typeof(object).GetMethod(nameof(Equals), PublicStatic, null, [typeof(object), typeof(object)], null)!;
internal static readonly MethodInfo ObjectStaticReferenceEquals = typeof(object).GetMethod(nameof(ReferenceEquals), PublicStatic, null, [typeof(object), typeof(object)], null)!;

private static readonly HashSet<MemberInfo> ObjectToStringAndObjectEquals =
[
ObjectToString,
ObjectInstanceEquals,
ObjectStaticEquals,
ObjectStaticReferenceEquals
];
private readonly Dictionary<Type, HashSet<MemberInfo>> _supported = new()
{
{ typeof(bool), [] },
{ typeof(byte), [] },
{ typeof(char), [] },
{ typeof(DateTime), [] },
{ typeof(DateTimeOffset), [] },
{ typeof(decimal), [] },
{ typeof(double), [] },
// { typeof(Enum), [] },
{ typeof(float), [] },
{ typeof(Guid), [] },
{ typeof(int), [] },
{ typeof(long), [] },
{ typeof(sbyte), [] },
{ typeof(short), [] },
{ typeof(string), [] },
{ typeof(TimeSpan), [] },
{ typeof(uint), [] },
{ typeof(ulong), [] },
{ typeof(Uri), [] },
{ typeof(ushort), [] },
#if NET6_0_OR_GREATER
{ typeof(DateOnly), [] },
{ typeof(TimeOnly), [] },
#endif
};

public PredefinedMethodsHelper(ParsingConfig config)
{
foreach (var kvp in _supported)
{
TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(Equals), PublicInstance, null, [kvp.Key], null));
TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(Equals), PublicInstance, null, [typeof(object)], null));

TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(ToString), PublicInstance, null, Type.EmptyTypes, null));
TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(ToString), PublicInstance, null, [typeof(string)], null));
TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(ToString), PublicInstance, null, [typeof(IFormatProvider)], null));
TryAdd(kvp.Key, kvp.Key.GetMethod(nameof(ToString), PublicInstance, null, [typeof(string), typeof(IFormatProvider)], null));
}

if (config.AllowEqualsAndToStringMethodsOnObject)
{
_supported[typeof(object)] = [ObjectInstanceToString, ObjectInstanceEquals, ObjectStaticEquals, ObjectStaticReferenceEquals];
}
}

public static bool IsPredefinedMethod(ParsingConfig config, MemberInfo member)
public bool IsPredefinedMethod(Type type, Type declaringType, MemberInfo member)
{
Check.NotNull(config);
Check.NotNull(type);
Check.NotNull(member);

return config.AllowEqualsAndToStringMethodsOnObject && ObjectToStringAndObjectEquals.Contains(member);
if (_supported.TryGetValue(type, out var supportedMethodsForType) && supportedMethodsForType.Count > 0)
{
return supportedMethodsForType.Contains(member);
}

if (_supported.TryGetValue(declaringType, out var supportedMethodsForDeclaringType) && supportedMethodsForDeclaringType.Count > 0)
{
return supportedMethodsForDeclaringType.Contains(member);
}

// Last resort, check if the method name is supported for object
if (_supported.TryGetValue(typeof(object), out var supportedMethodsForObject) && supportedMethodsForObject.Count > 0)
{
return supportedMethodsForObject.Any(x => x.Name == member.Name);
}

return false;
}

private void TryAdd(Type type, MethodInfo? method)
{
if (method != null)
{
_supported[type].Add(method);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ public int FindMethod(Type? type, string methodName, bool staticAccess, ref Expr
return 0;
}

public int FindBestMethodBasedOnArguments(IEnumerable<MethodBase> methods, ref Expression[] args, out MethodBase? method)
public int FindBestMethodBasedOnArguments(IEnumerable<MethodBase> methods, ref Expression[] args, out MethodBase? methodOrConstructor)
{
// Passing args by reference is now required with the params array support.
var inlineArgs = args;

MethodData[] applicable = methods
var applicable = methods
.Select(m => new MethodData { MethodBase = m, Parameters = m.GetParameters() })
.Where(m => IsApplicable(m, inlineArgs))
.ToArray();
Expand All @@ -175,11 +175,16 @@ public int FindBestMethodBasedOnArguments(IEnumerable<MethodBase> methods, ref E
var methodData = applicable[0];
if (methodData.MethodBase is MethodInfo methodInfo)
{
method = methodInfo.GetBaseDefinition();
// It's a method
var baseMethodInfo = methodInfo.GetBaseDefinition();

// If the declaring type is an object, do not take the object-MethodInfo but keep the original MethodInfo
methodOrConstructor = baseMethodInfo.DeclaringType == typeof(object) ? methodInfo : baseMethodInfo;
}
else
{
method = methodData.MethodBase;
// It's a constructor
methodOrConstructor = methodData.MethodBase;
}

if (args.Length == 0 || args.Length != methodData.Args.Length)
Expand All @@ -205,7 +210,7 @@ public int FindBestMethodBasedOnArguments(IEnumerable<MethodBase> methods, ref E
}
else
{
method = null;
methodOrConstructor = null;
}

return applicable.Length;
Expand Down
15 changes: 8 additions & 7 deletions test/System.Linq.Dynamic.Core.Tests/Parser/MethodFinderTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Linq.Dynamic.Core.Parser;
using System.Linq.Dynamic.Core.Tests.Helpers.Models;
using System.Linq.Expressions;
using Xunit;
using static System.Linq.Expressions.Expression;
Expand All @@ -13,13 +14,13 @@ public void Method_ToString_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
// Arrange
var config = new ParsingConfig
{
AllowEqualsAndToStringMethodsOnObject = true
// AllowEqualsAndToStringMethodsOnObject = true
};

Expression<Func<int?, string?>> expr = x => x.ToString();
Expression<Func<int, string>> expr = x => x.ToString();

var selector = "ToString()";
var prm = Parameter(typeof(int?));
var prm = Parameter(typeof(int));
var parser = new ExpressionParser([prm], selector, [], config);

// Act
Expand All @@ -30,18 +31,18 @@ public void Method_ToString_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
}

[Fact]
public void Method_Equals1_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
public void Method_InstanceEquals_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
{
// Arrange
var config = new ParsingConfig
{
AllowEqualsAndToStringMethodsOnObject = true
};

Expression<Func<int?, bool>> expr = x => x.Equals("a");
Expression<Func<User, bool>> expr = x => x.Equals("a");

var selector = "Equals(\"a\")";
var prm = Parameter(typeof(int?));
var prm = Parameter(typeof(User));
var parser = new ExpressionParser([prm], selector, [], config);

// Act
Expand All @@ -52,7 +53,7 @@ public void Method_Equals1_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
}

[Fact]
public void Method_Equals2_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
public void Method_StaticEquals_OnDynamicLinq_And_SystemLinq_ShouldBeEqual()
{
// Arrange
var config = new ParsingConfig
Expand Down
Loading
Loading