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

C# Expressions use Base Class Virtual Methodinfo #580

Closed
jogibear9988 opened this issue Mar 21, 2022 · 8 comments
Closed

C# Expressions use Base Class Virtual Methodinfo #580

jogibear9988 opened this issue Mar 21, 2022 · 8 comments
Assignees
Labels

Comments

@jogibear9988
Copy link
Contributor

If you write a Expression where you call a Virtual Method, the C# Expression Parser uses the MethodInfo from the BaseClass where System.Dynamic.Linq uses the MethodInfo from the Derived Class.

see issue: linq2db/linq2db#3475

@jogibear9988 jogibear9988 changed the title C# Expression use Base Virtual Methodinfo C# Expressions use Base Class Virtual Methodinfo Mar 21, 2022
@StefH StefH self-assigned this Mar 22, 2022
@jogibear9988
Copy link
Contributor Author

This will be fixed in linq2db, but ,aybe will also fail in other Linq Frameworks

@StefH
Copy link
Collaborator

StefH commented Mar 22, 2022

@jogibear9988

  • Do you have any lead where this should be fixed in the code ?
  • Or maybe create a PR with unit tests?

@zspitz
Copy link

zspitz commented Mar 22, 2022

If I understand correctly, the following code:

using System.Linq.Expressions;
using static System.Console;
using static System.Linq.Expressions.Expression;
using System.Linq.Dynamic.Core.Parser;
using System.Linq.Dynamic.Core;

Expression<Func<int?, string?>> expr = x => x.ToString();
WriteLine(
    ((MethodCallExpression)expr.Body).Method.DeclaringType
);

var selector = "ToString()";
var prm = Parameter(typeof(int?));
var parser = new ExpressionParser(new[] { prm }, selector, new object[] { }, ParsingConfig.Default);
var expr1 = parser.Parse(null);
WriteLine(
    ((MethodCallExpression)expr1).Method.DeclaringType
);

prints out:

System.Object
System.Nullable`1[System.Int32]

where both should have been the same.

@jogibear9988
Copy link
Contributor Author

yes

@StefH
Copy link
Collaborator

StefH commented Apr 17, 2022

@jogibear9988 and @zspitz

I did try to find the specific changes in source code in (https://github.com/zzzprojects/System.Linq.Dynamic/blob/archived/Src/System.Linq.Dynamic/DynamicLinq.cs) and this library.

But finding a method and getting the base-type is done the same way.

And the reason System.Nullable`1[System.Int32] is returned in that example code, is because this code line:
image

And this is required.


So can you please provide some more details on why / how this should be fixed in this project?

@StefH
Copy link
Collaborator

StefH commented Sep 9, 2022

Hello @jogibear9988,

Can you please check my last comment?

@jogibear9988
Copy link
Contributor Author

I think FindMethod should not return the Method it found, but the result of ".GetBaseDefinition()" of the Method it found.
So you always return the MethodInfo TopMost in the Type hierarchy wich has it's own implementation.

jogibear9988 added a commit to jogibear9988/System.Linq.Dynamic.Core that referenced this issue Sep 12, 2022
uses the topmost implementation of the virtual method
@jogibear9988
Copy link
Contributor Author

see pull #630

@StefH StefH closed this as completed in fca6cb2 Oct 20, 2022
@StefH StefH added the bug label Oct 23, 2022
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

3 participants