[XSG] Pre-compute x:DataType with PropagateDataTypeVisitor#34740
Draft
simonrozsival wants to merge 6 commits intonet11.0from
Draft
[XSG] Pre-compute x:DataType with PropagateDataTypeVisitor#34740simonrozsival wants to merge 6 commits intonet11.0from
simonrozsival wants to merge 6 commits intonet11.0from
Conversation
When a binding uses Source={x:Reference}, resolve the referenced element's
type by walking namescopes and compile the binding against it. This enables
compiled bindings for paths like Path=Text on a referenced Label.
When the path can't be fully resolved (e.g. Path=BindingContext.X where
BindingContext is 'object'), fall back silently to runtime binding without
emitting MAUIG2045 — these bindings were never compiled before, so a new
warning would be a regression.
The MAUIG2045 diagnostic is moved from TryParsePath to the caller via an
out parameter, letting the caller decide whether to emit it (only for
x:DataType-sourced bindings, not x:Reference).
Fixes #34490
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34740Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34740" |
Member
Author
DependenciesThis PR should be rebased on top of #34513 once it merges — there is overlapping code in After #34513 merges, we can adopt its |
…assignments in XAML (#32654) <!-- 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! Fixes #3059 ## Description of Change Adds XC0067/MAUIX2006 warning diagnostic to detect when XAML **implicit content properties** are set multiple times (e.g., `<Border><Label/><Label/></Border>`). This implementation specifically targets the issue described in #3059: detecting multiple children in single-child content properties. ## Key Changes - **Focused implementation**: Only warns for implicit content property duplicates - **Does NOT warn for**: Explicit attribute duplicates like `<Label Text="A" Text="B" />` (XAML parser already rejects these at parse time) - **Collection-aware**: Correctly handles collection properties (e.g., `Style.Setters`, `VisualStateGroup.States`) - no warnings when adding multiple items - **Works for both implicit and explicit content property syntax**: - `<Border><Label/><Label/></Border>` → warns about `Border.Content` - `<Border><Border.Content><Label/><Label/></Border.Content></Border>` → warns about `Border.Content` ## Implementation Details - Duplicate detection only in `Visit(ElementNode)` for implicit content properties - Removed checks for explicit attribute assignments (ValueNode) - not needed - Added property type inspection to detect collection properties - Collections use `Add()` method, so multiple children are expected and don't trigger warnings - Improved NoWarn parsing to properly handle comma/semicolon-delimited codes - Implemented in both Build.Tasks and SourceGen ## Testing - Added comprehensive unit tests in SourceGen.UnitTests - Tests verify warnings for single-value content properties - Tests verify NO warnings for collection properties - Tests verify NO warnings for single children - Removed unnecessary tests for explicit attribute duplicates ## Issues Fixed Fixes #3059
Add BindingContextDataType record and PropagateDataTypeVisitor that
pre-computes x:DataType for all ElementNodes in the XAML tree. This is
a new visitor in the SourceGen pipeline that runs after
SetNamescopesAndRegisterNamesVisitor and before SetResourcesVisitor.
The visitor:
- Resolves x:DataType declarations (string names, {x:Type}, {x:Null})
- Propagates resolved types to children (top-down)
- Respects DoesNotInheritDataType boundaries
- Tracks DataTemplate boundary crossings
- Handles x:DataType={x:Null} as explicit opt-out (blocks inheritance)
Also integrates into template re-inflation in SetPropertiesVisitor,
seeding child contexts with the parent's BindingContextDataType.
No behavior change yet — the dictionary is populated but not consumed
by binding compilation. That refactor comes next.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the TryGetXDataType/TryFindXDataTypeNode tree walk in
ProvideValueForBindingExtension with an O(1) dictionary lookup into
context.BindingContextDataTypes, pre-computed by PropagateDataTypeVisitor.
Key changes:
- Remove TryGetXDataType, TryFindXDataTypeNode, DoesNotInheritDataType
local functions from KnownMarkups (replaced by the visitor)
- Add TryGetBindingContextDataType with BindingContext={Binding} skip
logic: looks up grandparent's type when target is BindingContext
- Add PropagateDataTypeVisitor to lazy resource inflation path in
SetPropertyHelpers.AddLazyResourceToResourceDictionary
- Seed BindingContextDataType into lambda contexts for both template
inflation and lazy resource paths
All 200 SourceGen unit tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
New test cases:
- XReferenceDirectProperty: x:Reference with direct property path
(e.g., Slider.Value) does not produce false MAUIG2045
- XReferenceInDataTemplate: x:Reference inside DataTemplate does not
resolve against the DataTemplate's x:DataType
- XReferenceNonExistentProperty: non-existent property on referenced
element correctly reports MAUIG2045
- XDataTypeExplicitNull: x:DataType={x:Null} blocks inheritance and
prevents compilation (no MAUIG2045)
All 204 SourceGen unit tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match SetPropertiesVisitor behavior: copy parentDataType as-is instead of forcing CrossedTemplateBoundary=true. The visitor handles the flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
dev/simonrozsival/propagate-binding-context-datatype
branch
from
f27ab72 to
efdf50d
Compare
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Adds a
PropagateDataTypeVisitorto the XAML Source Generator pipeline that pre-computesx:DataTypefor everyElementNodein the tree. This replaces the per-binding tree walk with an O(1) dictionary lookup.Part of #34696. Based on #34513.
What changed
BindingContextDataTypemodel — new record withResolved/ExplicitNull/Unresolvedstates, stored per-node inSourceGenContext.BindingContextDataTypesPropagateDataTypeVisitor— top-down visitor that resolvesx:DataTypedeclarations and propagates them to children, respectingDoesNotInheritDataTypeboundaries, DataTemplate scope crossings, andx:DataType="{x:Null}"opt-outs. Reports diagnostics for unresolvablex:DataTypevalues.Refactored binding compilation —
TryGetXDataType/TryFindXDataTypeNodetree walk replaced with dictionary lookup. Removed ~130 lines of redundant code.Pipeline integration — visitor runs in the main pipeline, template inflation (
SetPropertiesVisitor), and lazy resource inflation (SetPropertyHelpers). Template and lazy resource contexts are seeded with the parent'sBindingContextDataType.Not yet implemented (follow-up)
Path=BindingContext.SelectCommandwithSource={x:Reference PageRoot}needs path rewriting to generate((VM)source.BindingContext).SelectCommandx:DataTypefromItemsSourcecollection generic types ([Feature] Auto compiled binding on CollectionView's ItemTemplate, GroupHeaderTemplate and GroupFooterTemplate #8161)Tests
All 205 SourceGen unit tests pass (201 from #34513 + 4 new).
New tests:
XReferenceDirectProperty_DoesNotFallBackSilentlyXReferenceInDataTemplate_DoesNotReportFalsePositiveXReferenceNonExistentProperty_FallsBackToRuntimeXDataTypeExplicitNull_BlocksInheritance