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.

Sign up for GitHub

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

Fix NumberParsing for double with exponent #532

Merged
merged 3 commits into from
Jul 12, 2021
Merged

Fix NumberParsing for double with exponent #532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d0c553
Fix NumberParsing dor double with exponent
StefH Jul 11, 2021
3d8f35c
.
StefH Jul 11, 2021
1970a0d
e
StefH Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 30 additions & 80 deletions src/System.Linq.Dynamic.Core/Parser/ExpressionParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public class ExpressionParser
private readonly MethodFinder _methodFinder;
private readonly IKeywordsHelper _keywordsHelper;
private readonly TextParser _textParser;
private readonly NumberParser _numberParser;
private readonly IExpressionHelper _expressionHelper;
private readonly ITypeFinder _typeFinder;
private readonly ITypeConverterFactory _typeConverterFactory;
Expand Down Expand Up @@ -83,6 +84,7 @@ public ExpressionParser([CanBeNull] ParameterExpression[] parameters, [NotNull]

_keywordsHelper = new KeywordsHelper(_parsingConfig);
_textParser = new TextParser(_parsingConfig, expression);
_numberParser = new NumberParser(parsingConfig);
_methodFinder = new MethodFinder(_parsingConfig);
_expressionHelper = new ExpressionHelper(_parsingConfig);
_typeFinder = new TypeFinder(_parsingConfig, _keywordsHelper);
Expand Down Expand Up @@ -876,16 +878,14 @@ Expression ParseIntegerLiteral()
if (!string.IsNullOrEmpty(qualifier))
{
if (qualifier == "L" || qualifier == "l")
{
return ConstantExpressionHelper.CreateLiteral(value, text);
}

if (qualifier == "F" || qualifier == "f")
return TryParseAsFloat(text, qualifier[0]);

if (qualifier == "D" || qualifier == "d")
return TryParseAsDouble(text, qualifier[0]);

if (qualifier == "M" || qualifier == "m")
return TryParseAsDecimal(text, qualifier[0]);
if (qualifier == "F" || qualifier == "f" || qualifier == "D" || qualifier == "d" || qualifier == "M" || qualifier == "m")
{
return ParseRealLiteral(text, qualifier[0], false);
}

throw ParseError(Res.MinusCannotBeAppliedToUnsignedInteger);
}
Expand All @@ -904,54 +904,40 @@ Expression ParseRealLiteral()
_textParser.ValidateToken(TokenId.RealLiteral);

string text = _textParser.CurrentToken.Text;
char qualifier = text[text.Length - 1];

_textParser.NextToken();
return TryParseAsFloat(text, qualifier);
}

Expression TryParseAsFloat(string text, char qualifier)
{
if (qualifier == 'F' || qualifier == 'f')
{
if (float.TryParse(text.Substring(0, text.Length - 1), NumberStyles.Float, _parsingConfig.NumberParseCulture, out float f))
{
return ConstantExpressionHelper.CreateLiteral(f, text);
}
}

// not possible to find float qualifier, so try to parse as double
return TryParseAsDecimal(text, qualifier);
return ParseRealLiteral(text, text[text.Length - 1], true);
}

Expression TryParseAsDecimal(string text, char qualifier)
Expression ParseRealLiteral(string text, char qualifier, bool stripQualifier)
{
if (qualifier == 'M' || qualifier == 'm')
object o;
switch (qualifier)
{
if (decimal.TryParse(text.Substring(0, text.Length - 1), NumberStyles.Number, _parsingConfig.NumberParseCulture, out decimal d))
{
return ConstantExpressionHelper.CreateLiteral(d, text);
}
}
case 'f':
case 'F':
o = _numberParser.ParseNumber(stripQualifier ? text.Substring(0, text.Length - 1) : text, typeof(float));
break;

// not possible to find float qualifier, so try to parse as double
return TryParseAsDouble(text, qualifier);
}
case 'm':
case 'M':
o = _numberParser.ParseNumber(stripQualifier ? text.Substring(0, text.Length - 1) : text, typeof(decimal));
break;

