From c3f02518e5f01650dfeb133f7b36b4dc2214f69d Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Fri, 4 Apr 2025 01:22:41 +0200 Subject: [PATCH] Fix binding null conditional operator not working with AvaloniaProperty (#18583) * Add failing tests for null conditional operator with AvaloniaPropery * Fix null conditional operator with AvaloniaProperty --- .../XamlIlBindingPathHelper.cs | 52 +++--- .../Data/Core/NullConditionalBindingTests.cs | 168 ++++++++++++++++-- 2 files changed, 187 insertions(+), 33 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs index d921184b50..cfb9c11ad5 100644 --- a/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs +++ b/src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/XamlIlBindingPathHelper.cs @@ -203,7 +203,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions : null; propertyType ??= XamlIlAvaloniaPropertyHelper.GetAvaloniaPropertyType(avaloniaPropertyFieldMaybe, context.GetAvaloniaTypes(), lineInfo); - nodes.Add(new XamlIlAvaloniaPropertyPropertyPathElementNode(avaloniaPropertyFieldMaybe, propertyType)); + nodes.Add(new XamlIlAvaloniaPropertyPropertyPathElementNode(avaloniaPropertyFieldMaybe, propertyType, propName.AcceptsNull)); } else if (GetAllDefinedProperties(targetType).FirstOrDefault(p => p.Name == propName.PropertyName) is IXamlProperty clrProperty) { @@ -279,7 +279,8 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } nodes.Add(new XamlIlAvaloniaPropertyPropertyPathElementNode(avaloniaPropertyField, - XamlIlAvaloniaPropertyHelper.GetAvaloniaPropertyType(avaloniaPropertyField, context.GetAvaloniaTypes(), lineInfo))); + XamlIlAvaloniaPropertyHelper.GetAvaloniaPropertyType(avaloniaPropertyField, context.GetAvaloniaTypes(), lineInfo), + attachedProp.AcceptsNull)); break; case BindingExpressionGrammar.SelfNode _: nodes.Add(new SelfPathElementNode(selfType)); @@ -433,6 +434,27 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions } } + private static void EmitPropertyCall(XamlIlEmitContext context, IXamlILEmitter codeGen, bool acceptsNull) + { + // By default use the 2-argument overload of CompiledBindingPathBuilder.Property, + // unless a "?." null conditional operator appears in the path, in which case use + // the 3-parameter version with the `acceptsNull` parameter. This ensures we don't + // get a missing method exception if we run against an old version of Avalonia. + var methodArgumentCount = 2; + + if (acceptsNull) + { + methodArgumentCount = 3; + codeGen.Ldc_I4(1); + } + + codeGen + .EmitCall(context.GetAvaloniaTypes() + .CompiledBindingPathBuilder.GetMethod(m => + m.Name == "Property" && + m.Parameters.Count == methodArgumentCount)); + } + class ScopeRegistrationFinder : IXamlAstVisitor { private Stack _stack = new Stack(); @@ -661,11 +683,13 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions class XamlIlAvaloniaPropertyPropertyPathElementNode : IXamlIlBindingPathElementNode { private readonly IXamlField _field; + private readonly bool _acceptsNull; - public XamlIlAvaloniaPropertyPropertyPathElementNode(IXamlField field, IXamlType propertyType) + public XamlIlAvaloniaPropertyPropertyPathElementNode(IXamlField field, IXamlType propertyType, bool acceptsNull) { _field = field; Type = propertyType; + _acceptsNull = acceptsNull; } public void Emit(XamlIlEmitContext context, IXamlILEmitter codeGen) @@ -673,8 +697,8 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions codeGen.Ldsfld(_field); context.Configuration.GetExtra() .EmitLoadAvaloniaPropertyAccessorFactory(context, codeGen); - codeGen.EmitCall(context.GetAvaloniaTypes() - .CompiledBindingPathBuilder.GetMethod(m => m.Name == "Property")); + + EmitPropertyCall(context, codeGen, _acceptsNull); } public IXamlType Type { get; } @@ -699,23 +723,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions context.Configuration.GetExtra() .EmitLoadInpcPropertyAccessorFactory(context, codeGen); - // By default use the 2-argument overload of CompiledBindingPathBuilder.Property, - // unless a "?." null conditional operator appears in the path, in which case use - // the 3-parameter version with the `acceptsNull` parameter. This ensures we don't - // get a missing method exception if we run against an old version of Avalonia. - var methodArgumentCount = 2; - - if (_acceptsNull) - { - methodArgumentCount = 3; - codeGen.Ldc_I4(1); - } - - codeGen - .EmitCall(context.GetAvaloniaTypes() - .CompiledBindingPathBuilder.GetMethod(m => - m.Name == "Property" && - m.Parameters.Count == methodArgumentCount)); + EmitPropertyCall(context, codeGen, _acceptsNull); } public IXamlType Type => _property.PropertyType; diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/NullConditionalBindingTests.cs b/tests/Avalonia.Base.UnitTests/Data/Core/NullConditionalBindingTests.cs index 818e5ed67d..fc5810dc55 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/NullConditionalBindingTests.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/NullConditionalBindingTests.cs @@ -25,7 +25,7 @@ public class NullConditionalBindingTests [Theory] [InlineData(false)] [InlineData(true)] - public void Should_Report_Error_Without_Null_Conditional_Operator(bool compileBindings) + public void Should_Report_Error_Without_Null_Conditional_Operator_For_Clr_Property(bool compileBindings) { // Testing the baseline: should report a null error without null conditionals. using var app = Start(); @@ -40,7 +40,7 @@ public class NullConditionalBindingTests """; - var data = new First(new Second(null)); + var data = new First { Second = new Second() }; var window = CreateTarget(xaml, data); var textBox = Assert.IsType(window.Content); var error = Assert.IsType(textBox.Error); @@ -56,7 +56,38 @@ public class NullConditionalBindingTests [Theory] [InlineData(false)] [InlineData(true)] - public void Should_Not_Report_Error_With_Null_Conditional_Operator(bool compileBindings) + public void Should_Report_Error_Without_Null_Conditional_Operator_For_Avalonia_Property(bool compileBindings) + { + // Testing the baseline: should report a null error without null conditionals. + using var app = Start(); + using var log = TestLogger.Create(); + + var xaml = $$$""" + + + + """; + var data = new First { StyledSecond = new Second() }; + var window = CreateTarget(xaml, data); + var textBox = Assert.IsType(window.Content); + var error = Assert.IsType(textBox.Error); + var message = Assert.Single(log.Messages); + + Assert.Null(textBox.Text); + Assert.Equal("StyledSecond.StyledThird.StyledFinal", error.Expression); + Assert.Equal("StyledThird", error.ExpressionErrorPoint); + Assert.Equal(BindingValueType.BindingError, textBox.ErrorState); + Assert.Equal("An error occurred binding {Property} to {Expression} at {ExpressionErrorPoint}: {Message}", message); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_Not_Report_Error_With_Null_Conditional_Operator_For_Clr_Property(bool compileBindings) { using var app = Start(); using var log = TestLogger.Create(); @@ -69,7 +100,7 @@ public class NullConditionalBindingTests """; - var data = new First(new Second(null)); + var data = new First { Second = new Second() }; var window = CreateTarget(xaml, data); var textBox = Assert.IsType(window.Content); @@ -82,7 +113,33 @@ public class NullConditionalBindingTests [Theory] [InlineData(false)] [InlineData(true)] - public void Should_Not_Report_Error_With_Null_Conditional_Operator_Before_Method(bool compileBindings) + public void Should_Not_Report_Error_With_Null_Conditional_Operator_For_Avalonia_Property(bool compileBindings) + { + using var app = Start(); + using var log = TestLogger.Create(); + var xaml = $$$""" + + + + """; + var data = new First { StyledSecond = new Second() }; + var window = CreateTarget(xaml, data); + var textBox = Assert.IsType(window.Content); + + Assert.Null(textBox.Text); + Assert.Null(textBox.Error); + Assert.Equal(BindingValueType.Value, textBox.ErrorState); + Assert.Empty(log.Messages); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_Not_Report_Error_With_Null_Conditional_Operator_Before_Method_For_Clr_Property(bool compileBindings) { using var app = Start(); using var log = TestLogger.Create(); @@ -95,7 +152,33 @@ public class NullConditionalBindingTests """; - var data = new First(new Second(null)); + var data = new First { Second = new Second() }; + var window = CreateTarget(xaml, data); + var textBox = Assert.IsType(window.Content); + + Assert.Null(textBox.Text); + Assert.Null(textBox.Error); + Assert.Equal(BindingValueType.Value, textBox.ErrorState); + Assert.Empty(log.Messages); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_Not_Report_Error_With_Null_Conditional_Operator_Before_Method_For_Avalonia_Property(bool compileBindings) + { + using var app = Start(); + using var log = TestLogger.Create(); + var xaml = $$$""" + + + + """; + var data = new First { StyledSecond = new Second() }; var window = CreateTarget(xaml, data); var textBox = Assert.IsType(window.Content); @@ -108,7 +191,7 @@ public class NullConditionalBindingTests [Theory] [InlineData(false)] [InlineData(true)] - public void Should_Use_TargetNullValue_With_Null_Conditional_Operator(bool compileBindings) + public void Should_Use_TargetNullValue_With_Null_Conditional_Operator_For_Clr_Property(bool compileBindings) { using var app = Start(); using var log = TestLogger.Create(); @@ -121,7 +204,33 @@ public class NullConditionalBindingTests """; - var data = new First(new Second(null)); + var data = new First { Second = new Second() }; + var window = CreateTarget(xaml, data); + var textBox = Assert.IsType(window.Content); + + Assert.Equal("ItsNull", textBox.Text); + Assert.Null(textBox.Error); + Assert.Equal(BindingValueType.Value, textBox.ErrorState); + Assert.Empty(log.Messages); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Should_Use_TargetNullValue_With_Null_Conditional_Operator_For_Avalonia_Property(bool compileBindings) + { + using var app = Start(); + using var log = TestLogger.Create(); + var xaml = $$$""" + + + + """; + var data = new First { StyledSecond = new Second() }; var window = CreateTarget(xaml, data); var textBox = Assert.IsType(window.Content); @@ -144,10 +253,47 @@ public class NullConditionalBindingTests return UnitTestApplication.Start(TestServices.StyledWindow); } - public record First(Second? Second); - public record Second(Third? Third); - public record Third(string Final) + public class First : StyledElement { + public static readonly StyledProperty StyledSecondProperty = + AvaloniaProperty.Register(nameof(StyledSecond)); + + public Second? Second { get; set; } + + public Second? StyledSecond + { + get => GetValue(StyledSecondProperty); + set => SetValue(StyledSecondProperty, value); + } + } + + public class Second : StyledElement + { + public static readonly StyledProperty StyledThirdProperty = + AvaloniaProperty.Register(nameof(StyledThird)); + + public Third? Third { get; set; } + + public Third? StyledThird + { + get => GetValue(StyledThirdProperty); + set => SetValue(StyledThirdProperty, value); + } + } + + public class Third : StyledElement + { + public static readonly StyledProperty StyledFinalProperty = + AvaloniaProperty.Register(nameof(StyledFinal)); + + public string? Final { get; set; } + + public string? StyledFinal + { + get => GetValue(StyledFinalProperty); + set => SetValue(StyledFinalProperty, value); + } + public string Greeting() => "Hello!"; }