-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix for RelativeSource AncestorType bindings not generating compiled bindings under AOT #34408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: net11.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -343,20 +343,37 @@ private static bool ProvideValueForBindingExtension(ElementNode markupNode, Inde | |
| returnType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.BindingBase")!; | ||
| ITypeSymbol? dataTypeSymbol = null; | ||
|
|
||
| // When Source is RelativeSource, the type is determined at runtime — skip compilation. | ||
| // When Source is x:Reference, resolve the referenced element's type and compile against it. | ||
| // Otherwise, use x:DataType from the current scope. | ||
| bool hasRelativeSource = HasRelativeSourceBinding(markupNode); | ||
|
|
||
| context.Variables.TryGetValue(markupNode, out ILocalValue? extVariable); | ||
|
|
||
| if ( !hasRelativeSource | ||
| && extVariable is not null) | ||
| if (extVariable is not null) | ||
| { | ||
| ITypeSymbol? xRefSourceType = TryResolveXReferenceSourceType(markupNode, context); | ||
| dataTypeSymbol = xRefSourceType; | ||
| if (dataTypeSymbol is null) | ||
| TryGetXDataType(markupNode, context, out dataTypeSymbol); | ||
| // Determine the source type for compiled binding based on the binding's Source configuration: | ||
| // | ||
| // 1. RelativeSource with a resolvable AncestorType: use the AncestorType as the source | ||
| // type. The symbol is already registered in context.Types by | ||
| // ProvideValueForRelativeSourceExtension, enabling trim-safe TypedBinding generation. | ||
| // | ||
| // 2. RelativeSource without AncestorType (Self, TemplatedParent, or FindAncestor without | ||
| // a type): the binding source is resolved at runtime. Using x:DataType as the source | ||
| // type here would produce a compiled binding with an incorrect source type, leading to | ||
| // runtime failures. Fall through to the string-based Binding path instead. | ||
| // | ||
| // 3. x:Reference: resolve the referenced element's type and compile against it. | ||
| // | ||
| // 4. No explicit source: use x:DataType if available to produce a compiled TypedBinding. | ||
| ITypeSymbol? xRefSourceType = null; | ||
| if (TryGetRelativeSourceAncestorType(markupNode, context, out var ancestorTypeSymbol) | ||
| && ancestorTypeSymbol is not null) | ||
| { | ||
| dataTypeSymbol = ancestorTypeSymbol; | ||
| } | ||
| else if (!HasRelativeSourceBinding(markupNode)) | ||
| { | ||
| xRefSourceType = TryResolveXReferenceSourceType(markupNode, context); | ||
| dataTypeSymbol = xRefSourceType; | ||
| if (dataTypeSymbol is null) | ||
| TryGetXDataType(markupNode, context, out dataTypeSymbol); | ||
| } | ||
|
|
||
| if (dataTypeSymbol is not null) | ||
| { | ||
|
|
@@ -708,6 +725,50 @@ static bool HasRelativeSourceBinding(ElementNode bindingNode) | |
|
|
||
| return null; | ||
| } | ||
|
|
||
| // Checks if the binding has a Source property that is a RelativeSource extension | ||
| // with a resolvable AncestorType. If so, returns the already-resolved AncestorType | ||
| // symbol from context.Types (populated earlier by ProvideValueForRelativeSourceExtension). | ||
| // This allows AncestorType bindings to use the compiled (trim-safe) TypedBinding path. | ||
| static bool TryGetRelativeSourceAncestorType(ElementNode bindingNode, SourceGenContext context, out ITypeSymbol? ancestorType) | ||
| { | ||
| ancestorType = null; | ||
|
|
||
| // Check if Source property exists | ||
| if (!bindingNode.Properties.TryGetValue(new XmlName("", "Source"), out INode? sourceNode) | ||
| && !bindingNode.Properties.TryGetValue(new XmlName(null, "Source"), out sourceNode)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the Source is a RelativeSourceExtension | ||
| if (sourceNode is not ElementNode relativeSourceNode | ||
| || (relativeSourceNode.XmlType.Name != "RelativeSourceExtension" | ||
| && relativeSourceNode.XmlType.Name != "RelativeSource")) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Find the AncestorType property on the RelativeSource node | ||
| if (!relativeSourceNode.Properties.TryGetValue(new XmlName("", "AncestorType"), out INode? ancestorTypeNode) | ||
| && !relativeSourceNode.Properties.TryGetValue(new XmlName(null, "AncestorType"), out ancestorTypeNode)) | ||
| relativeSourceNode.Properties.TryGetValue(new XmlName(XamlParser.MauiUri, "AncestorType"), out ancestorTypeNode); | ||
|
|
||
| if (ancestorTypeNode is null) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // The AncestorType is typically an x:Type extension (ElementNode). | ||
| // ProvideValueForRelativeSourceExtension already resolved this type | ||
| // and registered it in context.Types — just look it up. | ||
| if (ancestorTypeNode is ElementNode typeExtNode) | ||
|
BagavathiPerumal marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This isn't a regression (they weren't compiled before either) — just a missed enhancement for a rarer syntax. If you choose to support it, add a |
||
| { | ||
| return context.Types.TryGetValue(typeExtNode, out ancestorType) && ancestorType is not null; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
|
|
||
| internal static bool ProvideValueForDataTemplateExtension(ElementNode markupNode, IndentedTextWriter writer, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate? getNodeValue, out ITypeSymbol? returnType, out string value) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| xmlns:local="clr-namespace:Microsoft.Maui.Controls.Xaml.UnitTests" | ||
| x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui34056" | ||
| x:DataType="local:Maui34056PageViewModel"> | ||
| <VerticalStackLayout> | ||
| <!-- Scenario 1: RelativeSource AncestorType inside DataTemplate with x:DataType (issue fix) --> | ||
| <CollectionView x:Name="TestCollectionView" | ||
| ItemsSource="{Binding Items}"> | ||
| <CollectionView.ItemTemplate> | ||
| <DataTemplate x:DataType="local:Maui34056ItemViewModel"> | ||
| <Button x:Name="TestButton" | ||
| Text="{Binding ItemName}" | ||
| Command="{Binding x:DataType='local:Maui34056PageViewModel', Source={RelativeSource AncestorType={x:Type local:Maui34056PageViewModel}}, Path=TestCommand}" /> | ||
| </DataTemplate> | ||
| </CollectionView.ItemTemplate> | ||
| </CollectionView> | ||
|
|
||
| <!-- Scenario 2: {RelativeSource Self} inside DataTemplate with x:DataType. | ||
| SourceGen must not use x:DataType as the source type; the source is the element itself. | ||
| Path=ItemName exists on Maui34056ItemViewModel to ensure the guard, not a failed lookup, prevents compiled binding. --> | ||
| <CollectionView x:Name="SelfBindingCollectionView" | ||
| ItemsSource="{Binding Items}"> | ||
| <CollectionView.ItemTemplate> | ||
| <DataTemplate x:DataType="local:Maui34056ItemViewModel"> | ||
| <Label x:Name="SelfBindingLabel" | ||
| Text="{Binding Path=ItemName, Source={RelativeSource Self}}" /> | ||
| </DataTemplate> | ||
| </CollectionView.ItemTemplate> | ||
| </CollectionView> | ||
| </VerticalStackLayout> | ||
| </ContentPage> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| using System.Collections.ObjectModel; | ||
| using System.Windows.Input; | ||
| using Microsoft.Maui.Controls.Internals; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Maui.Controls.Xaml.UnitTests; | ||
|
|
||
| // Page-level ViewModel — this is what the RelativeSource AncestorType points to | ||
| public class Maui34056PageViewModel | ||
| { | ||
| public ObservableCollection<Maui34056ItemViewModel> Items { get; } = | ||
| [new Maui34056ItemViewModel { ItemName = "Item 1" }]; | ||
|
|
||
| public ICommand TestCommand { get; } = new Command(() => { }); | ||
| } | ||
|
|
||
| // Item ViewModel — this is what the DataTemplate's x:DataType is set to | ||
| public class Maui34056ItemViewModel | ||
| { | ||
| public string ItemName { get; set; } = ""; | ||
| } | ||
|
|
||
| public partial class Maui34056 : ContentPage | ||
| { | ||
| public Maui34056() | ||
| { | ||
| InitializeComponent(); | ||
| BindingContext = new Maui34056PageViewModel(); | ||
| } | ||
|
|
||
| [Collection("Issue")] | ||
| public class Maui34056Tests | ||
|
BagavathiPerumal marked this conversation as resolved.
|
||
| { | ||
| [Theory] | ||
| [XamlInflatorData] | ||
| internal void RelativeSourceAncestorTypeInDataTemplateGeneratesCompiledBinding(XamlInflator inflator) | ||
| { | ||
| var page = new Maui34056(inflator); | ||
|
|
||
| var template = ((CollectionView)page.TestCollectionView).ItemTemplate; | ||
| var content = template.CreateContent() as Button; | ||
| Assert.NotNull(content); | ||
|
|
||
| var bindingContext = content.GetContext(Button.CommandProperty); | ||
| Assert.NotNull(bindingContext); | ||
| var binding = bindingContext.Bindings.GetValue(); | ||
|
|
||
| if (inflator is XamlInflator.Runtime) | ||
| { | ||
| // Runtime inflator uses the string-based Binding — no compile-time type info available. | ||
| Assert.IsType<Binding>(binding); | ||
| } | ||
| else | ||
| { | ||
| // Both XamlC and SourceGen produce a trim-safe TypedBinding when x:DataType is present | ||
| // on the binding node alongside RelativeSource AncestorType. | ||
| Assert.IsType<TypedBinding<Maui34056PageViewModel, ICommand>>(binding); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: this asserts only the binding's runtime kind ( |
||
| } | ||
| } | ||
|
|
||
| [Theory] | ||
| [XamlInflatorData] | ||
| internal void RelativeSourceSelfInDataTemplateWithXDataTypeUsesStringBinding(XamlInflator inflator) | ||
| { | ||
| // Verifies SourceGen does not use the DataTemplate's x:DataType as the source type for | ||
| // {RelativeSource Self} bindings. The source is the element itself, resolved at runtime. | ||
| // Path=ItemName exists on Maui34056ItemViewModel to ensure the guard is what prevents | ||
| // compiled binding, not a failed type lookup. XamlC behavior is pre-existing and separate. | ||
| var page = new Maui34056(inflator); | ||
|
|
||
| var template = ((CollectionView)page.SelfBindingCollectionView).ItemTemplate; | ||
| var content = template.CreateContent() as Label; | ||
| Assert.NotNull(content); | ||
|
|
||
| var bindingContext = content.GetContext(Label.TextProperty); | ||
| Assert.NotNull(bindingContext); | ||
| var binding = bindingContext.Bindings.GetValue(); | ||
|
|
||
| if (inflator is XamlInflator.XamlC) | ||
| { | ||
| // XamlC pre-existing behavior: compiles RelativeSource Self using DataTemplate x:DataType. | ||
| // This is a separate issue, not addressed by this fix. | ||
| Assert.IsType<TypedBinding<Maui34056ItemViewModel, string>>(binding); | ||
| } | ||
| else | ||
| { | ||
| // Runtime: no compile-time type info, always string-based Binding. | ||
| // SourceGen (the fix): HasRelativeSourceBinding blocks x:DataType path for Self bindings. | ||
| Assert.IsType<Binding>(binding); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the AncestorType path is taken here,
xRefSourceTypestaysnull. IfTryCompileBindingthen fails because thePathcan't be resolved on the AncestorType, the guard a few lines below —if (propertyNotFoundDiagnostic is not null && xRefSourceType is null)— is satisfied and MAUIG2045BindingPropertyNotFound(a Warning) is reported (seeCompiledBindingMarkup.TryParsePath).RelativeSource AncestorTypebindings were never compiled before this PR (the oldhasRelativeSourcegate routed them straight to the stringBinding), so this surfaces a new diagnostic for them — and would break builds usingTreatWarningsAsErrors. That's inconsistent with the adjacent x:Reference handling, which deliberately suppresses the same diagnostic for exactly the "never compiled before" reason (see the comment immediately below).TryGetPropertydoes infer CommunityToolkit.Mvvm[ObservableProperty]/[RelayCommand]members, so common MVVM paths are covered; but properties produced by other source generators (or genuine typos) on the AncestorType will now warn where they previously didn't. Was emitting the diagnostic on this path intentional? If not, consider tracking that the source type came from the AncestorType (e.g. a localsourceFromAncestorTypeflag set in this branch) and adding&& !sourceFromAncestorTypeto the suppression condition, mirroring x:Reference.