Expression TryParseAsDouble(string text, char qualifier)
{
double d;
if (qualifier == 'D' || qualifier == 'd')
{
if (double.TryParse(text.Substring(0, text.Length - 1), NumberStyles.Number, _parsingConfig.NumberParseCulture, out d))
{
return ConstantExpressionHelper.CreateLiteral(d, text);
}
case 'd':
case 'D':
o = _numberParser.ParseNumber(stripQualifier ? text.Substring(0, text.Length - 1) : text, typeof(double));
break;

default:
o = _numberParser.ParseNumber(text, typeof(double));
break;
}

if (double.TryParse(text, NumberStyles.Number, _parsingConfig.NumberParseCulture, out d))
if (o != null)
{
return ConstantExpressionHelper.CreateLiteral(d, text);
return ConstantExpressionHelper.CreateLiteral(o, text);
}

throw ParseError(Res.InvalidRealLiteral, text);
Expand Down Expand Up @@ -1045,42 +1031,6 @@ Expression ParseIdentifier()
return expr;
}

//// This could be enum like "MyEnum.Value1"
//if (_textParser.CurrentToken.Id == TokenId.Identifier)
//{
// var parts = new List<string> { _textParser.CurrentToken.Text };

// _textParser.NextToken();
// _textParser.ValidateToken(TokenId.Dot, Res.DotExpected);
// while (_textParser.CurrentToken.Id == TokenId.Dot || _textParser.CurrentToken.Id == TokenId.Plus)
// {
// parts.Add(_textParser.CurrentToken.Text);

// _textParser.NextToken();
// _textParser.ValidateToken(TokenId.Identifier, Res.IdentifierExpected);

// parts.Add(_textParser.CurrentToken.Text);

// _textParser.NextToken();
// }

// var enumTypeAsString = string.Join("", parts.Take(parts.Count - 2).ToArray());
// var enumType = _typeFinder.FindTypeByName(enumTypeAsString, null, true);
// if (enumType == null)
// {
// throw ParseError(_textParser.CurrentToken.Pos, Res.EnumTypeNotFound, enumTypeAsString);
// }

// string enumValue = parts.Last();
// var @enum = TypeHelper.ParseEnum(enumValue, enumType);
// if (@enum == null)
// {
// throw ParseError(_textParser.CurrentToken.Pos, Res.EnumValueNotDefined, enumValue, enumTypeAsString);
// }

// return Expression.Constant(@enum);
//}

