[XSG] Fix expression binding TProperty type resolution for mismatched source/target types#33994
Conversation
When binding Entry.Text (string) to a decimal ViewModel property via C# expression syntax, the source generator incorrectly used the target BindableProperty type (string) as TProperty in TypedBinding, causing CS0029 compilation errors. Now resolves the expression's result type by walking the property chain on the x:DataType symbol, matching XamlC behavior. The TypedBinding infrastructure handles type conversion at runtime. Fixes #33992
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the XAML Source Generator where expression bindings incorrectly used the target BindableProperty type instead of the source expression type for the TProperty generic parameter in TypedBinding<TSource, TProperty>. This caused compilation errors when binding properties with different types (e.g., Entry.Text string to a decimal ViewModel property).
Changes:
- Added
ResolveExpressionType()method to walk property chains and determine the actual expression result type (e.g.,decimalforPrice,stringforUser.DisplayName) - Modified
SetExpressionBinding()to use the resolved expression type instead of the target BindableProperty type - Added comprehensive tests for two-way decimal binding via C# expression syntax (VM→UI, UI→VM, INPC)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Controls/src/SourceGen/SetPropertyHelpers.cs | Added ResolveExpressionType() to resolve expression result types and updated SetExpressionBinding() to use it instead of GetBPTypeAndConverter() |
| src/Controls/tests/Xaml.UnitTests/CSharpExpressions.sgen.xaml | Added test Entry element with two-way decimal binding within the x:DataType="local:SimpleViewModel" section |
| src/Controls/tests/Xaml.UnitTests/CSharpExpressions.sgen.xaml.cs | Added three test methods verifying decimal type conversion works correctly in both directions and with property change notifications |
| // Strip leading dot prefix (e.g., ".Name" → "Name") | ||
| if (expr.StartsWith(".", StringComparison.Ordinal)) |
There was a problem hiding this comment.
The leading dot stripping logic should check for the ".." range operator to avoid incorrectly transforming expressions like "{..5}". This matches the pattern in ExpressionAnalyzer.cs which includes a check for ".." before stripping the leading dot.
The current implementation would incorrectly transform "{..5}" to "{.5}", which would then fail when trying to resolve ".5" as a member name.
| // Strip leading dot prefix (e.g., ".Name" → "Name") | |
| if (expr.StartsWith(".", StringComparison.Ordinal)) | |
| // Strip leading dot prefix (e.g., ".Name" → "Name"), but avoid the ".." range operator | |
| if (expr.StartsWith(".", StringComparison.Ordinal) && | |
| !expr.StartsWith("..", StringComparison.Ordinal)) |
| // Use decimal.Parse to handle locale-specific decimal separators | ||
| Assert.Equal(42.5m, decimal.Parse(page.twoWayDecimalEntry.Text)); |
There was a problem hiding this comment.
Using decimal.Parse without specifying a culture could cause test failures in locales that use comma as decimal separator (e.g., "42,5" in German). Consider using decimal.Parse(page.twoWayDecimalEntry.Text, CultureInfo.InvariantCulture) to ensure consistent test behavior across all locales, or use Assert.Equal(42.5m.ToString(), page.twoWayDecimalEntry.Text) to compare the string representations directly.
| var vm = new SimpleViewModel { Price = 10m }; | ||
| page.BindingContext = vm; | ||
|
|
||
| Assert.Equal(10m, decimal.Parse(page.twoWayDecimalEntry.Text)); |
There was a problem hiding this comment.
Using decimal.Parse without specifying a culture could cause test failures in locales that use comma as decimal separator. Consider using decimal.Parse(page.twoWayDecimalEntry.Text, CultureInfo.InvariantCulture) to ensure consistent test behavior across all locales.
| Assert.Equal(10m, decimal.Parse(page.twoWayDecimalEntry.Text)); | ||
|
|
||
| vm.Price = 25.75m; | ||
| Assert.Equal(25.75m, decimal.Parse(page.twoWayDecimalEntry.Text)); |
There was a problem hiding this comment.
Using decimal.Parse without specifying a culture could cause test failures in locales that use comma as decimal separator. Consider using decimal.Parse(page.twoWayDecimalEntry.Text, CultureInfo.InvariantCulture) to ensure consistent test behavior across all locales.
simonrozsival
left a comment
There was a problem hiding this comment.
I would like to reuse the binding path parsing from the {Binding ...} known markup extension compilation code. We can do that later - I'll make sure to open a follow-up issue later today.
… in decimal.Parse
… source/target types (dotnet#33994) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Fixes dotnet#33992 When using XAML C# expression syntax to bind a property whose type differs from the target BindableProperty type (e.g., `Entry.Text` string to a `decimal` VM property), the source generator used the **target BP type** as `TProperty` in `TypedBinding<TSource, TProperty>`, causing `CS0029` compilation errors. ```xml <!-- This failed to compile with XSG --> <Entry Text="{Price}" /> ``` ### Root Cause `SetExpressionBinding()` called `bpFieldSymbol.GetBPTypeAndConverter()` and used the result (`string` for `Entry.TextProperty`) as `TProperty`. The getter lambda `(__source) => __source.Price` returns `decimal`, which cannot implicitly convert to `string`. ### Fix Added `ResolveExpressionType()` that walks the property chain on the `x:DataType` symbol to determine the expression's result type (e.g., `decimal` for `Price`, `string` for `User.DisplayName`). This matches how XamlC resolves property types via `TryParsePath()`. The `TypedBinding` infrastructure already handles type conversion at runtime via `BindingExpressionHelper.TryConvert()`, so using the correct source type is all that's needed. Handles: bare identifiers (`Price`), dot-prefixed (`.Name`), `BindingContext.` prefix, null-conditional (`User?.Name`). Falls back to `object` for complex expressions (operators, method calls). ### Tests Added 3 tests for two-way decimal binding via C# expression syntax: - **VM to UI**: Setting `vm.Price = 42.5m` updates `Entry.Text` - **UI to VM**: `SetValueFromRenderer("100")` updates `vm.Price` to `100m` - **INPC**: Changing `vm.Price` after initial binding updates `Entry.Text` All 1874 Xaml.UnitTests pass (0 failures, 8 skipped).
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
Fixes #33992
When using XAML C# expression syntax to bind a property whose type differs from the target BindableProperty type (e.g.,
Entry.Textstring to adecimalVM property), the source generator used the target BP type asTPropertyinTypedBinding<TSource, TProperty>, causingCS0029compilation errors.Root Cause
SetExpressionBinding()calledbpFieldSymbol.GetBPTypeAndConverter()and used the result (stringforEntry.TextProperty) asTProperty. The getter lambda(__source) => __source.Pricereturnsdecimal, which cannot implicitly convert tostring.Fix
Added
ResolveExpressionType()that walks the property chain on thex:DataTypesymbol to determine the expression's result type (e.g.,decimalforPrice,stringforUser.DisplayName). This matches how XamlC resolves property types viaTryParsePath().The
TypedBindinginfrastructure already handles type conversion at runtime viaBindingExpressionHelper.TryConvert(), so using the correct source type is all that's needed.Handles: bare identifiers (
Price), dot-prefixed (.Name),BindingContext.prefix, null-conditional (User?.Name). Falls back toobjectfor complex expressions (operators, method calls).Tests
Added 3 tests for two-way decimal binding via C# expression syntax:
vm.Price = 42.5mupdatesEntry.TextSetValueFromRenderer("100")updatesvm.Priceto100mvm.Priceafter initial binding updatesEntry.TextAll 1874 Xaml.UnitTests pass (0 failures, 8 skipped).