-
-
Notifications
You must be signed in to change notification settings - Fork 231
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
Incorrect Handling of Qualifiers in ConstantExpressionHelper #823
Conversation
this PR is related to this issue: |
@@ -152,5 +152,31 @@ public void NumberParser_ParseIntegerLiteral(string text, double expected) | |||
// Assert | |||
result.Value.Should().Be(expected); | |||
} | |||
|
|||
[Theory] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to split into multiple tests to make sure that the value is really the correct type:
[Theory]
[InlineData("42", 'm', 42m)]
[InlineData("-42", 'm', -42m)]
[InlineData("42m", 'm', 42m)]
[InlineData("-42m", 'm', -42m)]
public void NumberParser_ParseDecimalLiteral(string text, char qualifier, decimal expected)
{
// Act
var result = (ConstantExpression) new NumberParser(_parsingConfig).ParseRealLiteral(text, qualifier, true);
// Assert
result.Value.Should().Be(expected);
}
[Theory]
[InlineData("42", 'd', 42d)]
[InlineData("-42", 'd', -42d)]
[InlineData("42m", 'd', 42d)]
[InlineData("-42m", 'd', -42d)]
public void NumberParser_ParseDoubleLiteral(string text, char qualifier, double expected)
{
// Act
var result = (ConstantExpression) new NumberParser(_parsingConfig).ParseRealLiteral(text, qualifier, true);
// Assert
result.Value.Should().Be(expected);
}
[Theory]
[InlineData("42", 'f', 42f)]
[InlineData("-42", 'f', -42f)]
[InlineData("42m", 'f', 42f)]
[InlineData("-42m", 'f', -42f)]
public void NumberParser_ParseFloatLiteral(string text, char qualifier, float expected)
{
// Act
var result = (ConstantExpression) new NumberParser(_parsingConfig).ParseRealLiteral(text, qualifier, true);
// Assert
result.Value.Should().Be(expected);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @StefH if I do that we will have the sonar complain:
https://rules.sonarsource.com/csharp/RSPEC-4144/
Please let me know if you'd like me to do it anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please implement it
@RenanCarlosPereira |
yeah, I forgot to open a branch to it, doing it now 😅😅 I splited the tests, all passing |
That is the new PR, I will abandon this one: |
OK, You can indeed abandon this |
Description:
There is an issue in the
ExpressionPromoter
class where the qualifier in the numeric literal expressions is not correctly handled, leading to incorrect type promotions. This is due to the way qualifiers are stored and retrieved from theConstantExpressionHelper
.Steps to Reproduce:
ParsingConfig
.NumberParser
to parse a numeric literal with a qualifier.ExpressionPromoter
.Example 1:
Example 2:
Expected Behavior:
The
Promote
method should correctly handle the qualifier and convert the numeric literal to the specified target type.Actual Behavior:
The promotion fails, and the conversion does not occur as expected due to the incorrect handling of the qualifier in the
ConstantExpressionHelper
.Additional Information:
This issue affects applications that rely on dynamic LINQ queries where numeric-type promotions with qualifiers are necessary.
Proposed Solution:
Ensure that the
CreateLiteral
inConstantExpressionHelper
correctly handles qualifiers without the prefix when creating and retrieving constant expressions.