-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix handling of enum default values in RDF and RDG #63086
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1801,7 +1801,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres | |
| ? Expression.IfThen(TempSourceStringNotNullExpr, tryParseExpression) | ||
| : Expression.IfThenElse(TempSourceStringNotNullExpr, tryParseExpression, | ||
| Expression.Assign(argument, | ||
| Expression.Constant(parameter.DefaultValue, parameter.ParameterType))); | ||
| CreateDefaultValueExpression(parameter.DefaultValue, parameter.ParameterType))); | ||
|
|
||
| var loopExit = Expression.Label(); | ||
|
|
||
|
|
@@ -1963,7 +1963,7 @@ private static Expression BindParameterFromExpression( | |
| return Expression.Block( | ||
| Expression.Condition(Expression.NotEqual(valueExpression, Expression.Constant(null)), | ||
| valueExpression, | ||
| Expression.Convert(Expression.Constant(parameter.DefaultValue), parameter.ParameterType))); | ||
| Expression.Convert(CreateDefaultValueExpression(parameter.DefaultValue, parameter.ParameterType), parameter.ParameterType))); | ||
| } | ||
|
|
||
| private static Expression BindParameterFromProperty(ParameterInfo parameter, MemberExpression property, PropertyInfo itemProperty, string key, RequestDelegateFactoryContext factoryContext, string source) | ||
|
|
@@ -1981,6 +1981,34 @@ private static Expression BindParameterFromProperty(ParameterInfo parameter, Mem | |
| type == typeof(StringValues?) ? typeof(StringValues?) : | ||
| null; | ||
|
|
||
| private static Expression CreateDefaultValueExpression(object? defaultValue, Type parameterType) | ||
| { | ||
| if (defaultValue is null) | ||
| { | ||
| return Expression.Constant(null, parameterType); | ||
| } | ||
|
|
||
| var underlyingType = Nullable.GetUnderlyingType(parameterType); | ||
| var isNullable = underlyingType != null; | ||
| var targetType = isNullable ? underlyingType! : parameterType; | ||
| var converted = defaultValue; | ||
|
|
||
| // Apply a conversion for scenarios where the default value's type | ||
| // doesn't match the parameter type | ||
| if (targetType.IsEnum && defaultValue.GetType() != targetType) | ||
| { | ||
| converted = Enum.ToObject(targetType, defaultValue); | ||
| } | ||
| else if (!targetType.IsAssignableFrom(defaultValue.GetType())) | ||
| { | ||
| converted = Convert.ChangeType(defaultValue, targetType, CultureInfo.InvariantCulture); | ||
| } | ||
|
|
||
| var constant = Expression.Constant(converted, targetType); | ||
| // Cast nullable types as needed | ||
| return isNullable ? Expression.Convert(constant, parameterType) : constant; | ||
| } | ||
|
|
||
| private static Expression BindParameterFromRouteValueOrQueryString(ParameterInfo parameter, string key, RequestDelegateFactoryContext factoryContext) | ||
| { | ||
| var routeValue = GetValueFromProperty(RouteValuesExpr, RouteValuesIndexerProperty, key); | ||
|
|
@@ -2358,7 +2386,7 @@ private static Expression BindParameterFromBody(ParameterInfo parameter, bool al | |
| { | ||
| // Convert(bodyValue ?? SomeDefault, Todo) | ||
| return Expression.Convert( | ||
| Expression.Coalesce(BodyValueExpr, Expression.Constant(parameter.DefaultValue)), | ||
| Expression.Coalesce(BodyValueExpr, CreateDefaultValueExpression(parameter.DefaultValue, typeof(object))), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| parameter.ParameterType); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -154,56 +154,66 @@ public static object[][] DefaultValues | |||||||
| { | ||||||||
| get | ||||||||
| { | ||||||||
| return new[] | ||||||||
| { | ||||||||
| new object[] { "string?", "default", default(string), true }, | ||||||||
| new object[] { "string", "\"test\"", "test", true }, | ||||||||
| new object[] { "string", "\"a\" + \"b\"", "ab", true }, | ||||||||
| new object[] { "DateOnly?", "default", default(DateOnly?), false }, | ||||||||
| new object[] { "bool", "default", default(bool), true }, | ||||||||
| new object[] { "bool", "false", false, true }, | ||||||||
| new object[] { "bool", "true", true, true}, | ||||||||
| new object[] { "System.Threading.CancellationToken", "default", default(CancellationToken), false }, | ||||||||
| new object[] { "Todo?", "default", default(Todo), false }, | ||||||||
| new object[] { "char", "\'a\'", 'a', true }, | ||||||||
| new object[] { "int", "default", 0, true }, | ||||||||
| new object[] { "int", "1234", 1234, true }, | ||||||||
| new object[] { "int", "1234 * 4", 1234 * 4, true }, | ||||||||
| new object[] { "double", "1.0", 1.0, true }, | ||||||||
| new object[] { "double", "double.NaN", double.NaN, true }, | ||||||||
| new object[] { "double", "double.PositiveInfinity", double.PositiveInfinity, true }, | ||||||||
| new object[] { "double", "double.NegativeInfinity", double.NegativeInfinity, true }, | ||||||||
| new object[] { "double", "double.E", double.E, true }, | ||||||||
| new object[] { "double", "double.Epsilon", double.Epsilon, true }, | ||||||||
| new object[] { "double", "double.NegativeZero", double.NegativeZero, true }, | ||||||||
| new object[] { "double", "double.MaxValue", double.MaxValue, true }, | ||||||||
| new object[] { "double", "double.MinValue", double.MinValue, true }, | ||||||||
| new object[] { "double", "double.Pi", double.Pi, true }, | ||||||||
| new object[] { "double", "double.Tau", double.Tau, true }, | ||||||||
| new object[] { "float", "float.NaN", float.NaN, true }, | ||||||||
| new object[] { "float", "float.PositiveInfinity", float.PositiveInfinity, true }, | ||||||||
| new object[] { "float", "float.NegativeInfinity", float.NegativeInfinity, true }, | ||||||||
| new object[] { "float", "float.E", float.E, true }, | ||||||||
| new object[] { "float", "float.Epsilon", float.Epsilon, true }, | ||||||||
| new object[] { "float", "float.NegativeZero", float.NegativeZero, true }, | ||||||||
| new object[] { "float", "float.MaxValue", float.MaxValue, true }, | ||||||||
| new object[] { "float", "float.MinValue", float.MinValue, true }, | ||||||||
| new object[] { "float", "float.Pi", float.Pi, true }, | ||||||||
| new object[] { "float", "float.Tau", float.Tau, true }, | ||||||||
| new object[] {"decimal", "decimal.MaxValue", decimal.MaxValue, true }, | ||||||||
| new object[] {"decimal", "decimal.MinusOne", decimal.MinusOne, true }, | ||||||||
| new object[] {"decimal", "decimal.MinValue", decimal.MinValue, true }, | ||||||||
| new object[] {"decimal", "decimal.One", decimal.One, true }, | ||||||||
| new object[] {"decimal", "decimal.Zero", decimal.Zero, true }, | ||||||||
| new object[] {"long", "long.MaxValue", long.MaxValue, true }, | ||||||||
| new object[] {"long", "long.MinValue", long.MinValue, true }, | ||||||||
| new object[] {"short", "short.MaxValue", short.MaxValue, true }, | ||||||||
| new object[] {"short", "short.MinValue", short.MinValue, true }, | ||||||||
| new object[] {"ulong", "ulong.MaxValue", ulong.MaxValue, true }, | ||||||||
| new object[] {"ulong", "ulong.MinValue", ulong.MinValue, true }, | ||||||||
| new object[] {"ushort", "ushort.MaxValue", ushort.MaxValue, true }, | ||||||||
| new object[] {"ushort", "ushort.MinValue", ushort.MinValue, true }, | ||||||||
| }; | ||||||||
| return | ||||||||
| [ | ||||||||
| ["string?", "default", default(string), true], | ||||||||
| ["string", "\"test\"", "test", true], | ||||||||
| ["string", "\"a\" + \"b\"", "ab", true], | ||||||||
| ["DateOnly?", "default", default(DateOnly?), false], | ||||||||
| ["bool", "default", default(bool), true], | ||||||||
| ["bool", "false", false, true], | ||||||||
| ["bool", "true", true, true], | ||||||||
| ["System.Threading.CancellationToken", "default", default(CancellationToken), false], | ||||||||
| ["Todo?", "default", default(Todo), false], | ||||||||
| ["char", "\'a\'", 'a', true], | ||||||||
| ["int", "default", 0, true], | ||||||||
| ["int", "1234", 1234, true], | ||||||||
| ["int", "1234 * 4", 1234 * 4, true], | ||||||||
| ["double", "1.0", 1.0, true], | ||||||||
| ["double", "double.NaN", double.NaN, true], | ||||||||
| ["double", "double.PositiveInfinity", double.PositiveInfinity, true], | ||||||||
| ["double", "double.NegativeInfinity", double.NegativeInfinity, true], | ||||||||
| ["double", "double.E", double.E, true], | ||||||||
| ["double", "double.Epsilon", double.Epsilon, true], | ||||||||
| ["double", "double.NegativeZero", double.NegativeZero, true], | ||||||||
| ["double", "double.MaxValue", double.MaxValue, true], | ||||||||
| ["double", "double.MinValue", double.MinValue, true], | ||||||||
| ["double", "double.Pi", double.Pi, true], | ||||||||
| ["double", "double.Tau", double.Tau, true], | ||||||||
| ["float", "float.NaN", float.NaN, true], | ||||||||
| ["float", "float.PositiveInfinity", float.PositiveInfinity, true], | ||||||||
| ["float", "float.NegativeInfinity", float.NegativeInfinity, true], | ||||||||
| ["float", "float.E", float.E, true], | ||||||||
| ["float", "float.Epsilon", float.Epsilon, true], | ||||||||
| ["float", "float.NegativeZero", float.NegativeZero, true], | ||||||||
| ["float", "float.MaxValue", float.MaxValue, true], | ||||||||
| ["float", "float.MinValue", float.MinValue, true], | ||||||||
| ["float", "float.Pi", float.Pi, true], | ||||||||
| ["float", "float.Tau", float.Tau, true], | ||||||||
| ["decimal", "decimal.MaxValue", decimal.MaxValue, true], | ||||||||
| ["decimal", "decimal.MinusOne", decimal.MinusOne, true], | ||||||||
| ["decimal", "decimal.MinValue", decimal.MinValue, true], | ||||||||
| ["decimal", "decimal.One", decimal.One, true], | ||||||||
| ["decimal", "decimal.Zero", decimal.Zero, true], | ||||||||
| ["long", "long.MaxValue", long.MaxValue, true], | ||||||||
| ["long", "long.MinValue", long.MinValue, true], | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Non-enum types don't match There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto the about about casting. |
||||||||
| ["long", "(long)3.14", (long)3, true], | ||||||||
| ["short", "short.MaxValue", short.MaxValue, true], | ||||||||
| ["short", "short.MinValue", short.MinValue, true], | ||||||||
| ["ulong", "ulong.MaxValue", ulong.MaxValue, true], | ||||||||
| ["ulong", "ulong.MinValue", ulong.MinValue, true], | ||||||||
| ["ushort", "ushort.MaxValue", ushort.MaxValue, true], | ||||||||
| ["ushort", "ushort.MinValue", ushort.MinValue, true], | ||||||||
| ["TodoStatus", "TodoStatus.Done", TodoStatus.Done, true], | ||||||||
| ["TodoStatus", "TodoStatus.InProgress", TodoStatus.InProgress, true], | ||||||||
| ["TodoStatus", "TodoStatus.NotDone", TodoStatus.NotDone, true], | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Enum types don't match There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The compiler technically doesn't allow this syntax without an explicit conversion via |
||||||||
| ["TodoStatus", "(TodoStatus)1", TodoStatus.Done, true], | ||||||||
| ["MyEnum", "MyEnum.ValueA", MyEnum.ValueA, true], | ||||||||
| ["MyEnum", "MyEnum.ValueB", MyEnum.ValueB, true], | ||||||||
| // Test nullable enum values | ||||||||
| ["TodoStatus?", "TodoStatus.Done", (TodoStatus?)TodoStatus.Done, false], | ||||||||
| ["TodoStatus?", "default", default(TodoStatus?), false] | ||||||||
| ]; | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
|
|
||||||||
Uh oh!
There was an error while loading. Please reload this page.