[net11.0] Compile x:Reference bindings against resolved element type#34513
[net11.0] Compile x:Reference bindings against resolved element type#34513StephaneDelcroix merged 1 commit intonet11.0from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34513Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34513" |
There was a problem hiding this comment.
Pull request overview
Enhances the XAML source generator on net11.0 to compile Binding expressions that use Source={x:Reference ...} by resolving the referenced element’s type (via namescopes) and using that type for binding-path compilation, while suppressing MAUIG2045 for x:Reference scenarios that can’t be resolved statically.
Changes:
- Update binding compilation to resolve
x:Referencesource element types and compile against them (falling back to runtime binding without MAUIG2045 regressions). - Refactor compiled-binding diagnostics so MAUIG2045 can be returned to the caller (enabling conditional suppression/reporting).
- Add/update unit tests covering x:Reference bindings in templates and type-resolution scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/SourceGen/KnownMarkups.cs | Resolves x:Reference source types and conditionally suppresses/report MAUIG2045 based on binding origin. |
| src/Controls/src/SourceGen/CompiledBindingMarkup.cs | Changes compiled-binding path parsing to return (not directly emit) property-not-found diagnostics. |
| src/Controls/tests/SourceGen.UnitTests/BindingDiagnosticsTests.cs | Adds generator-level tests to ensure no false-positive MAUIG2045 and correct referenced-type resolution. |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui34490.xaml | Adds XAML reproduction for the DataTemplate + x:Reference scenario. |
| src/Controls/tests/Xaml.UnitTests/Issues/Maui34490.xaml.cs | Adds XAML unit test asserting diagnostics behavior for the reproduced scenario. |
| src/Controls/tests/Xaml.UnitTests/Issues/Gh3606.xaml.cs | Updates an existing issue test intended to validate x:Reference source-type-based compilation. |
| // BindingContext is 'object' on BindableObject, so SelectItemCommand warning on 'object' is expected, | ||
| // but a warning mentioning Maui34490ItemModel would mean it's still resolving against the wrong type. | ||
| Assert.DoesNotContain(result.Diagnostics, d => d.Id == "MAUIG2045" && d.GetMessage().Contains("Maui34490ItemModel", StringComparison.Ordinal)); |
| // When Source={x:Reference Name} is set on a binding, resolves the referenced element's type | ||
| // by walking namescopes (same logic as ProvideValueForReferenceExtension). | ||
| // Returns null if Source is not an x:Reference or the name cannot be resolved. | ||
| static ITypeSymbol? TryResolveXReferenceSourceType(ElementNode bindingNode, SourceGenContext context) | ||
| { | ||
| if (!bindingNode.Properties.TryGetValue(new XmlName("", "Source"), out INode? sourceNode) | ||
| && !bindingNode.Properties.TryGetValue(new XmlName(null, "Source"), out sourceNode)) | ||
| return null; | ||
|
|
||
| if (sourceNode is not ElementNode refNode) | ||
| return null; | ||
|
|
||
| if (refNode.XmlType.Name is not "ReferenceExtension" and not "Reference") | ||
| return null; | ||
|
|
||
| // Extract the Name from the x:Reference markup | ||
| if (!refNode.Properties.TryGetValue(new XmlName("", "Name"), out INode? refNameNode) | ||
| && !refNode.Properties.TryGetValue(new XmlName(null, "Name"), out refNameNode)) | ||
| { | ||
| refNameNode = refNode.CollectionItems.Count > 0 ? refNode.CollectionItems[0] : null; | ||
| } | ||
|
|
||
| if (refNameNode is not ValueNode vn || vn.Value is not string name) | ||
| return null; | ||
|
|
||
| // Walk namescopes to find the referenced element's type | ||
| ElementNode? node = bindingNode; | ||
| var currentContext = context; | ||
| while (currentContext is not null && node is not null) | ||
| { | ||
| while (currentContext is not null && !currentContext.Scopes.ContainsKey(node)) | ||
| currentContext = currentContext.ParentContext; | ||
| if (currentContext is null) | ||
| break; | ||
| var namescope = currentContext.Scopes[node]; | ||
| if (namescope.namesInScope != null && namescope.namesInScope.ContainsKey(name)) | ||
| return namescope.namesInScope[name].Type; | ||
| INode n = node; | ||
| while (n.Parent is ListNode ln) | ||
| n = ln.Parent; | ||
| node = n.Parent as ElementNode; | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
I agree with copilot here
| internal void BindingsWithXReferenceSourceResolveAgainstReferencedType(XamlInflator inflator) | ||
| { | ||
| // Source={x:Reference page} points to ContentPage, which has a Content property. | ||
| // The binding path "Content" resolves against ContentPage, so the source generator | ||
| // compiles it instead of falling back to runtime Binding. | ||
| var view = new Gh3606(inflator); | ||
|
|
||
| var binding = view.Label.GetContext(Label.TextProperty).Bindings.GetValue(); | ||
| Assert.IsType<Binding>(binding); | ||
| Assert.IsAssignableFrom<BindingBase>(binding); | ||
| } |
There was a problem hiding this comment.
I agree with copilot here, we should assert that the binding was compiled.
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>
893c4b9 to
6df5147
Compare
| if (!bindingNode.Properties.TryGetValue(new XmlName("", "Source"), out INode? sourceNode) | ||
| && !bindingNode.Properties.TryGetValue(new XmlName(null, "Source"), out sourceNode)) |
There was a problem hiding this comment.
One day I'll make copilot an ext method TryGetPropertyNode("Source", out INode? sourceNode) 😆
simonrozsival
left a comment
There was a problem hiding this comment.
I think we could take this a step further and detect the type of BindingContext on the referenced node and correctly cast the BindingContext in the path. I can look into this in a follow up PR though.
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
Follow-up to #34501 (which targets
mainwith a minimal fix).This PR enhances x:Reference binding compilation for
net11.0by resolving the referenced element's type and compiling the binding against it, instead of skipping compilation entirely.What it does
When a binding uses
Source={x:Reference Name}, the source generator now:ContentPageforx:Reference PageRoot,Labelforx:Reference StatusLabel)Path=Texton aLabel)Path=BindingContext.SelectItemCommandwhereBindingContextisobject)Key design decision:
out Diagnostic?onTryParsePathThe MAUIG2045 ("property not found") diagnostic is now returned as an
outparameter fromTryParsePath/TryCompileBindinginstead of being emitted directly. This lets the caller decide:Tests added
BindingWithXReferenceSourceInDataTemplate_DoesNotReportFalsePositive— verifies no false MAUIG2045 forPath=BindingContext.XBindingWithXReferenceToNonRootElement_ResolvesCorrectType— verifiesPath=Textagainst a referencedLabelcompiles with no warningsMaui34490.xaml— XAML unit test reproducing the original issueGh3606test — binding toContentvia x:Reference now compilesFixes #34490