From 3eb8e42206ea616a5c8fce5ab7b836839b3f0536 Mon Sep 17 00:00:00 2001 From: Holden Mai Date: Sun, 13 Apr 2025 07:42:17 -0600 Subject: [PATCH 1/6] Using a lazy access pattern instead of accessing ErrorMessages.[MessageProperty] when calling standard multi purpose methods such as ParseArgumentList. --- src/DynamicExpresso.Core/Parsing/Parser.cs | 58 +++++----- .../Resources/ErrorMessage.cs | 109 ++++++++++++++++++ 2 files changed, 138 insertions(+), 29 deletions(-) create mode 100644 src/DynamicExpresso.Core/Resources/ErrorMessage.cs diff --git a/src/DynamicExpresso.Core/Parsing/Parser.cs b/src/DynamicExpresso.Core/Parsing/Parser.cs index be429ed..0273580 100644 --- a/src/DynamicExpresso.Core/Parsing/Parser.cs +++ b/src/DynamicExpresso.Core/Parsing/Parser.cs @@ -67,7 +67,7 @@ private Expression Parse() { Expression expr = ParseExpressionSegment(_arguments.ExpressionReturnType); - ValidateToken(TokenId.End, ErrorMessages.SyntaxError); + ValidateToken(TokenId.End, ErrorMessage.SyntaxError); return expr; } @@ -170,7 +170,7 @@ private ParameterWithPosition[] ParseLambdaParameterList() var parameters = _token.id != TokenId.CloseParen ? ParseLambdaParameters() : new ParameterWithPosition[0]; if (hasOpenParen) { - ValidateToken(TokenId.CloseParen, ErrorMessages.CloseParenOrCommaExpected); + ValidateToken(TokenId.CloseParen, ErrorMessage.CloseParenOrCommaExpected); NextToken(); } @@ -255,7 +255,7 @@ private Expression ParseConditional() { NextToken(); var expr1 = ParseExpressionSegment(); - ValidateToken(TokenId.Colon, ErrorMessages.ColonExpected); + ValidateToken(TokenId.Colon, ErrorMessage.ColonExpected); NextToken(); var expr2 = ParseExpressionSegment(); expr = GenerateConditional(expr, expr1, expr2, errorPos); @@ -991,10 +991,10 @@ private static Expression CreateLiteral(object value) private Expression ParseParenExpression() { - ValidateToken(TokenId.OpenParen, ErrorMessages.OpenParenExpected); + ValidateToken(TokenId.OpenParen, ErrorMessage.OpenParenExpected); NextToken(); Expression innerParenthesesExpression = ParseExpressionSegment(); - ValidateToken(TokenId.CloseParen, ErrorMessages.CloseParenOrOperatorExpected); + ValidateToken(TokenId.CloseParen, ErrorMessage.CloseParenOrOperatorExpected); var constExp = innerParenthesesExpression as ConstantExpression; if (constExp != null && constExp.Value is Type) @@ -1108,11 +1108,11 @@ private Expression ParseDefaultOperator() { NextToken(); - ValidateToken(TokenId.OpenParen, ErrorMessages.OpenParenExpected); + ValidateToken(TokenId.OpenParen, ErrorMessage.OpenParenExpected); NextToken(); ValidateToken(TokenId.Identifier); var type = ParseKnownType(); - ValidateToken(TokenId.CloseParen, ErrorMessages.CloseParenOrCommaExpected); + ValidateToken(TokenId.CloseParen, ErrorMessage.CloseParenOrCommaExpected); NextToken(); return Expression.Default(type); @@ -1162,7 +1162,7 @@ private Expression GenerateConditionalDynamic(Expression test, Expression expr1, private Expression ParseNew() { NextToken(); - ValidateToken(TokenId.Identifier, ErrorMessages.IdentifierExpected); + ValidateToken(TokenId.Identifier, ErrorMessage.IdentifierExpected); var newType = ParseKnownType(); var args = new Expression[0]; @@ -1181,7 +1181,7 @@ private Expression ParseNew() else { // no aguments: expect an object initializer - ValidateToken(TokenId.OpenCurlyBracket, ErrorMessages.OpenCurlyBracketExpected); + ValidateToken(TokenId.OpenCurlyBracket, ErrorMessage.OpenCurlyBracketExpected); } var applicableConstructors = MethodResolution.FindBestMethod(newType.GetConstructors(), args); @@ -1202,17 +1202,17 @@ private Expression ParseNew() private Expression[] ParseArrayInitializerList() { - return ParseArgumentList(TokenId.OpenCurlyBracket, ErrorMessages.OpenCurlyBracketExpected, - TokenId.CloseCurlyBracket, ErrorMessages.CloseCurlyBracketExpected, + return ParseArgumentList(TokenId.OpenCurlyBracket, ErrorMessage.OpenCurlyBracketExpected, + TokenId.CloseCurlyBracket, ErrorMessage.CloseCurlyBracketExpected, allowTrailingComma: true); } private Expression ParseWithObjectInitializer(NewExpression newExpr, Type newType) { - ValidateToken(TokenId.OpenCurlyBracket, ErrorMessages.OpenCurlyBracketExpected); + ValidateToken(TokenId.OpenCurlyBracket, ErrorMessage.OpenCurlyBracketExpected); NextToken(); var initializedInstance = ParseMemberAndInitializerList(newExpr, newType); - ValidateToken(TokenId.CloseCurlyBracket, ErrorMessages.CloseCurlyBracketExpected); + ValidateToken(TokenId.CloseCurlyBracket, ErrorMessage.CloseCurlyBracketExpected); NextToken(); return initializedInstance; } @@ -1249,7 +1249,7 @@ private Expression ParseMemberAndInitializerList(NewExpression newExpr, Type new private void ParsePossibleMemberBinding(Type newType, int originalPos, List bindingList, List actions, ParameterExpression instance, bool allowCollectionInit) { - ValidateToken(TokenId.Identifier, ErrorMessages.IdentifierExpected); + ValidateToken(TokenId.Identifier, ErrorMessage.IdentifierExpected); var propertyOrFieldName = _token.text; var member = _memberFinder.FindPropertyOrField(newType, propertyOrFieldName, false); @@ -1282,7 +1282,7 @@ private void ParsePossibleMemberBinding(Type newType, int originalPos, List ParseTypeArgumentList() args = new List(new Type[arity]); } - ValidateToken(TokenId.GreaterThan, ErrorMessages.CloseTypeArgumentListExpected); + ValidateToken(TokenId.GreaterThan, ErrorMessage.CloseTypeArgumentListExpected); return args; } @@ -1569,7 +1569,7 @@ private Expression ParseTypeKeyword(Type type) return Expression.Constant(type); } - ValidateToken(TokenId.Dot, ErrorMessages.DotOrOpenParenExpected); + ValidateToken(TokenId.Dot, ErrorMessage.DotOrOpenParenExpected); NextToken(); return ParseMemberAccess(type, null); } @@ -1684,11 +1684,11 @@ private Expression GeneratePropertyOrFieldExpression(Type type, Expression insta private Expression ParseMethodInvocation(Type type, Expression instance, int errorPos, string methodName) { - return ParseMethodInvocation(type, instance, errorPos, methodName, TokenId.OpenParen, ErrorMessages.OpenParenExpected, TokenId.CloseParen, ErrorMessages.CloseParenOrCommaExpected); + return ParseMethodInvocation(type, instance, errorPos, methodName, TokenId.OpenParen, ErrorMessage.OpenParenExpected, TokenId.CloseParen, ErrorMessage.CloseParenOrCommaExpected); } - private Expression ParseMethodInvocation(Type type, Expression instance, int errorPos, string methodName, TokenId open, string openExpected, TokenId close, string closeExpected) + private Expression ParseMethodInvocation(Type type, Expression instance, int errorPos, string methodName, TokenId open, ErrorMessage openExpected, TokenId close, ErrorMessage closeExpected) { var args = ParseArgumentList(open, openExpected, close, closeExpected); @@ -1818,8 +1818,8 @@ private static Expression ParseDynamicIndex(Type type, Expression instance, Expr return Expression.Dynamic(new LateGetIndexCallSiteBinder(), typeof(object), argsDynamic); } - private Expression[] ParseArgumentList(TokenId openToken, string missingOpenTokenMsg, - TokenId closeToken, string missingCloseTokenMsg, + private Expression[] ParseArgumentList(TokenId openToken, ErrorMessage missingOpenTokenMsg, + TokenId closeToken, ErrorMessage missingCloseTokenMsg, bool allowTrailingComma = false) { ValidateToken(openToken, missingOpenTokenMsg); @@ -1840,15 +1840,15 @@ private Expression[] ParseArgumentList(TokenId openToken, string missingOpenToke private Expression[] ParseArgumentList() { - return ParseArgumentList(TokenId.OpenParen, ErrorMessages.OpenParenExpected, - TokenId.CloseParen, ErrorMessages.CloseParenOrCommaExpected); + return ParseArgumentList(TokenId.OpenParen, ErrorMessage.OpenParenExpected, + TokenId.CloseParen, ErrorMessage.CloseParenOrCommaExpected); } private Expression ParseElementAccess(Expression expr) { var errorPos = _token.pos; - var args = ParseArgumentList(TokenId.OpenBracket, ErrorMessages.OpenParenExpected, - TokenId.CloseBracket, ErrorMessages.CloseBracketOrCommaExpected); + var args = ParseArgumentList(TokenId.OpenBracket, ErrorMessage.OpenParenExpected, + TokenId.CloseBracket, ErrorMessage.CloseBracketOrCommaExpected); if (expr.Type.IsArray) { if (expr.Type.GetArrayRank() != args.Length) @@ -2571,7 +2571,7 @@ private void NextToken() private string GetIdentifier() { - ValidateToken(TokenId.Identifier, ErrorMessages.IdentifierExpected); + ValidateToken(TokenId.Identifier, ErrorMessage.IdentifierExpected); var id = _token.text; if (id.Length > 1 && id[0] == '@') id = id.Substring(1); @@ -2585,7 +2585,7 @@ private void ValidateDigit() } // ReSharper disable once UnusedParameter.Local - private void ValidateToken(TokenId t, string errorMessage) + private void ValidateToken(TokenId t, ErrorMessage errorMessage) { if (_token.id != t) throw ParseException.Create(_token.pos, errorMessage); diff --git a/src/DynamicExpresso.Core/Resources/ErrorMessage.cs b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs new file mode 100644 index 0000000..8567dc0 --- /dev/null +++ b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs @@ -0,0 +1,109 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace DynamicExpresso.Resources +{ + /// + /// Used to provide a lazily accessed Error Message. + /// + public class ErrorMessage + { + private string _message; + private Func _getMessage; + + /// + /// Initializes this instance to return the provided message. + /// + /// + public ErrorMessage(string message) + { + _message = message; + _getMessage = null; + } + + /// + /// Initializes this instance to call the provided method to retrieve the message. + /// + /// + public ErrorMessage(Func message) + { + _message = null; + _getMessage = message; + } + + /// + /// Returns the message this instance was initialized with. + /// + /// + public override string ToString() + { + return _message ?? _getMessage(); + } + + public static implicit operator string(ErrorMessage message) + { + return message.ToString(); + } + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage OpenCurlyBracketExpected = new ErrorMessage(() => ErrorMessages.OpenCurlyBracketExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage OpenParenExpected = new ErrorMessage(() => ErrorMessages.OpenParenExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage CloseCurlyBracketExpected = new ErrorMessage(() => ErrorMessages.CloseCurlyBracketExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage CloseParenOrCommaExpected = new ErrorMessage(() => ErrorMessages.CloseParenOrCommaExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage CloseBracketOrCommaExpected = new ErrorMessage(() => ErrorMessages.CloseBracketOrCommaExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage SyntaxError = new ErrorMessage(() => ErrorMessages.SyntaxError); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage ColonExpected = new ErrorMessage(() => ErrorMessages.ColonExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage CloseParenOrOperatorExpected = new ErrorMessage(() => ErrorMessages.CloseParenOrOperatorExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage IdentifierExpected = new ErrorMessage(() => ErrorMessages.IdentifierExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage EqualExpected = new ErrorMessage(() => ErrorMessages.EqualExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage CloseTypeArgumentListExpected = new ErrorMessage(() => ErrorMessages.CloseTypeArgumentListExpected); + + /// + /// The singleton instance that returns a call to . + /// + public static readonly ErrorMessage DotOrOpenParenExpected = new ErrorMessage(() => ErrorMessages.DotOrOpenParenExpected); + } +} From 42fbaec33cfec9c035b126fe957e965a737a3fd0 Mon Sep 17 00:00:00 2001 From: Holden Mai Date: Sun, 13 Apr 2025 07:46:23 -0600 Subject: [PATCH 2/6] Updating the FindBestMethod logic to reduce how many candidate methods are tracked throughout the process to keep our list size minimal. --- .../Resolution/MethodResolution.cs | 114 +++++++++++++----- 1 file changed, 86 insertions(+), 28 deletions(-) diff --git a/src/DynamicExpresso.Core/Resolution/MethodResolution.cs b/src/DynamicExpresso.Core/Resolution/MethodResolution.cs index a3ac76d..2670ff6 100644 --- a/src/DynamicExpresso.Core/Resolution/MethodResolution.cs +++ b/src/DynamicExpresso.Core/Resolution/MethodResolution.cs @@ -20,32 +20,55 @@ public static IList FindBestMethod(IEnumerable methods, public static IList FindBestMethod(IEnumerable methods, Expression[] args) { - var applicable = new List(); + List applicable = null; + bool allowPartial = true; foreach (var method in methods) { - if (CheckIfMethodIsApplicableAndPrepareIt(method, args)) - applicable.Add(method); - } - - if (applicable.Count > 1) - { - var bestCandidates = new List(applicable.Count); - foreach (var candidate in applicable) + var matchLevel = CheckMethodMatchAndPrepareIt(!allowPartial, method, args); + if (matchLevel == MethodMatchLevel.Exact || allowPartial && matchLevel == MethodMatchLevel.Partial) { - if (IsBetterThanAllCandidates(candidate, applicable, args)) - bestCandidates.Add(candidate); + allowPartial = matchLevel != MethodMatchLevel.Exact; + if (applicable?.Count > 0) + { + if (IsBetterThanAllCandidates(method, applicable, args)) + { + applicable.Clear(); + } + else + { + //Are the existing ones better? + bool skip = false; + foreach (var priorCandidate in applicable) + { + if (MethodHasPriority(args, priorCandidate, method)) + { + //The current method is not as good as the existing candidate(s) + skip = true; + break; + } + } + if (skip) + { + continue; + } + } + } + if (applicable == null) + { + applicable = new List(); + } + applicable.Add(method); + if (!allowPartial && applicable.Count > 1) + { + //We've hit 2 exact matches so no need to go any farther. + break; + } } - - // bestCandidates.Count == 0 means that no applicable method has priority - // we don't return bestCandidates to prevent callers from thinking no method was found - if (bestCandidates.Count > 0) - return bestCandidates; } - - return applicable; + return applicable as IList ?? Array.Empty(); } - private static bool IsBetterThanAllCandidates(MethodData candidate, IList otherCandidates, Expression[] args) + private static bool IsBetterThanAllCandidates(MethodData candidate, List otherCandidates, Expression[] args) { foreach (var other in otherCandidates) { @@ -56,10 +79,31 @@ private static bool IsBetterThanAllCandidates(MethodData candidate, IList !y.HasDefaultValue && !ReflectionExtensions.HasParamsArrayType(y)) > args.Length) - return false; + return CheckMethodMatchAndPrepareIt(false, method, args) != MethodMatchLevel.None; + } + + public static MethodMatchLevel CheckMethodMatchAndPrepareIt(bool isExactMatchRequired, MethodData method, Expression[] args) + { + int count = 0; + foreach (var y in method.Parameters) + { + if (!y.HasDefaultValue && !ReflectionExtensions.HasParamsArrayType(y)) + { + if (++count > args.Length) + { + return MethodMatchLevel.None; + } + } + } var promotedArgs = new List(method.Parameters.Count); var declaredWorkingParameters = 0; @@ -67,6 +111,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr Type paramsArrayTypeFound = null; List paramsArrayPromotedArgument = null; + MethodMatchLevel matchLevel = MethodMatchLevel.Exact; foreach (var currentArgument in args) { Type parameterType; @@ -79,13 +124,13 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr { if (declaredWorkingParameters >= method.Parameters.Count) { - return false; + return MethodMatchLevel.None; } var parameterDeclaration = method.Parameters[declaredWorkingParameters]; if (parameterDeclaration.IsOut) { - return false; + return MethodMatchLevel.None; } parameterType = parameterDeclaration.ParameterType; @@ -93,6 +138,11 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr if (ReflectionExtensions.HasParamsArrayType(parameterDeclaration)) { paramsArrayTypeFound = parameterType; + if (isExactMatchRequired) + { + return MethodMatchLevel.Partial; + } + matchLevel = MethodMatchLevel.Partial; } declaredWorkingParameters++; @@ -104,7 +154,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr { // an interpreter expression can only be matched to a parameter of type Func if (currentArgument is InterpreterExpression) - return false; + return MethodMatchLevel.None; promotedArgs.Add(currentArgument); continue; @@ -113,6 +163,14 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr var promoted = ExpressionUtils.PromoteExpression(currentArgument, parameterType); if (promoted != null) { + if (promoted != currentArgument) + { + if (isExactMatchRequired) + { + return MethodMatchLevel.Partial; + } + matchLevel = MethodMatchLevel.Partial; + } promotedArgs.Add(promoted); continue; } @@ -137,7 +195,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr } } - return false; + return MethodMatchLevel.None; } if (paramsArrayPromotedArgument != null) @@ -190,7 +248,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr { var genericMethod = MakeGenericMethod(method); if (genericMethod == null) - return false; + return MethodMatchLevel.None; // we have all the type information we can get, update interpreter expressions and evaluate them var actualMethodParameters = genericMethod.GetParameters(); @@ -201,7 +259,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr var actualParamInfo = actualMethodParameters[i]; var lambdaExpr = ie.EvalAs(actualParamInfo.ParameterType); if (lambdaExpr == null) - return false; + return MethodMatchLevel.None; method.PromotedParameters[i] = lambdaExpr; @@ -213,7 +271,7 @@ public static bool CheckIfMethodIsApplicableAndPrepareIt(MethodData method, Expr method.MethodBase = genericMethod; } - return true; + return matchLevel; } private static bool MethodHasPriority(Expression[] args, MethodData method, MethodData otherMethod) From 59c6cb950c01cf5ade805442b64f94446c09084e Mon Sep 17 00:00:00 2001 From: Holden Mai Date: Sun, 13 Apr 2025 07:48:16 -0600 Subject: [PATCH 3/6] Updating HasParamsArrayType to only check for the existence of a ParamsArrayAttr if we are actually an Array type. --- src/DynamicExpresso.Core/Reflection/ReflectionExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DynamicExpresso.Core/Reflection/ReflectionExtensions.cs b/src/DynamicExpresso.Core/Reflection/ReflectionExtensions.cs index d648873..c5fd286 100644 --- a/src/DynamicExpresso.Core/Reflection/ReflectionExtensions.cs +++ b/src/DynamicExpresso.Core/Reflection/ReflectionExtensions.cs @@ -95,7 +95,7 @@ public static Type GetActionType(int parameterCount) public static bool HasParamsArrayType(ParameterInfo parameterInfo) { - return parameterInfo.IsDefined(typeof(ParamArrayAttribute), false); + return parameterInfo.ParameterType.IsArray && parameterInfo.IsDefined(typeof(ParamArrayAttribute), false); } public static Type GetParameterType(ParameterInfo parameterInfo) From 4576ce67d5b837bbee4a3b071f4724d3ab037bb8 Mon Sep 17 00:00:00 2001 From: Holden Mai Date: Sun, 13 Apr 2025 08:10:11 -0600 Subject: [PATCH 4/6] Adding some caching logic to our member lookup as we can hit the exact same lookup multiple times even for simple expressions. --- .../Reflection/MemberFinder.cs | 120 +++++++++++++++--- .../Resources/ErrorMessage.cs | 4 +- 2 files changed, 107 insertions(+), 17 deletions(-) diff --git a/src/DynamicExpresso.Core/Reflection/MemberFinder.cs b/src/DynamicExpresso.Core/Reflection/MemberFinder.cs index 6a1471c..a420610 100644 --- a/src/DynamicExpresso.Core/Reflection/MemberFinder.cs +++ b/src/DynamicExpresso.Core/Reflection/MemberFinder.cs @@ -9,15 +9,61 @@ namespace DynamicExpresso.Reflection { internal class MemberFinder { + private struct MemberTypeKey + { + public Type _type; + public string _memberName; + public BindingFlags _flags; + + public MemberTypeKey(Type type, string memberName, BindingFlags flags) + { + _type = type; + _memberName = memberName; + _flags = flags; + } + } + + private class MemberTypeKeyEqualityComparer : IEqualityComparer + { + public static readonly MemberTypeKeyEqualityComparer Instance = new MemberTypeKeyEqualityComparer(); + + public bool Equals(MemberTypeKey x, MemberTypeKey y) + { + return x._type.Equals(y._type) && x._flags == y._flags && x._memberName.Equals(y._memberName, StringComparison.Ordinal); + } + + + public int GetHashCode(MemberTypeKey obj) + { + return obj._type.GetHashCode() ^ obj._flags.GetHashCode() ^ obj._memberName.GetHashCode(); + } + } + + private class MemberTypeKeyCaseInsensitiveEqualityComparer : IEqualityComparer + { + public static readonly MemberTypeKeyCaseInsensitiveEqualityComparer Instance = new MemberTypeKeyCaseInsensitiveEqualityComparer(); + public bool Equals(MemberTypeKey x, MemberTypeKey y) + { + return x._type.Equals(y._type) && x._flags == y._flags && x._memberName.Equals(y._memberName, StringComparison.OrdinalIgnoreCase); + } + + public int GetHashCode(MemberTypeKey obj) + { + return obj._type.GetHashCode() ^ obj._flags.GetHashCode() ^ StringComparer.InvariantCulture.GetHashCode(obj._memberName); + } + } + private readonly ParserArguments _arguments; private readonly BindingFlags _bindingCase; private readonly MemberFilter _memberFilterCase; + private readonly Dictionary _members; public MemberFinder(ParserArguments arguments) { _arguments = arguments; _bindingCase = arguments.Settings.CaseInsensitive ? BindingFlags.IgnoreCase : BindingFlags.Default; _memberFilterCase = arguments.Settings.CaseInsensitive ? Type.FilterNameIgnoreCase : Type.FilterName; + _members = new Dictionary(arguments.Settings.CaseInsensitive ? (IEqualityComparer)MemberTypeKeyCaseInsensitiveEqualityComparer.Instance : MemberTypeKeyEqualityComparer.Instance); } public MemberInfo FindPropertyOrField(Type type, string memberName, bool staticAccess) @@ -25,22 +71,61 @@ public MemberInfo FindPropertyOrField(Type type, string memberName, bool staticA var flags = BindingFlags.Public | BindingFlags.DeclaredOnly | (staticAccess ? BindingFlags.Static : BindingFlags.Instance) | _bindingCase; - foreach (var t in SelfAndBaseTypes(type)) + //Not looping here because FindMembers loops internally and returns us the highest level member + var members = FindMembers(type, MemberTypes.Property | MemberTypes.Field, flags, memberName); + if (members.Length != 0) { - var members = t.FindMembers(MemberTypes.Property | MemberTypes.Field, flags, _memberFilterCase, memberName); - if (members.Length != 0) - return members[0]; + return members[0]; } return null; } + private MemberInfo[] FindMembers(Type type, MemberTypes memberTypes, BindingFlags flags, string memberName) + { + var key = new MemberTypeKey(type, memberName, flags); + if (_members.TryGetValue(key, out var members)) + { + if (members != null) + { + return members; + } + } + members = type.FindMembers(memberTypes, flags, _memberFilterCase, memberName); + if (members.Length == 0) + { + foreach (var v in SelfAndBaseTypes(type)) + { + if (v == type) + { + continue; + } + var subMembers = FindMembers(v, memberTypes, flags, memberName); + if (members.Length == 0 && subMembers.Length > 0) + { + //We don't break here because there is a possibility that somebody outside here is also doing an additional tree prioritization (See FindMethods) + members = subMembers; + } + } + } + if (members.Length == 0) + { + members = Array.Empty(); + } + _members[key] = members; + return members; + } + public IList FindMethods(Type type, string methodName, bool staticAccess, Expression[] args) { var flags = BindingFlags.Public | BindingFlags.DeclaredOnly | (staticAccess ? BindingFlags.Static : BindingFlags.Instance) | _bindingCase; + //Yes, FindMembers loops internally, but there are certain circumstances where we end up also needing this + //An example is Type.GetMethods(), where there are multiple overloads on the base Type class + //and one override on a derived class. The DeclaredOnly flag will return the single method on the derived class unless we go lower. + //The existing unit tests do pass if we get rid of DeclaredOnly and remove the loop, but I'm not sure of the actual impact that may have. foreach (var t in SelfAndBaseTypes(type)) { - var members = t.FindMembers(MemberTypes.Method, flags, _memberFilterCase, methodName); + var members = FindMembers(t, MemberTypes.Method, flags, methodName); var applicableMethods = MethodResolution.FindBestMethod(members.Cast(), args); if (applicableMethods.Count > 0) @@ -100,14 +185,19 @@ private static IEnumerable SelfAndBaseTypes(Type type) { if (type.IsInterface) { - var types = new List(); - AddInterface(types, type); - - types.Add(typeof(object)); + var types = new HashSet(); + foreach (var v in AddInterface(types, type)) + { + yield return v; + } - return types; + yield return typeof(object); + yield break; + } + foreach (var t in SelfAndBaseClasses(type)) + { + yield return t; } - return SelfAndBaseClasses(type); } private static IEnumerable SelfAndBaseClasses(Type type) @@ -119,14 +209,14 @@ private static IEnumerable SelfAndBaseClasses(Type type) } } - private static void AddInterface(List types, Type type) + private static IEnumerable AddInterface(HashSet types, Type type) { if (!types.Contains(type)) { - types.Add(type); - foreach (var t in type.GetInterfaces()) + yield return type; + foreach (var i in type.GetInterfaces().SelectMany(x => AddInterface(types, x))) { - AddInterface(types, t); + yield return i; } } } diff --git a/src/DynamicExpresso.Core/Resources/ErrorMessage.cs b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs index 8567dc0..b67f67b 100644 --- a/src/DynamicExpresso.Core/Resources/ErrorMessage.cs +++ b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs @@ -9,8 +9,8 @@ namespace DynamicExpresso.Resources /// public class ErrorMessage { - private string _message; - private Func _getMessage; + private readonly string _message; + private readonly Func _getMessage; /// /// Initializes this instance to return the provided message. From cb7ea5f8abfd665e40fab0af55ae502db72e1221 Mon Sep 17 00:00:00 2001 From: holdenmai <111238081+holdenmai@users.noreply.github.com> Date: Sat, 17 May 2025 11:53:19 -0600 Subject: [PATCH 5/6] Update ErrorMessage from PR comments --- .../Resources/ErrorMessage.cs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/DynamicExpresso.Core/Resources/ErrorMessage.cs b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs index b67f67b..d24822f 100644 --- a/src/DynamicExpresso.Core/Resources/ErrorMessage.cs +++ b/src/DynamicExpresso.Core/Resources/ErrorMessage.cs @@ -7,28 +7,16 @@ namespace DynamicExpresso.Resources /// /// Used to provide a lazily accessed Error Message. /// - public class ErrorMessage + internal sealed class ErrorMessage { - private readonly string _message; private readonly Func _getMessage; - /// - /// Initializes this instance to return the provided message. - /// - /// - public ErrorMessage(string message) - { - _message = message; - _getMessage = null; - } - /// /// Initializes this instance to call the provided method to retrieve the message. /// /// public ErrorMessage(Func message) { - _message = null; _getMessage = message; } @@ -38,7 +26,7 @@ public ErrorMessage(Func message) /// public override string ToString() { - return _message ?? _getMessage(); + return _getMessage(); } public static implicit operator string(ErrorMessage message) From 1b000f3426622065d6998e79a9f5a183443c3fc6 Mon Sep 17 00:00:00 2001 From: holdenmai <111238081+holdenmai@users.noreply.github.com> Date: Sat, 17 May 2025 12:22:46 -0600 Subject: [PATCH 6/6] Update MemberFinder with PR comments. --- src/DynamicExpresso.Core/Reflection/MemberFinder.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/DynamicExpresso.Core/Reflection/MemberFinder.cs b/src/DynamicExpresso.Core/Reflection/MemberFinder.cs index a420610..197ff37 100644 --- a/src/DynamicExpresso.Core/Reflection/MemberFinder.cs +++ b/src/DynamicExpresso.Core/Reflection/MemberFinder.cs @@ -49,7 +49,7 @@ public bool Equals(MemberTypeKey x, MemberTypeKey y) public int GetHashCode(MemberTypeKey obj) { - return obj._type.GetHashCode() ^ obj._flags.GetHashCode() ^ StringComparer.InvariantCulture.GetHashCode(obj._memberName); + return obj._type.GetHashCode() ^ obj._flags.GetHashCode() ^ StringComparer.OrdinalIgnoreCase.GetHashCode(obj._memberName); } } @@ -107,10 +107,6 @@ private MemberInfo[] FindMembers(Type type, MemberTypes memberTypes, BindingFlag } } } - if (members.Length == 0) - { - members = Array.Empty(); - } _members[key] = members; return members; } @@ -211,7 +207,7 @@ private static IEnumerable SelfAndBaseClasses(Type type) private static IEnumerable AddInterface(HashSet types, Type type) { - if (!types.Contains(type)) + if (types.Add(type)) { yield return type; foreach (var i in type.GetInterfaces().SelectMany(x => AddInterface(types, x)))