From 1d2577d1679c52918b44b4b8313212e1b1e442fd Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Mon, 19 Aug 2024 14:20:50 +1000 Subject: [PATCH 01/11] Migrate to PEG parser. Introduce boolean operators and constants. --- .../Primitives/MathOperator.shared.cs | 7 +- .../MathExpressionConverterTests.cs | 47 +- .../MathExpression.shared.cs | 533 +++++++++++++----- .../MathExpressionConverter.shared.cs | 6 +- .../MultiMathExpressionConverter.shared.cs | 16 +- 5 files changed, 438 insertions(+), 171 deletions(-) diff --git a/src/CommunityToolkit.Maui.Core/Primitives/MathOperator.shared.cs b/src/CommunityToolkit.Maui.Core/Primitives/MathOperator.shared.cs index f2b137f5b1..ba7faedeef 100644 --- a/src/CommunityToolkit.Maui.Core/Primitives/MathOperator.shared.cs +++ b/src/CommunityToolkit.Maui.Core/Primitives/MathOperator.shared.cs @@ -27,17 +27,14 @@ public sealed class MathOperator /// /// Name /// Number of Numerals - /// Math Operator Preference /// Calculation Function public MathOperator( string name, int numericCount, - MathOperatorPrecedence precedence, - Func calculateFunc) + Func calculateFunc) { Name = name; CalculateFunc = calculateFunc; - Precedence = precedence; NumericCount = numericCount; } @@ -59,5 +56,5 @@ public MathOperator( /// /// Calculation Function /// - public Func CalculateFunc { get; } + public Func CalculateFunc { get; } } \ No newline at end of file diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index 771c9231ff..d2bd9e8dd8 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -30,7 +30,30 @@ public void MathExpressionConverter_ReturnsCorrectResult(string expression, doub var convertFromResult = mathExpressionConverter.ConvertFrom(x, expression); Assert.True(Math.Abs((double)convertResult - expectedResult) < tolerance); - Assert.True(Math.Abs(convertFromResult - expectedResult) < tolerance); + Assert.True(Math.Abs((double)convertFromResult - expectedResult) < tolerance); + } + + [Theory] + [InlineData("3 < x", 2d, false)] + [InlineData("x > 3", 2d, false)] + [InlineData("3 < x == x > 3", 2d, true)] + [InlineData("3 <= x != 3 >= x", 2d, true)] + [InlineData("x >= 1", 2d, true)] + [InlineData("x <= 3", 2d, true)] + [InlineData("x >= 1 && (x <= 3 || x >= 0)", 2d, true)] + [InlineData("true", 2d, true)] + [InlineData("false", 2d, false)] + [InlineData("-x > 2", 3d, false)] + [InlineData("!!! (---x > 2)", 3d, true)] + public void MathExpressionConverter_ReturnsCorrectBooleanResult(string expression, double x, bool expectedResult) + { + var mathExpressionConverter = new MathExpressionConverter(); + + var convertResult = ((ICommunityToolkitValueConverter)mathExpressionConverter).Convert(x, mathExpressionTargetType, expression, cultureInfo) ?? throw new NullReferenceException(); + var convertFromResult = mathExpressionConverter.ConvertFrom(x, expression); + + Assert.True((bool)convertResult == expectedResult); + Assert.True((bool)convertFromResult == expectedResult); } [Theory] @@ -47,6 +70,22 @@ public void MathExpressionConverter_WithMultiplyVariable_ReturnsCorrectResult(st Assert.True(Math.Abs((double)result - expectedResult) < tolerance); } + [Theory] + [InlineData("x == x1", new object?[] { 2d, 2d }, true)] + [InlineData("x == x1", new object?[] { 2d, null }, false)] + [InlineData("x == x1", new object?[] { null, 2d}, false)] + [InlineData("x == x1", new object?[] { null, null }, true)] + [InlineData("(x ? x1 : x2) == null", new object?[] { true, null, 2d }, true)] + public void MathExpressionConverter_WithMultiplyVariable_ReturnsCorrectBooleanResult(string expression, object[] variables, bool expectedResult) + { + var mathExpressionConverter = new MultiMathExpressionConverter(); + + object? result = mathExpressionConverter.Convert(variables, mathExpressionTargetType, expression); + + Assert.True(result is not null); + Assert.Equal(expectedResult, result); + } + [Theory] [InlineData("1 + 3 + 5 + (3 - 2))")] [InlineData("1 + 2) + (9")] @@ -74,7 +113,7 @@ public void MultiMathExpressionConverterInvalidParameterThrowsArgumentException( public void MultiMathExpressionConverterInvalidValuesReturnsNull() { var mathExpressionConverter = new MultiMathExpressionConverter(); - var result = mathExpressionConverter.Convert([0d, null], mathExpressionTargetType, "x", cultureInfo); + var result = mathExpressionConverter.Convert([0d, null], mathExpressionTargetType, "x + x1", cultureInfo); result.Should().BeNull(); } @@ -85,9 +124,9 @@ public void MathExpressionConverterNullInputTest() Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(0.0, null, "x", null)); Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(0.0, null, null, null)); #pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. - Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), "x", null)); + //Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), "x", null)); Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), null, null)); - Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(null, typeof(bool), null, null)); + //Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(null, typeof(bool), null, null)); } [Fact] diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index 2d1840407c..a0b4876167 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -4,6 +4,26 @@ namespace CommunityToolkit.Maui.Converters; +enum MathTokenType +{ + Value, + Operator, +}; + +class MathToken +{ + public MathToken(MathTokenType type, string text, object? value) + { + Type = type; + Text = text; + Value = value; + } + + public MathTokenType Type { get; } + public string Text { get; } + public object? Value { get; } +}; + sealed partial class MathExpression { const NumberStyles numberStyle = NumberStyles.Float | NumberStyles.AllowThousands; @@ -11,87 +31,152 @@ sealed partial class MathExpression static readonly IFormatProvider formatProvider = new CultureInfo("en-US"); readonly IReadOnlyList operators; - readonly IReadOnlyList arguments; + readonly IReadOnlyList arguments; + + internal static bool __bool(object? value) + { + if (value is null) + { + return false; + } + + if (value is bool boolValue) + { + return boolValue; + } + + if (value is double doubleValue) + { + return doubleValue != 0 && doubleValue != double.NaN; + } - internal MathExpression(string expression, IEnumerable? arguments = null) + if (value is string stringValue) + { + return !string.IsNullOrEmpty(stringValue); + } + + try + { + return Convert.ToBoolean(value); + } + catch (Exception ex) + { + } + + return !string.IsNullOrEmpty(value.ToString()); + } + + internal MathExpression(string expression, IEnumerable? arguments = null) { ArgumentException.ThrowIfNullOrEmpty(expression, "Expression can't be null or empty."); var argumentList = arguments?.ToList() ?? []; - Expression = expression.ToLower(); + expr = expression.ToLower(); var operators = new List { - new ("+", 2, MathOperatorPrecedence.Low, x => x[0] + x[1]), - new ("-", 2, MathOperatorPrecedence.Low, x => x[0] - x[1]), - new ("*", 2, MathOperatorPrecedence.Medium, x => x[0] * x[1]), - new ("/", 2, MathOperatorPrecedence.Medium, x => x[0] / x[1]), - new ("%", 2, MathOperatorPrecedence.Medium, x => x[0] % x[1]), - new ("abs", 1, MathOperatorPrecedence.Medium, x => Math.Abs(x[0])), - new ("acos", 1, MathOperatorPrecedence.Medium, x => Math.Acos(x[0])), - new ("asin", 1, MathOperatorPrecedence.Medium, x => Math.Asin(x[0])), - new ("atan", 1, MathOperatorPrecedence.Medium, x => Math.Atan(x[0])), - new ("atan2", 2, MathOperatorPrecedence.Medium, x => Math.Atan2(x[0], x[1])), - new ("ceiling", 1, MathOperatorPrecedence.Medium, x => Math.Ceiling(x[0])), - new ("cos", 1, MathOperatorPrecedence.Medium, x => Math.Cos(x[0])), - new ("cosh", 1, MathOperatorPrecedence.Medium, x => Math.Cosh(x[0])), - new ("exp", 1, MathOperatorPrecedence.Medium, x => Math.Exp(x[0])), - new ("floor", 1, MathOperatorPrecedence.Medium, x => Math.Floor(x[0])), - new ("ieeeremainder", 2, MathOperatorPrecedence.Medium, x => Math.IEEERemainder(x[0], x[1])), - new ("log", 2, MathOperatorPrecedence.Medium, x => Math.Log(x[0], x[1])), - new ("log10", 1, MathOperatorPrecedence.Medium, x => Math.Log10(x[0])), - new ("max", 2, MathOperatorPrecedence.Medium, x => Math.Max(x[0], x[1])), - new ("min", 2, MathOperatorPrecedence.Medium, x => Math.Min(x[0], x[1])), - new ("pow", 2, MathOperatorPrecedence.Medium, x => Math.Pow(x[0], x[1])), - new ("round", 2, MathOperatorPrecedence.Medium, x => Math.Round(x[0], Convert.ToInt32(x[1]))), - new ("sign", 1, MathOperatorPrecedence.Medium, x => Math.Sign(x[0])), - new ("sin", 1, MathOperatorPrecedence.Medium, x => Math.Sin(x[0])), - new ("sinh", 1, MathOperatorPrecedence.Medium, x => Math.Sinh(x[0])), - new ("sqrt", 1, MathOperatorPrecedence.Medium, x => Math.Sqrt(x[0])), - new ("tan", 1, MathOperatorPrecedence.Medium, x => Math.Tan(x[0])), - new ("tanh", 1, MathOperatorPrecedence.Medium, x => Math.Tanh(x[0])), - new ("truncate", 1, MathOperatorPrecedence.Medium, x => Math.Truncate(x[0])), - new ("^", 2, MathOperatorPrecedence.High, x => Math.Pow(x[0], x[1])), - new ("pi", 0, MathOperatorPrecedence.Constant, _ => Math.PI), - new ("e", 0, MathOperatorPrecedence.Constant, _ => Math.E), + new ("+", 2, x => Convert.ToDouble(x[0]) + Convert.ToDouble(x[1])), + new ("-", 2, x => Convert.ToDouble(x[0]) - Convert.ToDouble(x[1])), + new ("*", 2, x => Convert.ToDouble(x[0]) * Convert.ToDouble(x[1])), + new ("/", 2, x => Convert.ToDouble(x[0]) / Convert.ToDouble(x[1])), + new ("%", 2, x => Convert.ToDouble(x[0]) % Convert.ToDouble(x[1])), + + new ("&&", 2, x => __bool(x[0]) ? x[1] : x[0]), + new ("||", 2, x => __bool(x[0]) ? x[0] : x[1]), + + new ("==", 2, x => object.Equals(x[0], x[1])), + new ("!=", 2, x => !object.Equals(x[0], x[1])), + + new (">=", 2, x => Convert.ToDouble(x[0]) >= Convert.ToDouble(x[1])), + new (">", 2, x => Convert.ToDouble(x[0]) > Convert.ToDouble(x[1])), + new ("<=", 2, x => Convert.ToDouble(x[0]) <= Convert.ToDouble(x[1])), + new ("<", 2, x => Convert.ToDouble(x[0]) < Convert.ToDouble(x[1])), + new ("neg", 1, x => -Convert.ToDouble(x[0])), + new ("not", 1, x => !__bool(x[0])), + new ("if", 3, x => __bool(x[0]) ? x[1] : x[2]), + + new ("abs", 1, x => Math.Abs(Convert.ToDouble(x[0]))), + new ("acos", 1, x => Math.Acos(Convert.ToDouble(x[0]))), + new ("asin", 1, x => Math.Asin(Convert.ToDouble(x[0]))), + new ("atan", 1, x => Math.Atan(Convert.ToDouble(x[0]))), + new ("atan2", 2, x => Math.Atan2(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("ceiling", 1, x => Math.Ceiling(Convert.ToDouble(x[0]))), + new ("cos", 1, x => Math.Cos(Convert.ToDouble(x[0]))), + new ("cosh", 1, x => Math.Cosh(Convert.ToDouble(x[0]))), + new ("exp", 1, x => Math.Exp(Convert.ToDouble(x[0]))), + new ("floor", 1, x => Math.Floor(Convert.ToDouble(x[0]))), + new ("ieeeremainder", 2, x => Math.IEEERemainder(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("log", 2, x => Math.Log(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("log10", 1, x => Math.Log10(Convert.ToDouble(x[0]))), + new ("max", 2, x => Math.Max(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("min", 2, x => Math.Min(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("pow", 2, x => Math.Pow(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("round", 2, x => Math.Round(Convert.ToDouble(x[0]), Convert.ToInt32(x[1]))), + new ("sign", 1, x => Math.Sign(Convert.ToDouble(x[0]))), + new ("sin", 1, x => Math.Sin(Convert.ToDouble(x[0]))), + new ("sinh", 1, x => Math.Sinh(Convert.ToDouble(x[0]))), + new ("sqrt", 1, x => Math.Sqrt(Convert.ToDouble(x[0]))), + new ("tan", 1, x => Math.Tan(Convert.ToDouble(x[0]))), + new ("tanh", 1, x => Math.Tanh(Convert.ToDouble(x[0]))), + new ("truncate", 1, x => Math.Truncate(Convert.ToDouble(x[0]))), + new ("int", 1, x => Convert.ToInt32(x[0])), + new ("double", 1, x => Convert.ToDouble(x[0])), + new ("bool", 1, x => Convert.ToBoolean(x[0])), + new ("str", 1, x => x[0]?.ToString()), + new ("len", 1, x => x[0]?.ToString()?.Length), + new ("^", 2, x => Math.Pow(Convert.ToDouble(x[0]), Convert.ToDouble(x[1]))), + new ("pi", 0, _ => Math.PI), + new ("e", 0, _ => Math.E), + new ("true", 0, _ => true), + new ("false", 0, _ => false), + new ("null", 0, _ => null), }; if (argumentList.Count > 0) { - operators.Add(new MathOperator("x", 0, MathOperatorPrecedence.Constant, _ => argumentList[0])); + operators.Add(new MathOperator("x", 0, _ => argumentList[0])); } for (var i = 0; i < argumentList.Count; i++) { var index = i; - operators.Add(new MathOperator($"x{i}", 0, MathOperatorPrecedence.Constant, _ => argumentList[index])); + operators.Add(new MathOperator($"x{i}", 0, _ => argumentList[index])); } this.operators = operators; this.arguments = argumentList; } - internal string Expression { get; } + internal string expr { get; } + + internal int exprIndex { get; set; } = 0; + + internal Match patternMatch { get; set; } = Match.Empty; + + internal List rpn { get; } = new(); - public double Calculate() + public object? Calculate() { - var rpn = GetReversePolishNotation(Expression); + if (!ParseExpression()) + { + throw new ArgumentException("Invalid math expression."); + } - var stack = new Stack(); + var stack = new Stack(); - foreach (var value in rpn) + foreach (var token in rpn) { - if (double.TryParse(value, numberStyle, formatProvider, out var numeric)) + if (token.Type == MathTokenType.Value) { - stack.Push(numeric); + stack.Push(token.Value); continue; } - var mathOperator = operators.FirstOrDefault(x => x.Name == value) ?? - throw new ArgumentException($"Invalid math expression. Can't find operator or value with name \"{value}\"."); + var mathOperator = operators.FirstOrDefault(x => x.Name == token.Text) ?? + throw new ArgumentException($"Invalid math expression. Can't find operator or value with name \"{token.Text}\"."); - if (mathOperator.Precedence is MathOperatorPrecedence.Constant) + if (mathOperator.NumericCount == 0) { stack.Push(mathOperator.CalculateFunc([])); continue; @@ -104,15 +189,29 @@ public double Calculate() throw new ArgumentException("Invalid math expression."); } - var args = new List(); + bool nullGuard = false; + var args = new List(); for (var j = 0; j < operatorNumericCount; j++) { - args.Add(stack.Pop()); + object? val = stack.Pop(); + args.Add(val); + nullGuard = nullGuard || (val is null); } args.Reverse(); - stack.Push(mathOperator.CalculateFunc([.. args])); + switch (mathOperator.Name) + { + case "if": + nullGuard = args[0] is null; + break; + case "==": + case "!=": + nullGuard = false; + break; + } + + stack.Push(!nullGuard ? mathOperator.CalculateFunc([.. args]) : null); } if (stack.Count != 1) @@ -123,124 +222,270 @@ public double Calculate() return stack.Pop(); } - [GeneratedRegex(@"(? GetReversePolishNotation(string expression) + bool ParseExpr() { - var matches = MathExpressionRegexPattern().Matches(expression) ?? throw new ArgumentException("Invalid math expression."); + return ParseConditional(); + } - var output = new List(); - var stack = new Stack<(string Name, MathOperatorPrecedence Precedence)>(); + [GeneratedRegex("""^(\?)""")] + private static partial Regex ConditionalStart(); - foreach (var match in matches.Cast()) + [GeneratedRegex("""^(\:)""")] + private static partial Regex ConditionalElse(); + + bool ParseConditional() + { + if (!ParseLogicalOR()) { - if (string.IsNullOrEmpty(match?.Value)) - { - continue; - } + return false; + } + + if (!ParsePattern(ConditionalStart())) + { + return true; + } + + if (!ParseLogicalOR()) + { + return false; + } + + if (!ParsePattern(ConditionalElse())) + { + return false; + } + + if (!ParseLogicalOR()) + { + return false; + } + + rpn.Add(new MathToken(MathTokenType.Operator, "if", null)); + return true; + } + + [GeneratedRegex("""^(\|\||or)""")] + private static partial Regex LogicalOROperator(); + + bool ParseLogicalOR() => ParseBinaryOperators(LogicalOROperator(), ParseLogicalAnd); + + [GeneratedRegex("""^(\&\&|and)""")] + private static partial Regex LogicalAndOperator(); + + bool ParseLogicalAnd() => ParseBinaryOperators(LogicalAndOperator(), ParseEquality); + + [GeneratedRegex("""^(==|!=|eq|ne)""")] + private static partial Regex EqualityOperators(); + + bool ParseEquality() => ParseBinaryOperators(EqualityOperators(), ParseCompare); + + [GeneratedRegex("""^(\<\=|\>\=|\<|\>|le|ge|lt|gt)""")] + private static partial Regex CompareOperators(); + + bool ParseCompare() => ParseBinaryOperators(CompareOperators(), ParseSum); - var value = match.Value; + [GeneratedRegex("""^(\+|\-)""")] + private static partial Regex SumOperators(); - if (double.TryParse(value, numberStyle, formatProvider, out var numeric)) + bool ParseSum() => ParseBinaryOperators(SumOperators(), ParseProduct); + + [GeneratedRegex("""^(\*|\/|\%)""")] + private static partial Regex ProductOperators(); + + bool ParseProduct() => ParseBinaryOperators(ProductOperators(), ParsePower); + + [GeneratedRegex("""^(\^)""")] + private static partial Regex PowerOperator(); + + bool ParsePower() => ParseBinaryOperators(PowerOperator(), ParsePrimary); + + [GeneratedRegex("""^(\-|\!)""")] + private static partial Regex UnaryOperators(); + + static Dictionary unaryMapping { get; } = new Dictionary() + { + { "-", "neg" }, + { "!", "not" } + }; + + bool ParseBinaryOperators(Regex BinaryOperators, Func ParseNext) + { + if (!ParseNext()) + { + return false; + } + int index = exprIndex; + while (ParsePattern(BinaryOperators)) + { + string _operator = patternMatch.Groups[1].Value; + if (!ParseNext()) { - if (numeric < 0) - { - var isNegative = output.Count == 0 || stack.Count != 0; - - if (!isNegative) - { - stack.Push(("-", MathOperatorPrecedence.Low)); - output.Add(Math.Abs(numeric).ToString()); - continue; - } - } - - output.Add(value); - continue; + exprIndex = index; + return false; } + rpn.Add(new MathToken(MathTokenType.Operator, _operator, null)); + index = exprIndex; + } + return true; + } + + [GeneratedRegex("""^(\-?\d+\.\d+|\-?\d+)""")] + private static partial Regex NumberPattern(); + + [GeneratedRegex("""^["]([^"]*)["]""")] + private static partial Regex StringPattern(); - var @operator = operators.FirstOrDefault(x => x.Name == value); - if (@operator != null) + [GeneratedRegex("""^(\w+)""")] + private static partial Regex Constants(); + + [GeneratedRegex("""^(\()""")] + private static partial Regex ParenStart(); + + [GeneratedRegex("""^(\))""")] + private static partial Regex ParenEnd(); + + bool ParsePrimary() + { + if (ParsePattern(NumberPattern())) + { + string _number = patternMatch.Groups[1].Value; + rpn.Add(new MathToken(MathTokenType.Value, _number, double.Parse(_number))); + return true; + } + + if (ParsePattern(StringPattern())) + { + string _string = patternMatch.Groups[1].Value; + rpn.Add(new MathToken(MathTokenType.Value, _string, _string)); + return true; + } + + if (ParseFunction()) + { + return true; + } + + if (ParsePattern(Constants())) + { + string _constant = patternMatch.Groups[1].Value; + rpn.Add(new MathToken(MathTokenType.Operator, _constant, null)); + return true; + } + + int index = exprIndex; + if (ParsePattern(ParenStart())) + { + if (!ParseExpr()) { - if (@operator.Precedence is MathOperatorPrecedence.Constant) - { - output.Add(value); - continue; - } - - while (stack.Count > 0) - { - var (name, precedence) = stack.Peek(); - if (precedence >= @operator.Precedence) - { - output.Add(stack.Pop().Name); - } - else - { - break; - } - } - - stack.Push((value, @operator.Precedence)); + exprIndex = index; + return false; } - else if (value is "(") + if (!ParsePattern(ParenEnd())) { - stack.Push((value, MathOperatorPrecedence.Lowest)); + exprIndex = index; + return false; } - else if (value is ")") + return true; + } + + index = exprIndex; + if (ParsePattern(UnaryOperators())) + { + string _operator = patternMatch.Groups[1].Value; + if (unaryMapping.ContainsKey(_operator)) { - var isFound = false; - for (var i = stack.Count - 1; i >= 0; i--) - { - if (stack.Count == 0) - { - throw new ArgumentException("Invalid math expression."); - } - - var stackValue = stack.Pop().Name; - if (stackValue is "(") - { - isFound = true; - break; - } - - output.Add(stackValue); - } - - if (!isFound) - { - throw new ArgumentException("Invalid math expression."); - } + _operator = unaryMapping[_operator]; } - else if (value is ",") + if (!ParsePrimary()) { - while (stack.Count > 0) - { - var (name, precedence) = stack.Peek(); - if (precedence >= MathOperatorPrecedence.Low) - { - output.Add(stack.Pop().Name); - } - else - { - break; - } - } + exprIndex = index; + return false; } + rpn.Add(new MathToken(MathTokenType.Operator, _operator, null)); + return true; } - for (var i = stack.Count - 1; i >= 0; i--) + return false; + } + + [GeneratedRegex("""^(\w+)\(""")] + private static partial Regex FunctionStart(); + + [GeneratedRegex("""^(\,)""")] + private static partial Regex Comma(); + + [GeneratedRegex("""^(\))""")] + private static partial Regex FunctionEnd(); + + bool ParseFunction() + { + int index = exprIndex; + if (!ParsePattern(FunctionStart())) { - var (name, precedence) = stack.Pop(); - if (name is "(") + return false; + } + + string text = patternMatch.Groups[0].Value; + string functionName = patternMatch.Groups[1].Value; + + if (!ParseExpr()) + { + exprIndex = index; + return false; + } + + while (ParsePattern(Comma())) + { + if (!ParseExpr()) { - throw new ArgumentException("Invalid math expression."); + exprIndex = index; + return false; } + index = exprIndex; + } + + if (!ParsePattern(FunctionEnd())) + { + exprIndex = index; + return false; + } + + rpn.Add(new MathToken(MathTokenType.Operator, functionName, null)); + + return true; + } + + [GeneratedRegex("""^\s*""")] + private static partial Regex Whitespace(); - output.Add(name); + public bool ParsePattern(Regex regex) + { + var whitespaceMatch = Whitespace().Match(expr.Substring(exprIndex)); + if (whitespaceMatch.Success) + { + exprIndex += whitespaceMatch.Length; + } + + patternMatch = regex.Match(expr.Substring(exprIndex)); + if (!patternMatch.Success) + { + return false; + } + exprIndex += patternMatch.Length; + + whitespaceMatch = Whitespace().Match(expr.Substring(exprIndex)); + if (whitespaceMatch.Success) + { + exprIndex += whitespaceMatch.Length; } - return output; + return true; } -} \ No newline at end of file +} diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs index defe12120b..81535532db 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs @@ -5,10 +5,10 @@ namespace CommunityToolkit.Maui.Converters; /// /// Converters for Math expressions /// -public class MathExpressionConverter : BaseConverterOneWay +public class MathExpressionConverter : BaseConverterOneWay { /// - public override double DefaultConvertReturnValue { get; set; } = 0.0d; + public override object DefaultConvertReturnValue { get; set; } = 0.0d; /// /// Calculate the incoming expression string with one variable. @@ -17,7 +17,7 @@ public class MathExpressionConverter : BaseConverterOneWayThe expression to calculate. /// The culture to use in the converter. This is not implemented. /// A The result of calculating an expression. - public override double ConvertFrom(double value, string parameter, CultureInfo? culture = null) + public override object ConvertFrom(object value, string parameter, CultureInfo? culture = null) { ArgumentNullException.ThrowIfNull(parameter); diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs index 11317d780b..eb6a2f151f 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs @@ -28,21 +28,7 @@ public class MultiMathExpressionConverter : MultiValueConverterExtension, ICommu throw new ArgumentException("The parameter should be of type String."); } - if (values is null || values.Any(x => !double.TryParse(x?.ToString(), out _))) - { - return null; - } - - var args = new List(); - foreach (var value in values) - { - var valueString = value?.ToString() ?? throw new ArgumentException("Values cannot be null."); - - var xValue = double.Parse(valueString); - args.Add(xValue); - } - - var math = new MathExpression(expression, args); + var math = new MathExpression(expression, values!); return math.Calculate(); } From bf34590cc73f0e11500b9c74fdf05ede78bf94da Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Fri, 6 Sep 2024 11:18:11 +1000 Subject: [PATCH 02/11] Update UnitTest. Added conditional sample. --- .../MultiMathExpressionConverterPage.xaml | 125 +++++++++++------- .../MathExpressionConverterTests.cs | 4 +- 2 files changed, 76 insertions(+), 53 deletions(-) diff --git a/samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml b/samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml index 3d5b64b484..63a6353420 100644 --- a/samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml +++ b/samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml @@ -15,63 +15,86 @@ - - + + + \ No newline at end of file diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index d2bd9e8dd8..5b541009e0 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -124,9 +124,9 @@ public void MathExpressionConverterNullInputTest() Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(0.0, null, "x", null)); Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(0.0, null, null, null)); #pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. - //Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), "x", null)); + Assert.True(((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), "x", null) is null); Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).Convert(null, typeof(bool), null, null)); - //Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(null, typeof(bool), null, null)); + Assert.Throws(() => ((ICommunityToolkitValueConverter)new MathExpressionConverter()).ConvertBack(null, typeof(bool), null, null)); } [Fact] From 5a830f463c2d6f1931302fe2aa963af82001dbd5 Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Tue, 10 Sep 2024 17:35:16 +1000 Subject: [PATCH 03/11] Improve null handling and added extra boolean/null unit tests --- .../MathExpressionConverterTests.cs | 46 +++++++++++++++++-- .../MathExpression.shared.cs | 2 + 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index 5b541009e0..4fe9c12ba9 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -45,7 +45,7 @@ public void MathExpressionConverter_ReturnsCorrectResult(string expression, doub [InlineData("false", 2d, false)] [InlineData("-x > 2", 3d, false)] [InlineData("!!! (---x > 2)", 3d, true)] - public void MathExpressionConverter_ReturnsCorrectBooleanResult(string expression, double x, bool expectedResult) + public void MathExpressionConverter_WithComparisonOperator_ReturnsCorrectBooleanResult(string expression, double x, bool expectedResult) { var mathExpressionConverter = new MathExpressionConverter(); @@ -61,7 +61,7 @@ public void MathExpressionConverter_ReturnsCorrectBooleanResult(string expressio [InlineData("(x1 + x) * x1", new object[] { 2d, 3d }, 15d)] [InlineData("3 + x * x1 / (1 - 5)^x1", new object[] { 4d, 2d }, 3.5d)] [InlineData("3 + 4 * 2 + cos(100 + x) / (x1 - 5)^2 + pow(x0, 2)", new object[] { 20d, 1d }, 411.05088631065792d)] - public void MathExpressionConverter_WithMultiplyVariable_ReturnsCorrectResult(string expression, object[] variables, double expectedResult) + public void MathExpressionConverter_WithMultipleVariable_ReturnsCorrectResult(string expression, object[] variables, double expectedResult) { var mathExpressionConverter = new MultiMathExpressionConverter(); @@ -70,13 +70,53 @@ public void MathExpressionConverter_WithMultiplyVariable_ReturnsCorrectResult(st Assert.True(Math.Abs((double)result - expectedResult) < tolerance); } + [Theory] + [InlineData("x == 3 && x1", new object?[] { 3d, 4d }, 4d)] + [InlineData("x != 3 || x1", new object?[] { 3d, 4d }, 4d)] + public void MathExpressionConverter_WithBooleanOperator_ReturnsCorrectNumberResult(string expression, object[] variables, double expectedResult) + { + var mathExpressionConverter = new MultiMathExpressionConverter(); + + object? result = mathExpressionConverter.Convert(variables, mathExpressionTargetType, expression); + + Assert.True(result is not null); + Assert.Equal(expectedResult, result); + } + + [Theory] + [InlineData("x != 3 && x1", new object?[] { 3d, 4d }, false)] + [InlineData("x == 3 || x1", new object?[] { 3d, 4d }, true)] + public void MathExpressionConverter_WithBooleanOperator_ReturnsCorrectBooleanResult(string expression, object[] variables, bool expectedResult) + { + var mathExpressionConverter = new MultiMathExpressionConverter(); + + object? result = mathExpressionConverter.Convert(variables, mathExpressionTargetType, expression); + + Assert.True(result is not null); + Assert.Equal(expectedResult, result); + } + + [Theory] + [InlineData("x == 3 && x1", new object?[] { 3d, null})] + [InlineData("x != 3 || x1", new object?[] { 3d, null })] + [InlineData("x == 3 ? x1 : x2", new object?[] { 3d, null, 5d })] + [InlineData("x != 3 ? x1 : x2", new object?[] { 3d, 4d, null})] + public void MathExpressionConverter_ReturnsCorrectNullResult(string expression, object[] variables) + { + var mathExpressionConverter = new MultiMathExpressionConverter(); + + object? result = mathExpressionConverter.Convert(variables, mathExpressionTargetType, expression); + + Assert.True(result is null); + } + [Theory] [InlineData("x == x1", new object?[] { 2d, 2d }, true)] [InlineData("x == x1", new object?[] { 2d, null }, false)] [InlineData("x == x1", new object?[] { null, 2d}, false)] [InlineData("x == x1", new object?[] { null, null }, true)] [InlineData("(x ? x1 : x2) == null", new object?[] { true, null, 2d }, true)] - public void MathExpressionConverter_WithMultiplyVariable_ReturnsCorrectBooleanResult(string expression, object[] variables, bool expectedResult) + public void MathExpressionConverter_WithEqualityOperator_ReturnsCorrectBooleanResult(string expression, object[] variables, bool expectedResult) { var mathExpressionConverter = new MultiMathExpressionConverter(); diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index a0b4876167..4858402dc5 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -203,6 +203,8 @@ internal MathExpression(string expression, IEnumerable? arguments = null switch (mathOperator.Name) { case "if": + case "&&": + case "||": nullGuard = args[0] is null; break; case "==": From 7472ad0b9053e71f4e7486b2e9250a1321190a0a Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Tue, 10 Sep 2024 19:10:09 +1000 Subject: [PATCH 04/11] Additional boolean tests --- .../Converters/MathExpressionConverterTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index 4fe9c12ba9..ee9b7056b2 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -73,6 +73,8 @@ public void MathExpressionConverter_WithMultipleVariable_ReturnsCorrectResult(st [Theory] [InlineData("x == 3 && x1", new object?[] { 3d, 4d }, 4d)] [InlineData("x != 3 || x1", new object?[] { 3d, 4d }, 4d)] + [InlineData("x + x1 || true", new object?[] { 3d, 4d }, 7d)] + [InlineData("x + x1 && false", new object?[] { 2d, -2d }, 0d)] public void MathExpressionConverter_WithBooleanOperator_ReturnsCorrectNumberResult(string expression, object[] variables, double expectedResult) { var mathExpressionConverter = new MultiMathExpressionConverter(); From d580a45e85100d40bc9546f6be990c3194793cac Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Tue, 10 Sep 2024 19:33:00 +1000 Subject: [PATCH 05/11] Allow nulls if operator is ? : && || == != --- .../Converters/MathExpressionConverter/MathExpression.shared.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index 4858402dc5..bad11a5763 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -205,8 +205,6 @@ internal MathExpression(string expression, IEnumerable? arguments = null case "if": case "&&": case "||": - nullGuard = args[0] is null; - break; case "==": case "!=": nullGuard = false; From c60652ff7c5555014263b6cbadbcc5a256b9e59b Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Wed, 18 Sep 2024 12:39:33 +1000 Subject: [PATCH 06/11] __bool and MathToken record refactor --- .../MathExpression.shared.cs | 63 +++++-------------- 1 file changed, 14 insertions(+), 49 deletions(-) diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index bad11a5763..d54bd17379 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -10,19 +10,7 @@ enum MathTokenType Operator, }; -class MathToken -{ - public MathToken(MathTokenType type, string text, object? value) - { - Type = type; - Text = text; - Value = value; - } - - public MathTokenType Type { get; } - public string Text { get; } - public object? Value { get; } -}; +record MathToken(MathTokenType type, string text, object? value); sealed partial class MathExpression { @@ -33,38 +21,15 @@ sealed partial class MathExpression readonly IReadOnlyList operators; readonly IReadOnlyList arguments; - internal static bool __bool(object? value) - { - if (value is null) - { - return false; - } - - if (value is bool boolValue) - { - return boolValue; - } - - if (value is double doubleValue) - { - return doubleValue != 0 && doubleValue != double.NaN; - } - - if (value is string stringValue) - { - return !string.IsNullOrEmpty(stringValue); - } - - try - { - return Convert.ToBoolean(value); - } - catch (Exception ex) - { - } - - return !string.IsNullOrEmpty(value.ToString()); - } + internal static bool __bool(object? b) => + b switch + { + bool x => x, + null => false, + double doubleValue => doubleValue != 0 && doubleValue != double.NaN, + string stringValue => !string.IsNullOrEmpty(stringValue), + _ => Convert.ToBoolean(b) + }; internal MathExpression(string expression, IEnumerable? arguments = null) { @@ -167,14 +132,14 @@ internal MathExpression(string expression, IEnumerable? arguments = null foreach (var token in rpn) { - if (token.Type == MathTokenType.Value) + if (token.type == MathTokenType.Value) { - stack.Push(token.Value); + stack.Push(token.value); continue; } - var mathOperator = operators.FirstOrDefault(x => x.Name == token.Text) ?? - throw new ArgumentException($"Invalid math expression. Can't find operator or value with name \"{token.Text}\"."); + var mathOperator = operators.FirstOrDefault(x => x.Name == token.text) ?? + throw new ArgumentException($"Invalid math expression. Can't find operator or value with name \"{token.text}\"."); if (mathOperator.NumericCount == 0) { From ef3380a4b967487ad31845ae47cfc85462025ec6 Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Wed, 18 Sep 2024 12:47:20 +1000 Subject: [PATCH 07/11] First attempt to block null reference return --- .../Converters/MathExpressionConverter/MathExpression.shared.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index d54bd17379..74e58257a9 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -28,7 +28,7 @@ internal static bool __bool(object? b) => null => false, double doubleValue => doubleValue != 0 && doubleValue != double.NaN, string stringValue => !string.IsNullOrEmpty(stringValue), - _ => Convert.ToBoolean(b) + _ => Convert.ToBoolean(b!) }; internal MathExpression(string expression, IEnumerable? arguments = null) From cd8def9aabbf5c445c00b4665eb6176e919ac4a6 Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Wed, 18 Sep 2024 13:19:37 +1000 Subject: [PATCH 08/11] Second attempt to block null reference return --- .../MathExpressionConverter/MathExpression.shared.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index 74e58257a9..5bf9bf68bb 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -24,11 +24,8 @@ sealed partial class MathExpression internal static bool __bool(object? b) => b switch { - bool x => x, - null => false, - double doubleValue => doubleValue != 0 && doubleValue != double.NaN, string stringValue => !string.IsNullOrEmpty(stringValue), - _ => Convert.ToBoolean(b!) + _ => b is null ? false : Convert.ToBoolean(b!) }; internal MathExpression(string expression, IEnumerable? arguments = null) From 3c9b632ecd26314c26f53ca13cffcfaa4117f4e6 Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Wed, 18 Sep 2024 13:38:08 +1000 Subject: [PATCH 09/11] Addessed CS8603 possible null reference return issues --- .../Converters/MathExpressionConverterTests.cs | 2 ++ .../MathExpressionConverter/MathExpression.shared.cs | 9 ++++++--- .../MathExpressionConverter.shared.cs | 6 +++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index ee9b7056b2..b36a8a6e7b 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -29,6 +29,7 @@ public void MathExpressionConverter_ReturnsCorrectResult(string expression, doub var convertResult = ((ICommunityToolkitValueConverter)mathExpressionConverter).Convert(x, mathExpressionTargetType, expression, cultureInfo) ?? throw new NullReferenceException(); var convertFromResult = mathExpressionConverter.ConvertFrom(x, expression); + Assert.True(convertFromResult is not null); Assert.True(Math.Abs((double)convertResult - expectedResult) < tolerance); Assert.True(Math.Abs((double)convertFromResult - expectedResult) < tolerance); } @@ -52,6 +53,7 @@ public void MathExpressionConverter_WithComparisonOperator_ReturnsCorrectBoolean var convertResult = ((ICommunityToolkitValueConverter)mathExpressionConverter).Convert(x, mathExpressionTargetType, expression, cultureInfo) ?? throw new NullReferenceException(); var convertFromResult = mathExpressionConverter.ConvertFrom(x, expression); + Assert.True(convertFromResult is not null); Assert.True((bool)convertResult == expectedResult); Assert.True((bool)convertFromResult == expectedResult); } diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index 5bf9bf68bb..c412dbb90c 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -19,16 +19,19 @@ sealed partial class MathExpression static readonly IFormatProvider formatProvider = new CultureInfo("en-US"); readonly IReadOnlyList operators; - readonly IReadOnlyList arguments; + readonly IReadOnlyList arguments; internal static bool __bool(object? b) => b switch { + bool x => x, + null => false, + double doubleValue => doubleValue != 0 && doubleValue != double.NaN, string stringValue => !string.IsNullOrEmpty(stringValue), - _ => b is null ? false : Convert.ToBoolean(b!) + _ => Convert.ToBoolean(b) }; - internal MathExpression(string expression, IEnumerable? arguments = null) + internal MathExpression(string expression, IEnumerable? arguments = null) { ArgumentException.ThrowIfNullOrEmpty(expression, "Expression can't be null or empty."); diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs index 81535532db..597a222877 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs @@ -5,10 +5,10 @@ namespace CommunityToolkit.Maui.Converters; /// /// Converters for Math expressions /// -public class MathExpressionConverter : BaseConverterOneWay +public class MathExpressionConverter : BaseConverterOneWay { /// - public override object DefaultConvertReturnValue { get; set; } = 0.0d; + public override object? DefaultConvertReturnValue { get; set; } = 0.0d; /// /// Calculate the incoming expression string with one variable. @@ -17,7 +17,7 @@ public class MathExpressionConverter : BaseConverterOneWayThe expression to calculate. /// The culture to use in the converter. This is not implemented. /// A The result of calculating an expression. - public override object ConvertFrom(object value, string parameter, CultureInfo? culture = null) + public override object? ConvertFrom(object? value, string parameter, CultureInfo? culture = null) { ArgumentNullException.ThrowIfNull(parameter); From 9bf1bef9740263e3bd54b4791bf1fd97c07a74e6 Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Wed, 25 Sep 2024 10:26:38 +1000 Subject: [PATCH 10/11] Use correct C# property name pattern. Log invalid math expressions instead of raising exceptions. --- .../MathExpressionConverterTests.cs | 6 +- .../MathExpression.shared.cs | 111 ++++++++++-------- 2 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index b36a8a6e7b..09a5d0b9f7 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -134,12 +134,12 @@ public void MathExpressionConverter_WithEqualityOperator_ReturnsCorrectBooleanRe [InlineData("1 + 3 + 5 + (3 - 2))")] [InlineData("1 + 2) + (9")] [InlineData("100 + pow(2)")] - public void MathExpressionConverterThrowsArgumentException(string expression) + public void MathExpressionConverter_WithInvalidExpressions_ReturnsNullResult(string expression) { var mathExpressionConverter = new MathExpressionConverter(); - Assert.Throws(() => ((ICommunityToolkitValueConverter)mathExpressionConverter).Convert(0d, mathExpressionTargetType, expression, cultureInfo)); - Assert.Throws(() => mathExpressionConverter.ConvertFrom(0d, expression)); + Assert.Null(((ICommunityToolkitValueConverter)mathExpressionConverter).Convert(0d, mathExpressionTargetType, expression, cultureInfo)); + Assert.Null(mathExpressionConverter.ConvertFrom(0d, expression)); } [Theory] diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs index c412dbb90c..2966245230 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs @@ -1,5 +1,6 @@ using System.Globalization; using System.Text.RegularExpressions; +using System.Diagnostics; using CommunityToolkit.Maui.Core; namespace CommunityToolkit.Maui.Converters; @@ -37,7 +38,7 @@ internal MathExpression(string expression, IEnumerable? arguments = nul var argumentList = arguments?.ToList() ?? []; - expr = expression.ToLower(); + Expression = expression.ToLower(); var operators = new List { @@ -113,24 +114,25 @@ internal MathExpression(string expression, IEnumerable? arguments = nul this.arguments = argumentList; } - internal string expr { get; } + internal string Expression { get; } - internal int exprIndex { get; set; } = 0; + internal int ExpressionIndex { get; set; } = 0; - internal Match patternMatch { get; set; } = Match.Empty; + internal Match PatternMatch { get; set; } = Match.Empty; - internal List rpn { get; } = new(); + internal List RPN { get; } = new(); public object? Calculate() { if (!ParseExpression()) { - throw new ArgumentException("Invalid math expression."); + Trace.TraceWarning("Invalid math expression. Failed to parse expression."); + return null; } var stack = new Stack(); - foreach (var token in rpn) + foreach (var token in RPN) { if (token.type == MathTokenType.Value) { @@ -138,8 +140,12 @@ internal MathExpression(string expression, IEnumerable? arguments = nul continue; } - var mathOperator = operators.FirstOrDefault(x => x.Name == token.text) ?? - throw new ArgumentException($"Invalid math expression. Can't find operator or value with name \"{token.text}\"."); + var mathOperator = operators.FirstOrDefault(x => x.Name == token.text); + if (mathOperator is null) + { + Trace.TraceWarning($"Invalid math expression. Can't find operator or value with name \"{token.text}\"."); + return null; + } if (mathOperator.NumericCount == 0) { @@ -151,7 +157,8 @@ internal MathExpression(string expression, IEnumerable? arguments = nul if (stack.Count < operatorNumericCount) { - throw new ArgumentException("Invalid math expression."); + Trace.TraceWarning($"Invalid math expression. Insufficient parameters to operator \"{mathOperator.Name}\"."); + return null; } bool nullGuard = false; @@ -179,9 +186,15 @@ internal MathExpression(string expression, IEnumerable? arguments = nul stack.Push(!nullGuard ? mathOperator.CalculateFunc([.. args]) : null); } - if (stack.Count != 1) + if (stack.Count == 0) + { + Trace.TraceWarning($"Invalid math expression. Stack is unexpectedly empty."); + return null; + } + + if (stack.Count > 1) { - throw new ArgumentException("Invalid math expression."); + Trace.WriteLine($"Invalid math expression. Stack unexpectedly contains too many ({stack.Count}) items."); } return stack.Pop(); @@ -189,9 +202,9 @@ internal MathExpression(string expression, IEnumerable? arguments = nul bool ParseExpression() { - exprIndex = 0; - rpn.Clear(); - return ParseExpr() && exprIndex == expr.Length; + ExpressionIndex = 0; + RPN.Clear(); + return ParseExpr() && ExpressionIndex == Expression.Length; } bool ParseExpr() @@ -232,7 +245,7 @@ bool ParseConditional() return false; } - rpn.Add(new MathToken(MathTokenType.Operator, "if", null)); + RPN.Add(new MathToken(MathTokenType.Operator, "if", null)); return true; } @@ -286,17 +299,17 @@ bool ParseBinaryOperators(Regex BinaryOperators, Func ParseNext) { return false; } - int index = exprIndex; + int index = ExpressionIndex; while (ParsePattern(BinaryOperators)) { - string _operator = patternMatch.Groups[1].Value; + string _operator = PatternMatch.Groups[1].Value; if (!ParseNext()) { - exprIndex = index; + ExpressionIndex = index; return false; } - rpn.Add(new MathToken(MathTokenType.Operator, _operator, null)); - index = exprIndex; + RPN.Add(new MathToken(MathTokenType.Operator, _operator, null)); + index = ExpressionIndex; } return true; } @@ -320,15 +333,15 @@ bool ParsePrimary() { if (ParsePattern(NumberPattern())) { - string _number = patternMatch.Groups[1].Value; - rpn.Add(new MathToken(MathTokenType.Value, _number, double.Parse(_number))); + string _number = PatternMatch.Groups[1].Value; + RPN.Add(new MathToken(MathTokenType.Value, _number, double.Parse(_number))); return true; } if (ParsePattern(StringPattern())) { - string _string = patternMatch.Groups[1].Value; - rpn.Add(new MathToken(MathTokenType.Value, _string, _string)); + string _string = PatternMatch.Groups[1].Value; + RPN.Add(new MathToken(MathTokenType.Value, _string, _string)); return true; } @@ -339,41 +352,41 @@ bool ParsePrimary() if (ParsePattern(Constants())) { - string _constant = patternMatch.Groups[1].Value; - rpn.Add(new MathToken(MathTokenType.Operator, _constant, null)); + string _constant = PatternMatch.Groups[1].Value; + RPN.Add(new MathToken(MathTokenType.Operator, _constant, null)); return true; } - int index = exprIndex; + int index = ExpressionIndex; if (ParsePattern(ParenStart())) { if (!ParseExpr()) { - exprIndex = index; + ExpressionIndex = index; return false; } if (!ParsePattern(ParenEnd())) { - exprIndex = index; + ExpressionIndex = index; return false; } return true; } - index = exprIndex; + index = ExpressionIndex; if (ParsePattern(UnaryOperators())) { - string _operator = patternMatch.Groups[1].Value; + string _operator = PatternMatch.Groups[1].Value; if (unaryMapping.ContainsKey(_operator)) { _operator = unaryMapping[_operator]; } if (!ParsePrimary()) { - exprIndex = index; + ExpressionIndex = index; return false; } - rpn.Add(new MathToken(MathTokenType.Operator, _operator, null)); + RPN.Add(new MathToken(MathTokenType.Operator, _operator, null)); return true; } @@ -391,18 +404,18 @@ bool ParsePrimary() bool ParseFunction() { - int index = exprIndex; + int index = ExpressionIndex; if (!ParsePattern(FunctionStart())) { return false; } - string text = patternMatch.Groups[0].Value; - string functionName = patternMatch.Groups[1].Value; + string text = PatternMatch.Groups[0].Value; + string functionName = PatternMatch.Groups[1].Value; if (!ParseExpr()) { - exprIndex = index; + ExpressionIndex = index; return false; } @@ -410,19 +423,19 @@ bool ParseFunction() { if (!ParseExpr()) { - exprIndex = index; + ExpressionIndex = index; return false; } - index = exprIndex; + index = ExpressionIndex; } if (!ParsePattern(FunctionEnd())) { - exprIndex = index; + ExpressionIndex = index; return false; } - rpn.Add(new MathToken(MathTokenType.Operator, functionName, null)); + RPN.Add(new MathToken(MathTokenType.Operator, functionName, null)); return true; } @@ -432,23 +445,23 @@ bool ParseFunction() public bool ParsePattern(Regex regex) { - var whitespaceMatch = Whitespace().Match(expr.Substring(exprIndex)); + var whitespaceMatch = Whitespace().Match(Expression.Substring(ExpressionIndex)); if (whitespaceMatch.Success) { - exprIndex += whitespaceMatch.Length; + ExpressionIndex += whitespaceMatch.Length; } - patternMatch = regex.Match(expr.Substring(exprIndex)); - if (!patternMatch.Success) + PatternMatch = regex.Match(Expression.Substring(ExpressionIndex)); + if (!PatternMatch.Success) { return false; } - exprIndex += patternMatch.Length; + ExpressionIndex += PatternMatch.Length; - whitespaceMatch = Whitespace().Match(expr.Substring(exprIndex)); + whitespaceMatch = Whitespace().Match(Expression.Substring(ExpressionIndex)); if (whitespaceMatch.Success) { - exprIndex += whitespaceMatch.Length; + ExpressionIndex += whitespaceMatch.Length; } return true; From 9b64f7bcfb90a922af8d2cf507e9f7479cfcc73e Mon Sep 17 00:00:00 2001 From: Stephen Quan Date: Mon, 7 Oct 2024 10:17:02 +1100 Subject: [PATCH 11/11] Null Forgiving Operator removed. Adjust null checks in MultiMathExpressionConverter and corresponding unit tests. --- .../Converters/MathExpressionConverterTests.cs | 1 + .../MultiMathExpressionConverter.shared.cs | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs index 09a5d0b9f7..18d08d87e6 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs @@ -69,6 +69,7 @@ public void MathExpressionConverter_WithMultipleVariable_ReturnsCorrectResult(st var result = mathExpressionConverter.Convert(variables, mathExpressionTargetType, expression); + Assert.NotNull(result); Assert.True(Math.Abs((double)result - expectedResult) < tolerance); } diff --git a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs index eb6a2f151f..7281258050 100644 --- a/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs +++ b/src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs @@ -17,7 +17,6 @@ public class MultiMathExpressionConverter : MultiValueConverterExtension, ICommu /// The expression to calculate. /// The culture to use in the converter. This is not implemented. /// A The result of calculating an expression. - [return: NotNullIfNotNull(nameof(values))] public object? Convert(object?[]? values, Type targetType, [NotNull] object? parameter, CultureInfo? culture = null) { ArgumentNullException.ThrowIfNull(targetType); @@ -28,7 +27,12 @@ public class MultiMathExpressionConverter : MultiValueConverterExtension, ICommu throw new ArgumentException("The parameter should be of type String."); } - var math = new MathExpression(expression, values!); + if (values is null) + { + return null; + } + + var math = new MathExpression(expression, values); return math.Calculate(); }