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

Handle default parameters type explicitly #105

Merged
merged 1 commit into from
Feb 3, 2019

Conversation

weltall
Copy link
Contributor

@weltall weltall commented Feb 2, 2019

When an argument default value is gathered from a method definition be sure to pass also its associated argument when creating the Expression.Constant.

This avoids a problem which happens when the argument is null: in this case the type of the argument will be System.Object and that will fail when binding the arguments to a method defining a different type (for example string)

Additionally see remarks section from https://docs.microsoft.com/en-us/dotnet/api/system.linq.expressions.expression.constant?view=netframework-4.7.2#System_Linq_Expressions_Expression_Constant_System_Object_ )

In addition a new unit test was added to showcase this situation.

When an argument default value is gathered from a method definition be sure to pass also its associated argument when creating the Expression.Constant.

This avoids a problem which happens when the argument is null: in this case the type of the argument will be System.Object and that will fail when binding the arguments a method defining a different type (for example string)

In addition a new unit test was added to showcase this situation.
@davideicardi davideicardi changed the title Handle default parameters explicitly Handle default parameters type explicitly Feb 3, 2019
@davideicardi
Copy link
Member

davideicardi commented Feb 3, 2019

Thanks @weltall , just merged! See v2.3.1

@davideicardi davideicardi merged commit 273e373 into dynamicexpresso:master Feb 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants