From 00a619fe223566cb3c811f6671aa731119032fce Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 20 Oct 2023 23:27:30 +0200 Subject: [PATCH] Log errors for failed conversions. --- .../Data/Core/BindingExpression.cs | 14 ++- .../Data/BindingTests_Logging.cs | 116 ++++++++++-------- 2 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/BindingExpression.cs b/src/Avalonia.Base/Data/Core/BindingExpression.cs index 2dfa2a3314..1c225e0b1d 100644 --- a/src/Avalonia.Base/Data/Core/BindingExpression.cs +++ b/src/Avalonia.Base/Data/Core/BindingExpression.cs @@ -498,7 +498,7 @@ internal class BindingExpression : IObservable, else if (_targetTypeConverter is not null && value is not null) { // Otherwise, if we have a target type converter, convert the value to the target type. - value = UpdateAndUnwrap(ConvertFrom(_targetTypeConverter, value, false), ref notification); + value = UpdateAndUnwrap(ConvertFrom(_targetTypeConverter, value), ref notification); } } @@ -574,7 +574,7 @@ internal class BindingExpression : IObservable, return AvaloniaProperty.UnsetValue; } - private object? ConvertFrom(TargetTypeConverter? converter, object value, bool isFallback) + private object? ConvertFrom(TargetTypeConverter? converter, object value) { if (converter is null || _targetProperty is null) return value; @@ -586,10 +586,12 @@ internal class BindingExpression : IObservable, var valueString = value?.ToString() ?? "(null)"; var valueTypeName = value?.GetType().FullName ?? "null"; - var fallbackMessage = isFallback ? " fallback value " : " "; - var ex = new InvalidCastException( - $"Cannot convert{fallbackMessage}'{valueString}' ({valueTypeName}) to '{targetType}'."); - return new BindingNotification(ex, BindingErrorType.Error); + var message = $"Cannot convert '{valueString}' ({valueTypeName}) to '{targetType}'."; + + if (ShouldLogError(out var target)) + Log(target, message, LogEventLevel.Warning); + + return new BindingNotification(new InvalidCastException(message), BindingErrorType.Error); } private static object? UpdateAndUnwrap(object? value, ref BindingNotification? notification) diff --git a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_Logging.cs b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_Logging.cs index 4d215ccc3b..d2bee8b15f 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/BindingTests_Logging.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/BindingTests_Logging.cs @@ -121,52 +121,6 @@ namespace Avalonia.Markup.UnitTests.Data } } - public class NamedElement - { - [Fact] - public void Should_Log_NameScope_Not_Found() - { - var target = new Decorator { }; - var root = new TestRoot(target); - var binding = new Binding("#source") { TypeResolver = ResolveType }; - - using (AssertLog(target, binding.Path, "NameScope not found.", "#source")) - { - target.Bind(Control.TagProperty, binding); - } - } - - [Fact] - public void Should_Not_Log_Element_Property_Null_For_Unrooted_Control() - { - var ns = new NameScope(); - var source = new Canvas { Name = "source" }; - var target = new Decorator { }; - var binding = new Binding("#source.DataContext.Foo") { TypeResolver = ResolveType, NameScope = new(ns) }; - var container = new StackPanel - { - [NameScope.NameScopeProperty] = ns, - Children = { source, target } - }; - - ns.Register(source.Name, source); - - using (AssertNoLog()) - { - target.Bind(Control.TagProperty, binding); - } - - // Sanity check to that the binding works when rooted: make sure that we're not just testing a broken - // binding! - using (AssertNoLog()) - { - var root = new TestRoot(container); - root.DataContext = new { Foo = "foo" }; - Assert.Equal("foo", target.Tag); - } - } - } - public class VisualAncestor { [Fact] @@ -174,8 +128,8 @@ namespace Avalonia.Markup.UnitTests.Data { var target = new Decorator { }; var root = new TestRoot(target); - var binding = new Binding - { + var binding = new Binding + { RelativeSource = new RelativeSource(RelativeSourceMode.FindAncestor) { AncestorType = typeof(TextBlock), @@ -230,6 +184,72 @@ namespace Avalonia.Markup.UnitTests.Data } } + public class NamedElement + { + [Fact] + public void Should_Log_NameScope_Not_Found() + { + var target = new Decorator { }; + var root = new TestRoot(target); + var binding = new Binding("#source") { TypeResolver = ResolveType }; + + using (AssertLog(target, binding.Path, "NameScope not found.", "#source")) + { + target.Bind(Control.TagProperty, binding); + } + } + + [Fact] + public void Should_Not_Log_Element_Property_Null_For_Unrooted_Control() + { + var ns = new NameScope(); + var source = new Canvas { Name = "source" }; + var target = new Decorator { }; + var binding = new Binding("#source.DataContext.Foo") { TypeResolver = ResolveType, NameScope = new(ns) }; + var container = new StackPanel + { + [NameScope.NameScopeProperty] = ns, + Children = { source, target } + }; + + ns.Register(source.Name, source); + + using (AssertNoLog()) + { + target.Bind(Control.TagProperty, binding); + } + + // Sanity check to that the binding works when rooted: make sure that we're not just testing a broken + // binding! + using (AssertNoLog()) + { + var root = new TestRoot(container); + root.DataContext = new { Foo = "foo" }; + Assert.Equal("foo", target.Tag); + } + } + } + + public class Converter + { + [Fact] + public void Should_Log_Error_For_Unconvertible_Type() + { + var target = new Decorator { DataContext = new { Foo = new System.Version() } }; + var root = new TestRoot(target); + var binding = new Binding("Foo"); + + using (AssertLog( + target, + binding.Path, + "Cannot convert '0.0' (System.Version) to 'Avalonia.Thickness'.", + property: Control.MarginProperty)) + { + target.Bind(Control.MarginProperty, binding); + } + } + } + public class Fallback { [Theory]