Handle DataValidationError notifications returned by ConvertBack (#22171)

* Handle DataValidationError notifications returned by ConvertBack

A converter returning a BindingNotification with DataValidationError fell
through to the target type converter, which replaced it with a cast error
that stringified the notification. Publish the original error instead.

* Add additional failing tests.

There are more `BindingNotification` cases to handle.

* Handle the other `BindingNotification` cases.

---------

Co-authored-by: grokys <grokys@users.noreply.github.com>

src/Avalonia.Base/Data/Core/BindingExpression.cs

@@ -3,11 +3,9 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
-using System.Linq.Expressions;
 using System.Text;
 using Avalonia.Data.Converters;
 using Avalonia.Data.Core.ExpressionNodes;
-using Avalonia.Data.Core.Parsers;
 using Avalonia.Input;
 using Avalonia.Interactivity;
 using Avalonia.Logging;
@@ -316,13 +314,12 @@ internal class BindingExpression : UntypedBindingExpressionBase, IDescription, I
         if (_nodes.Count == 0 || LeafNode is not ISettableNode setter || setter.ValueType is not { } type)
             return false;
 
-        if (Converter is { } converter &&
-            value != AvaloniaProperty.UnsetValue &&
-            value != BindingOperations.DoNothing)
-        {
-            value = ConvertBack(converter, ConverterCulture, ConverterParameter, value, type);
-        }
+        // Invoke any converter on the value before writing it to the source. If the converter
+        // returns an error then we don't write the value to the source and return false.
+        if (!TryConvertBack(type, ref value))
+            return false;
 
+        // A converter may return DoNothing.
         if (value == BindingOperations.DoNothing)
             return true;
 
@@ -586,6 +583,41 @@ internal class BindingExpression : UntypedBindingExpressionBase, IDescription, I
         return AvaloniaProperty.UnsetValue;
     }
 
+    private bool TryConvertBack(Type valueType, ref object? value)
+    {
+        if (Converter is { } converter &&
+            value != AvaloniaProperty.UnsetValue &&
+            value != BindingOperations.DoNothing)
+        {
+            value = ConvertBack(converter, ConverterCulture, ConverterParameter, value, valueType);
+
+            if (value is BindingNotification notification)
+            {
+                if (notification.Error is { } error)
+                {
+                    switch (notification.ErrorType)
+                    {
+                        case BindingErrorType.DataValidationError:
+                            if (IsDataValidationEnabled)
+                                OnDataValidationError(notification.Error);
+                            break;
+                        default:
+                            if (ShouldLogError(out var target))
+                                Log(target, error.Message);
+                            PublishValue(UnchangedValue, new(error, BindingErrorType.Error));
+                            break;
+                    }
+
+                    return false;
+                }
+
+                value = notification.Value;
+            }
+        }
+
+        return true;
+    }
+
     /// <summary>
     /// Uncommonly used fields are separated out to reduce memory usage.
     /// </summary>

tests/Avalonia.Base.UnitTests/Data/Core/BindingExpressionTests.DataValidation.cs

@@ -2,8 +2,10 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
+using System.Globalization;
 using System.Linq;
 using Avalonia.Data;
+using Avalonia.Data.Converters;
 using Avalonia.Data.Core.Plugins;
 using Avalonia.UnitTests;
 using Xunit;
@@ -576,6 +578,115 @@ public partial class BindingExpressionTests
         }
     }   
 
+    private class InvalidIdConverter(BindingErrorType errorType) : IValueConverter
+    {
+        private readonly BindingErrorType _errorType = errorType;
+
+        public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture) => value;
+
+        public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture)
+        {
+            if (value is int i)
+                return i;
+
+            if (value is string s && int.TryParse(s, out var parsed))
+                return parsed;
+
+            return new BindingNotification(
+                new FormatException($"'{value}' is not a valid ID."),
+                _errorType);
+        }
+    }
+
+    [Fact]
+    public void ConvertBack_DataValidationError_Updates_Data_Validation()
+    {
+        var data = new ViewModel { IntValue = 1 };
+        var target = CreateTargetWithSource(
+            data,
+            o => o.IntValue,
+            converter: new InvalidIdConverter(BindingErrorType.DataValidationError),
+            enableDataValidation: true,
+            mode: BindingMode.TwoWay,
+            targetProperty: TargetClass.TagProperty);
+
+        target.Tag = "42";
+
+        AssertNoError(target, TargetClass.TagProperty);
+
+        target.Tag = "0x555g";
+
+        Assert.Equal(42, data.IntValue);
+        AssertBindingError(
+            target,
+            TargetClass.TagProperty,
+            new FormatException("'0x555g' is not a valid ID."),
+            BindingErrorType.DataValidationError);
+
+        GC.KeepAlive(data);
+    }
+
+    [Fact]
+    public void ConvertBack_Error_Updates_Data_Validation()
+    {
+        var data = new ViewModel { IntValue = 1 };
+        var target = CreateTargetWithSource(
+            data,
+            o => o.IntValue,
+            converter: new InvalidIdConverter(BindingErrorType.Error),
+            enableDataValidation: true,
+            mode: BindingMode.TwoWay,
+            targetProperty: TargetClass.TagProperty);
+
+        target.Tag = "42";
+
+        AssertNoError(target, TargetClass.TagProperty);
+
+        target.Tag = "0x555g";
+
+        Assert.Equal(42, data.IntValue);
+        AssertBindingError(
+            target,
+            TargetClass.TagProperty,
+            new FormatException("'0x555g' is not a valid ID."),
+            BindingErrorType.Error);
+
+        GC.KeepAlive(data);
+    }
+
+    [Fact]
+    public void ConvertBack_Notification_With_Value_Writes_Value()
+    {
+        var data = new ViewModel { IntValue = 1 };
+        var target = CreateTargetWithSource(
+            data,
+            o => o.IntValue,
+            converter: new FuncValueConverter(v => new BindingNotification(42)),
+            enableDataValidation: true,
+            mode: BindingMode.TwoWay,
+            targetProperty: TargetClass.TagProperty);
+
+        target.Tag = "foo";
+
+        Assert.Equal(42, data.IntValue);
+        AssertNoError(target, TargetClass.TagProperty);
+
+        GC.KeepAlive(data);
+    }
+
+    private class FuncValueConverter : IValueConverter
+    {
+        private readonly Func<object?, object?> _convertBack;
+
+        public FuncValueConverter(Func<object?, object?> convertBack) => _convertBack = convertBack;
+
+        public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture)
+            => value;
+
+        public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture)
+            => _convertBack(value);
+    }
+
     private static void AssertNoError(TargetClass target, AvaloniaProperty property)
     {
         Assert.False(target.BindingNotifications.TryGetValue(property, out var notification));