Migrate to PEG parser. Introduce boolean operators and constants.

[stephenquan]

Description of Change

• MultiMathExpressionConverterPage.xaml
  • Add VerticalStackLayout and move existing sample inside
  • Added a Label with a DataTrigger on "x0 + x1 + x2 >= 60" to demonstrate the comparison operator
• MathOperator.shared.cs
  • Replaced double with object
  • MathOperatorPrecedence obsolete (part of the original parsing engine but no longer relevant in new engine)
• MathExpressionConverterTests.cs
  • New tests for comparison, equality, and boolean operators
  • Certain operations allow nulls now
• MathExpression.shared.cs
  • Refactor/rewrite to use Parsing expression grammar
  • The grammar parity with original operators +, -, *, / and ^ as well as math functions
  • Added to grammar and, or, ?, :, >=, >, <=, <, ==, != as well as true and false constants
  • Relax null strictness (e.g. equality and ternary operators can have nulls)
• MathExpressionConverter.shared.cs
  • Replaced double with object
• MultiMathExpressionConverter.cs
  • Relax null strictness (e.g. equality and ternary operators can have nulls)

General comments:

• Original implementation sanitizes all inputs by calling double.Parse(value.ToString) rejecting nulls
• New implementation calls Convert.ToDouble() when needed and, in certain situations, allows nulls
• When a boolean value is needed, boolean coercion uses the following rules:
  • If value is null, return false
  • If value is a boolean, return value
  • If value is a double, return true if value != 0 && value != NaN, false otherwise
  • If value is a string, return true, if value is not null or empty, false otherwise

Linked Issues

#2181

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: Add new boolean operators MicrosoftDocs/CommunityToolkit#465

Additional information

[stephenquan]

@dotnet-policy-service agree company="Esri"

[stephenquan]

Hi @brminnick since your review, I have:

• attempted to update the fork with the base branch
• corrected the unit test issues you raised
• added more unit test for better coverage of the boolean operators
• updated the handling of nulls

The fork is ready for another round of review

[pictos]

@stephenquan here's the build log:

D:\a\1\s\src\CommunityToolkit.Maui\Converters\MathExpressionConverter\MathExpressionConverter.shared.cs(25,10): error CS8603: Possible null reference return. [D:\a\1\s\src\CommunityToolkit.Maui\CommunityToolkit.Maui.csproj::TargetFramework=net8.0-ios]
D:\a\1\s\src\CommunityToolkit.Maui\Converters\MathExpressionConverter\MathExpressionConverter.shared.cs(25,10): error CS8603: Possible null reference return. [D:\a\1\s\src\CommunityToolkit.Maui\CommunityToolkit.Maui.csproj::TargetFramework=net8.0-windows10.0.19041.0]
D:\a\1\s\src\CommunityToolkit.Maui\Converters\MathExpressionConverter\MathExpressionConverter.shared.cs(25,10): error CS8603: Possible null reference return. [D:\a\1\s\src\CommunityToolkit.Maui\CommunityToolkit.Maui.csproj::TargetFramework=net8.0-tizen]
D:\a\1\s\src\CommunityToolkit.Maui\Converters\MathExpressionConverter\MathExpressionConverter.shared.cs(25,10): error CS8603: Possible null reference return. [D:\a\1\s\src\CommunityToolkit.Maui\CommunityToolkit.Maui.csproj::TargetFramework=net8.0-maccatalyst]
D:\a\1\s\src\CommunityToolkit.Maui\Converters\MathExpressionConverter\MathExpressionConverter.shared.cs(25,10): error CS8603: Possible null reference return. [D:\a\1\s\src\CommunityToolkit.Maui\CommunityToolkit.Maui.csproj::TargetFramework=net8.0-android]
  

[stephenquan]

Thanks @pictos, I have reviewed the CS8063 possible null reference return issues and have resolved it by making nullability updates to both the MathExpression.shared.cs and MathExpressionConverter.shared.cs. The nullability updates also required additional Asserts added to the MathExpressionConverterTests.cs. With these changes, the build checks are now passing.

[pictos]

I think that we're getting really close to merge this!

[stephenquan]

@pictos @bijington I have pushed updates to the fork address both the C# property name pattern and logging invalid math expressions with TraceWarning. Because we're no longer raising invalid math expressions I had to change the relevant unit test from an exception test to a null check. The fork is ready for review again.

[stephenquan]

@pictos @brminnick as requested, I have removed the Null Forgiving Operator from the MultiMathExpressionConverter. I have corrected the null checks in the MultiMathExpressionConverter and that resulted in a necessary minor update to the unit tests.

This fork is ready for review again.

[stephenquan]

Hi all, we are using snapshot copies of these classes in our projects. I am eager to help out and remove any blockers on this PR.

[pictos]

LGTM

[stephenquan]

Hi guys, I think this PR is good to merge.

[bijington]

Hi guys, I think this PR is good to merge.

Thanks. We are trying to get to the point of merging #2215 before any other PRs due to that size of it and that it provides .NET 9.0 support. Then we would ideally merge PRs like this. @brminnick is that correct order for our plan?

[brminnick]

Edit: Yup - Shaun is correct. I see we commented at the same time 😊 🙌

We are blocked from merging any PRs until we've merged our PR adding support for .NET 9.

Our .NET 9 PR is complete, however, it is blocked by this bug in .NET MAUI: dotnet/maui#25871

It may encourage the MAUI engineering team to increase its priority if you navigate to the .NET MAUI bug blocking our .NET 9 PR and leave a comment asking the team to prioritize it.