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

Copilot reviewed 5 out of 6 changed files in this pull request and generated no suggestions.

Files not reviewed (1)

• samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml: Language not supported

Comments skipped due to low confidence (3)

src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs:66

• [nitpick] The variable name 'variables' should be renamed to 'values' for consistency with the method parameter name in MultiMathExpressionConverter.shared.cs.

public void MathExpressionConverter_WithMultipleVariable_ReturnsCorrectResult(string expression, object[] variables, double expectedResult)

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs:20

• [nitpick] The variable name 'value' is less descriptive in the context of mathematical expressions. Consider renaming it to 'inputValue' for clarity.

public override object? ConvertFrom(object? value, string parameter, CultureInfo? culture = null)

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpressionConverter.shared.cs:20

• The removal of the '[return: NotNullIfNotNull(nameof(values))]' attribute might lead to unclear error messages when null values are encountered. Consider adding a clear error message for null values.

public override object? ConvertFrom(object? value, string parameter, CultureInfo? culture = null)

[TheCodeTraveler]

Hi @stephenquan! Could you update this PR to resolve the merge conflicts?

We've finally merged the massive .NET 9 PR today and it introduced a few merge conflicts across our open PRs.

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)

• samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml: Language not supported

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Files not reviewed (1)

• samples/CommunityToolkit.Maui.Sample/Pages/Converters/MultiMathExpressionConverterPage.xaml: Language not supported

[TheCodeTraveler]

Thanks @stephenquan! Approved ✅