Migrate to PEG parser. Introduce boolean operators and constants.
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
doublewithobject - MathOperatorPrecedence obsolete (part of the original parsing engine but no longer relevant in new engine)
- Replaced
- 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 astrueandfalseconstants - Relax null strictness (e.g. equality and ternary operators can have nulls)
- MathExpressionConverter.shared.cs
- Replaced
doublewithobject
- Replaced
- 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
https://github.com/CommunityToolkit/Maui/issues/2181
PR Checklist
- [x] Has a linked Issue, and the Issue has been
approved(bug) orChampioned(feature/proposal) - [x] Has tests (if omitted, state reason in description)
- [x] Has samples (if omitted, state reason in description)
- [x] Rebased on top of
mainat time of PR - [x] Changes adhere to coding standard
- [x] Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pull/465
Additional information
@dotnet-policy-service agree company="Esri"
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
@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]
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 @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.
@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.
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.
Hi guys, I think this PR is good to merge.
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?
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: https://github.com/dotnet/maui/issues/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.
I have some new developers working with Boolean operators in XAML, but they struggled with the syntax.
To simplify I things added new commits which adds XAML-friendly alternatives. Additionally, I added more coverage to the logical AND/OR unit tests.
Unfortunately, in doing so, 3 of the 6 automated test are failing and I am not sure why. Also, the PR will now require a re-review.
| Operator | XAML | Alternate |
|---|---|---|
&& |
&& |
and |
|| |
|| |
or |
>= |
>= |
ge |
> |
> |
gt |
<= |
<= |
le |
< |
< |
lt |
Here are some examples of the alternate syntax:
| Original | Alternate |
|---|---|
x >= 0 ? x1 : x2 |
x ge 0 ? x1 : x2 |
x && !x1 |
x and !x1 |
x >= 0 && x <= 10 |
x ge 0 and x le 10 |
The azure pipelines https://dev.azure.com/dotnet/CommunityToolkit/_build/results?buildId=112270&view=logs&j=67da6a11-18ef-5f70-9b43-fae7e6c83e18&t=54ba8226-26d8-53c8-39e8-392f99f22929&l=538 is showing 11 new errors that I do not know how to resolve. These 11 new errors do not relate the commits done, so, I'm hoping they will automatically resolve when the fork catches up with the main branch:
D:\a\1\s\samples\CommunityToolkit.Maui.Sample\MauiProgram.cs(38,20): error CS0234: The type or namespace name 'Composition' does not exist in the namespace 'Microsoft.UI' (are you missing an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-windows10.0.19041.0] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\MauiProgram.cs(39,20): error CS0234: The type or namespace name 'Windowing' does not exist in the namespace 'Microsoft.UI' (are you missing an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-windows10.0.19041.0] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\MauiProgram.cs(40,25): error CS0234: The type or namespace name 'Media' does not exist in the namespace 'Microsoft.UI.Xaml' (are you missing an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-windows10.0.19041.0] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\Platforms\Windows\App.xaml.cs(6,28): error CS0246: The type or namespace name 'MauiWinUIApplication' could not be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-windows10.0.19041.0] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\Platforms\MacCatalyst\AppDelegate.cs(6,28): error CS0246: The type or namespace name 'MauiUIApplicationDelegate' could not be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-maccatalyst] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\Platforms\iOS\AppDelegate.cs(6,28): error CS0246: The type or namespace name 'MauiUIApplicationDelegate' could not be found (are you missing a using directive or an assembly reference?) [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-ios] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: attribute tabGravity (aka com.microsoft.CommunityToolkit.Maui.Sample:tabGravity) not found. [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: attribute tabIndicatorColor (aka com.microsoft.CommunityToolkit.Maui.Sample:tabIndicatorColor) not found. [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: attribute tabMode (aka com.microsoft.CommunityToolkit.Maui.Sample:tabMode) not found. [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] D:\a\1\s\samples\CommunityToolkit.Maui.Sample\obj\Release\net8.0-android\lp\148\jl\res\layout\tabbar.xml(2): error APT2260: [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] C:\hostedtoolcache\windows\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.145\tools\Xamarin.Android.Aapt2.targets(156,3): error APT2061: failed linking file resources. [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android] C:\hostedtoolcache\windows\dotnet\packs\Microsoft.Android.Sdk.Windows\34.0.145\tools\Xamarin.Android.Aapt2.targets(156,3): error APT2061: [D:\a\1\s\samples\CommunityToolkit.Maui.Sample\CommunityToolkit.Maui.Sample.csproj::TargetFramework=net8.0-android]
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.