Skip to content
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

Extending with extensions methods (DynamicLinqType attribute) #497

Closed
alex-vorobyev opened this issue Apr 6, 2021 · 3 comments
Closed
Assignees
Labels

Comments

@alex-vorobyev
Copy link

alex-vorobyev commented Apr 6, 2021

Hello.

When working with extensions methods, we encountered the following defects (in our opinion):

  1. If several extensions methods are created with the same number and types of arguments, then we get the error "Ambiguous invocation of method ...".
  2. If the base class has an extensions method, then it is not found, the error is "No applicable method ..."
  3. If there is a generic extensions method with the same name, then we also get an error "Ambiguous invocation of method ...".

It is proposed to slightly improve the method FindMethod in a class MethodFinder (System.Linq.Dynamic.Core.Parser.SupportedMethods):

  1. Search extension methods based on the method name.
  2. Search including base classes.
  3. Skip generic methods.

For example, instead (line 55):

                // Try to solve with registered extension methods 
                if (_parsingConfig.CustomTypeProvider.GetExtensionMethods().TryGetValue(type, out var methods))
                {
                   ...
                }

Do the following:

                var methods = new List<MethodInfo>();
                foreach (var t in SelfAndBaseTypes(type))
                {
                    // Try to solve with registered extension methods 
                    if (_parsingConfig.CustomTypeProvider.GetExtensionMethods().TryGetValue(t, out var methodsOfType))
                    {
                        methods.AddRange(methodsOfType.Where(item =>
                            item.Name.Equals(methodName, StringComparison.OrdinalIgnoreCase) && !item.IsGenericMethod));
                    }
                }
                if (methods.Any())
                {
                   ...
                }
@kriszek
Copy link

kriszek commented Apr 9, 2021

Hi,

I have also run into above issue in my current project. I agree that MethodFinder only uses parameters to match a method and skips it's name.

You can test it by adding:

public static int IncrementMeAlso(this int values)
{
return values + 1;
}

next to IncrementMe method in Utlis class of System.Linq.Dynamic.Core.Tests.Parser.DynamicLinqTypeTest.
ExtensionMethod_NoParameter test will fail.

@nanov
Copy link

nanov commented May 5, 2021

+1 on this one, practically it turns extension methods into unusable at best, unpredictable at worst.

@StefH
Copy link
Collaborator

StefH commented May 11, 2021

Hello @kriszek, thank you for this detailed description + solution.

I did update fix the code as suggested and updated unit-tests to verify the solution.

I'll merge the PR and a this fix will be included in next NuGet version.

@StefH StefH closed this as completed May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants