Migrate to PEG parser. Introduce boolean operators and constants. #2182

stephenquan · 2024-09-05T22:31:36Z

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

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

stephenquan · 2024-09-05T23:18:23Z

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

stephenquan · 2024-09-16T23:30:24Z

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

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs

pictos · 2024-09-17T01:10:26Z

@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 · 2024-09-18T04:01:00Z

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!

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs

…stead of raising exceptions.

stephenquan · 2024-09-25T02:45:12Z

@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.

...munityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs

…ssionConverter and corresponding unit tests.

stephenquan · 2024-10-06T23:41:04Z

@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 · 2024-10-20T21:30:07Z

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

…ators.

stephenquan · 2024-12-05T10:57:29Z

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`

…es in unit tests.

stephenquan · 2024-12-06T08:47:32Z

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]

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)

brminnick · 2024-12-18T01:23:13Z

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

brminnick · 2025-01-02T19:27:53Z

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

+	[InlineData("Cat", false, false, "Cat")]
+	[InlineData("", false, "", false)]
+	[InlineData(false, "", false, "")]
+	[InlineData(null, "Cat", null, "Cat")]


Hey @stephenquan! There's one thing I'm having trouble wrapping my head around in this PR: How/why we are evaluating null using boolean operators, as demonstrated here in this test?

If I'm understanding everything correctly, the below line of code is equivalent to evaluating the boolean math equation of null OR null, right?

mathExpressionConverter.Convert(variables, typeof(object), "x || x1")

I haven't studied boolean algebra since college, but, my understanding is that boolean algebra does not include the concept of null. Is my understanding correct?

My gut tells me that we should be throwing an Exception in the scenarios where null is being used in a Boolean Algebraic equation.

In other words, MathExpression.Calculate() should not return null in these scenarios and instead throw an exception alerting the developer that they've input an invalid equation.

@brminnick thanks for your question. The inspiration for logical-AND and logical-OR operators is derived from both Javascript and Python with respect to how they handle truthy and falsy concepts. For reference:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_AND

If a value can be converted to true, the value is so-called truthy. If a value can be converted to false, the value is so-called falsy.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_OR

If x can be converted to true, returns x; else, returns y.

And the unit test mirror the results from the Javascript documentation examples.

The Python documentation also describes this behavior: https://docs.python.org/3/reference/expressions.html#boolean-operations

The expression x and y first evaluates x; if x is false, its value is returned; otherwise, y is evaluated and the resulting value is returned.
The expression x or y first evaluates x; if x is true, its value is returned; otherwise, y is evaluated and the resulting value is returned.

If I wanted to implement the following case statement

// states of water if (x0 < 0) then x1 // "ice"; else if (x0 < 100) then x2 // "water" else x3 // "steam"

I could write it as:

// using the conditional operator x0 < 0 ? x1 : x0 < 100 ? x2 : x3

Or, using the truthy and falsy states, I could rewrite it as

// using the truthy and falsy application of logical-AND and logical-OR (x0 < 0) and x1 or (x0 < 100) and x2 or x3

Generalizing the pattern, it would be:

conditional-1 and result-1 or conditional-2 and result-2 or ... or conditional-n and result-n or result-else

There are limitations to this approach since the things fail if any of the intermediate results are "falsy" then it doesn't work. In the above example, "ice", "water" and "steam" are truthy values since they're non-empty strings. It is also because of this behavior, that we do not require any of the inputs to be strictly boolean. They can be of any object including null and it would still behave.

Javascript

console.log(null || null); // output: null console.log(null && null); // output: null

Python

>>> print(None or None) // output: None None >>> print(None and None) // output: None None

It's the equivalent of a Nullable<bool> expression so I think it's fine

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

Migrate to PEG parser. Introduce boolean operators and constants.

1d2577d

brminnick reviewed Sep 5, 2024

View reviewed changes

src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs Outdated Show resolved Hide resolved

src/CommunityToolkit.Maui.UnitTests/Converters/MathExpressionConverterTests.cs Outdated Show resolved Hide resolved

Update UnitTest. Added conditional sample.

bf34590

stephenquan marked this pull request as ready for review September 6, 2024 01:49

stephenquan added 4 commits September 6, 2024 13:17

Merge branch 'main' of github.com:CommunityToolkit/Maui

584eb1e

Improve null handling and added extra boolean/null unit tests

5a830f4

Additional boolean tests

7472ad0

Allow nulls if operator is ? : && || == !=

d580a45

pictos reviewed Sep 17, 2024

View reviewed changes

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs Outdated Show resolved Hide resolved

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs Outdated Show resolved Hide resolved

stephenquan added 4 commits September 18, 2024 12:39

__bool and MathToken record refactor

c60652f

First attempt to block null reference return

ef3380a

Second attempt to block null reference return

cd8def9

Addessed CS8603 possible null reference return issues

3c9b632

pictos added 2 commits September 18, 2024 01:22

Merge branch 'main' into main

5aaf5ef

Merge branch 'main' into main

fde4fc1

pictos reviewed Sep 22, 2024

View reviewed changes

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs Outdated Show resolved Hide resolved

src/CommunityToolkit.Maui/Converters/MathExpressionConverter/MathExpression.shared.cs Outdated Show resolved Hide resolved

Use correct C# property name pattern. Log invalid math expressions in…

9bf1bef

…stead of raising exceptions.

pictos and others added 2 commits September 25, 2024 23:54

Merge branch 'main' into main

34d3883

Merge branch 'main' into main

88b0103

pictos reviewed Oct 6, 2024

View reviewed changes

...munityToolkit.Maui/Converters/MathExpressionConverter/MultiMathExpressionConverter.shared.cs Outdated Show resolved Hide resolved

Null Forgiving Operator removed. Adjust null checks in MultiMathExpre…

9b64f7b

…ssionConverter and corresponding unit tests.

Merge branch 'main' into main

c3ea28b

pictos previously approved these changes Oct 22, 2024

View reviewed changes

Add more logical and/or unit tests. Add XAML friendly comparator oper…

6808590

…ators.

stephenquan dismissed pictos’s stale review via 6808590 December 5, 2024 09:09

Improve unit test coverage on alternate operators.

31d712e

stephenquan force-pushed the main branch from 47b5f7a to 31d712e Compare December 6, 2024 03:19

Improve null handling for logical operators && || and cover these cas…

a983e7a

…es in unit tests.

Merge branch 'main' into main

a4bcd8a

VladislavAntonyuk requested a review from Copilot December 10, 2024 23:10

Copilot AI reviewed Dec 10, 2024

View reviewed changes

stephenquan added 2 commits December 12, 2024 19:51

Miscellaneous fixes for Copilot review

ebf0d3a

Merge branch 'main' of github.com:stephenquan/Maui

91bd65b

brminnick added 3 commits January 2, 2025 10:27

Merge branch 'main' into pr/2182

c59b197

Update to .NET 9

241ed90

Use Primary Constructor

11e9d72

brminnick removed the help wanted This proposal has been approved and is ready to be implemented label Jan 2, 2025

Merge branch 'main' into main

53510d9

brminnick requested a review from Copilot January 2, 2025 18:54

Copilot AI reviewed Jan 2, 2025

View reviewed changes

brminnick added 4 commits January 2, 2025 10:58

Remove redundant CodeAnalysis

3e5b075

Update Regex naming

5103c68

Use Index Operator

2967e7c

Merge branch 'main' of https://github.com/stephenquan/Maui into pr/2182

46e7b38

brminnick reviewed Jan 2, 2025

View reviewed changes

brminnick requested review from Copilot January 2, 2025 19:30

Copilot AI reviewed Jan 2, 2025

View reviewed changes

brminnick added 2 commits January 3, 2025 15:03

Update namings

0bc4544

Refactor

3d6c9c8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to PEG parser. Introduce boolean operators and constants. #2182

Migrate to PEG parser. Introduce boolean operators and constants. #2182

stephenquan commented Sep 5, 2024 •

edited

Loading

stephenquan commented Sep 5, 2024

stephenquan commented Sep 16, 2024

pictos commented Sep 17, 2024

stephenquan commented Sep 18, 2024

pictos left a comment

stephenquan commented Sep 25, 2024 •

edited

Loading

stephenquan commented Oct 6, 2024 •

edited

Loading

stephenquan commented Oct 20, 2024 •

edited

Loading

pictos left a comment

stephenquan commented Dec 5, 2024

stephenquan commented Dec 6, 2024

brminnick commented Dec 18, 2024

brminnick Jan 2, 2025 •

edited

Loading

brminnick Jan 2, 2025

stephenquan Jan 4, 2025 •

edited

Loading

stephenquan Jan 4, 2025 •

edited

Loading

bijington Jan 4, 2025

Migrate to PEG parser. Introduce boolean operators and constants. #2182

Are you sure you want to change the base?

Migrate to PEG parser. Introduce boolean operators and constants. #2182

Conversation

stephenquan commented Sep 5, 2024 • edited Loading

Description of Change

Linked Issues

PR Checklist

Additional information

stephenquan commented Sep 5, 2024

stephenquan commented Sep 16, 2024

pictos commented Sep 17, 2024

stephenquan commented Sep 18, 2024

pictos left a comment

Choose a reason for hiding this comment

stephenquan commented Sep 25, 2024 • edited Loading

stephenquan commented Oct 6, 2024 • edited Loading

stephenquan commented Oct 20, 2024 • edited Loading

pictos left a comment

Choose a reason for hiding this comment

stephenquan commented Dec 5, 2024

stephenquan commented Dec 6, 2024

Choose a reason for hiding this comment

brminnick commented Dec 18, 2024

Choose a reason for hiding this comment

brminnick Jan 2, 2025 • edited Loading

Choose a reason for hiding this comment

brminnick Jan 2, 2025

Choose a reason for hiding this comment

stephenquan Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

stephenquan Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

bijington Jan 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephenquan commented Sep 5, 2024 •

edited

Loading

stephenquan commented Sep 25, 2024 •

edited

Loading

stephenquan commented Oct 6, 2024 •

edited

Loading

stephenquan commented Oct 20, 2024 •

edited

Loading

brminnick Jan 2, 2025 •

edited

Loading

stephenquan Jan 4, 2025 •

edited

Loading

stephenquan Jan 4, 2025 •

edited

Loading