if (_it != null)
{
return ParseMemberAccess(null, _it);
Expand Down
5 changes: 4 additions & 1 deletion src/System.Linq.Dynamic.Core/Parser/ExpressionPromoter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ public virtual Expression Promote(Expression expr, Type type, bool exact, bool c
break;

case TypeCode.Double:
if (target == typeof(decimal) || target == typeof(double)) value = _numberParser.ParseNumber(text, target);
if (target == typeof(decimal) || target == typeof(double))
{
value = _numberParser.ParseNumber(text, target);
}
break;

case TypeCode.String:
Expand Down
67 changes: 41 additions & 26 deletions src/System.Linq.Dynamic.Core/Parser/NumberParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,34 @@
namespace System.Linq.Dynamic.Core.Parser
using System.Globalization;
using JetBrains.Annotations;

namespace System.Linq.Dynamic.Core.Parser
{
/// <summary>
/// NumberParser
/// </summary>
public class NumberParser
{
private readonly ParsingConfig _config;
private readonly CultureInfo _culture;

/// <summary>
/// Initializes a new instance of the <see cref="NumberParser"/> class.
/// </summary>
/// <param name="config">The ParsingConfig.</param>
public NumberParser(ParsingConfig config)
public NumberParser([CanBeNull] ParsingConfig config)
{
_culture = config?.NumberParseCulture ?? CultureInfo.InvariantCulture;
}

/// <summary>
/// Tries to parse the number (text) into the specified type.
/// </summary>
/// <param name="text">The text.</param>
/// <param name="type">The type.</param>
/// <param name="result">The result.</param>
public bool TryParseNumber(string text, Type type, out object result)
{
_config = config;
result = ParseNumber(text, type);
return type != null;
}

/// <summary>
Expand All @@ -29,73 +44,73 @@ public object ParseNumber(string text, Type type)
switch (Type.GetTypeCode(TypeHelper.GetNonNullableType(type)))
{
case TypeCode.SByte:
return sbyte.Parse(text, _config.NumberParseCulture);
return sbyte.Parse(text, _culture);
case TypeCode.Byte:
return byte.Parse(text, _config.NumberParseCulture);
return byte.Parse(text, _culture);
case TypeCode.Int16:
return short.Parse(text, _config.NumberParseCulture);
return short.Parse(text, _culture);
case TypeCode.UInt16:
return ushort.Parse(text, _config.NumberParseCulture);
return ushort.Parse(text, _culture);
case TypeCode.Int32:
return int.Parse(text, _config.NumberParseCulture);
return int.Parse(text, _culture);
case TypeCode.UInt32:
return uint.Parse(text, _config.NumberParseCulture);
return uint.Parse(text, _culture);
case TypeCode.Int64:
return long.Parse(text, _config.NumberParseCulture);
return long.Parse(text, _culture);
case TypeCode.UInt64:
return ulong.Parse(text, _config.NumberParseCulture);
return ulong.Parse(text, _culture);
case TypeCode.Single:
return float.Parse(text, _config.NumberParseCulture);
return float.Parse(text, _culture);
case TypeCode.Double:
return double.Parse(text, _config.NumberParseCulture);
return double.Parse(text, _culture);
case TypeCode.Decimal:
return decimal.Parse(text, _config.NumberParseCulture);
return decimal.Parse(text, _culture);
}
#else
var tp = TypeHelper.GetNonNullableType(type);
if (tp == typeof(sbyte))
{
return sbyte.Parse(text, _config.NumberParseCulture);
return sbyte.Parse(text, _culture);
}
if (tp == typeof(byte))
{
return byte.Parse(text, _config.NumberParseCulture);
return byte.Parse(text, _culture);
}
if (tp == typeof(short))
{
return short.Parse(text, _config.NumberParseCulture);
return short.Parse(text, _culture);
}
if (tp == typeof(ushort))
{
return ushort.Parse(text, _config.NumberParseCulture);
return ushort.Parse(text, _culture);
}
if (tp == typeof(int))
{
return int.Parse(text, _config.NumberParseCulture);
return int.Parse(text, _culture);
}
if (tp == typeof(uint))
{
return uint.Parse(text, _config.NumberParseCulture);
return uint.Parse(text, _culture);
}
if (tp == typeof(long))
{
return long.Parse(text, _config.NumberParseCulture);
return long.Parse(text, _culture);
}
if (tp == typeof(ulong))
{
return ulong.Parse(text, _config.NumberParseCulture);
return ulong.Parse(text, _culture);
}
if (tp == typeof(float))
{
return float.Parse(text, _config.NumberParseCulture);
return float.Parse(text, _culture);
}
if (tp == typeof(double))
{
return double.Parse(text, _config.NumberParseCulture);
return double.Parse(text, _culture);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I think the default will work great for us. FWIW, the default value seems to be NumberStyles.Float | NumberStyles.AllowThousands. According to the somewhat confusing NumberStyles docs, this actually does exclude one flag that was enabled prior to this change, which was NumberStyles.AllowTrailingSign.

This flag is really strange, it lets me specify the sign after the number like in this fiddle. I haven't seen any actual use cases of this in the wild, and I don't know if the parser even supports it. I definitely don't need trailing signs, just wanting to make sure that we aren't regressing this edge case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it, and it looks like trailing signs are already rejected elsewhere in the parser. So, this isn't a regression at all, we can ignore the trailing sign flag 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

}
if (tp == typeof(decimal))
{
return decimal.Parse(text, _config.NumberParseCulture);
return decimal.Parse(text, _culture);
}
#endif
}
Expand Down
Loading