[XSG] Fix invalid code generation for Setter with OnPlatform without Default#33681
Merged
[XSG] Fix invalid code generation for Setter with OnPlatform without Default#33681
Conversation
…out Default Fixes #33676 When a Setter uses {OnPlatform} without a Default value and the target platform doesn't match, SimplifyOnPlatformVisitor removes the Value property. However, SetterValueProvider.CanProvideValue() incorrectly returned true for this case (null is neither MarkupNode nor ElementNode), causing invalid code generation like: ((IValueProvider)).ProvideValue(...) The fix adds a null check in CanProvideValue() to return false when there's no value node, preventing the invalid code path.
StephaneDelcroix
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in SourceGen where Setter elements using {OnPlatform} without a Default value would generate invalid C# code when the target platform didn't match any specified platforms. The fix adds a null check in SetterValueProvider.CanProvideValue() to prevent attempting to generate code for setters with no value.
Changes:
- Added null check in
SetterValueProvider.CanProvideValue()to return false whenGetValueNode()returns null - Added comprehensive test coverage for both non-matching and matching platform scenarios
- Test verifies that invalid code is not generated and that matching platforms still work correctly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Controls/src/SourceGen/SetterValueProvider.cs | Added null check in CanProvideValue() to handle cases where OnPlatform removes the Value property (lines 19-22) |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui33676.xaml | XAML test file with Setter using OnPlatform without Default value for iOS platform only |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui33676.xaml.cs | Test file with two test methods: one verifying no invalid code is generated for non-matching platforms (Android), and one verifying correct behavior for matching platforms (iOS) |
StephaneDelcroix
force-pushed
the
fix/sourcegen-onplatform-setter-33676
branch
from
582ea08 to
83c7aa5
Compare
When a Setter inside a VisualState.Setters has an OnPlatform value with no matching platform and no Default, the Setter should be skipped entirely rather than falling through to IValueProvider handling. The fix: 1. SetterValueProvider.TryProvideValue now returns true with null returnType when valueNode is null (signaling skip) 2. NodeSGExtensions.TryProvideValue detects this sentinel and removes the node from Variables 3. SetPropertiesVisitor and SetPropertyHelpers check if node exists in Variables before adding to collection Adds unit test Maui33676_VisualState for the VisualState.Setters case.
simonrozsival
approved these changes
Jan 23, 2026
Member
|
/azp run maui-pr-uitests, maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
This was referenced Jan 23, 2026
PureWeen
approved these changes
Jan 23, 2026
StephaneDelcroix
added a commit
that referenced
this pull request
Feb 14, 2026
Fixes #34039 When <Setter.Value> is used as a property element with a namespace URI other than MauiUri (e.g., MauiGlobalUri with implicit xmlns), GetValueNode() failed to find it. Since PR #33681 changed the null return to a skip sentinel, this caused the Setter to be removed from Variables entirely, preventing the .Add() call from being generated. The fix makes GetValueNode() iterate all properties matching LocalName == "Value" instead of checking specific namespace URIs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 #33676
When a
Setteruses{OnPlatform}without aDefaultvalue and the target platform doesn't match any specified platforms, SourceGen generates invalid C# code like:This causes compilation errors:
CS0119: 'IValueProvider' is a type, which is not valid in the given contextCS0120: An object reference is required for the non-static memberRoot Cause
When
{OnPlatform}has no matching platform and no Default,SimplifyOnPlatformVisitorremoves theValueproperty from the Setter. However,SetterValueProvider.CanProvideValue()incorrectly returnedtruefor this case (becausenullis neitherMarkupNodenorElementNode), causingCreateValuesVisitorto register a placeholder with an empty name. Later,ProvideValuegenerated invalid code using that empty name.Fix
Added a null check in
CanProvideValue()to returnfalsewhen there's no value node, preventing the invalid code path.Testing
Maui33676that reproduces the issue