Correctly convert BindingNotifications.

In ExpressionSubject.

src/Avalonia.Base/Data/BindingNotification.cs

@@ -92,7 +92,7 @@ namespace Avalonia.Data
         /// <summary>
         /// Gets a value indicating whether <see cref="Value"/> should be pushed to the target.
         /// </summary>
-        public bool HasValue { get; }
+        public bool HasValue { get; set; }
 
         /// <summary>
         /// Gets the error that occurred on the source, if any.

src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs

@@ -183,56 +183,121 @@ namespace Avalonia.Markup.Data
         {
             var notification = value as BindingNotification;
 
-            if (notification?.HasValue == true)
+            if (notification == null)
             {
-                value = notification.Value;
-            }
+                var converted = Converter.Convert(
+                    value,
+                    _targetType,
+                    ConverterParameter,
+                    CultureInfo.CurrentUICulture);
 
-            var converted = Converter.Convert(
-                value,
-                _targetType,
-                ConverterParameter,
-                CultureInfo.CurrentUICulture);
+                notification = converted as BindingNotification;
 
-            if (_fallbackValue != AvaloniaProperty.UnsetValue &&
-                (converted == AvaloniaProperty.UnsetValue ||
-                 converted is BindingNotification))
-            {
-                var error = converted as BindingNotification;
-                
-                if (TypeUtilities.TryConvert(
-                    _targetType, 
-                    _fallbackValue,
-                    CultureInfo.InvariantCulture,
-                    out converted))
+                if (notification?.ErrorType == BindingErrorType.None)
                 {
-                    if (error != null)
-                    {
-                        converted = new BindingNotification(error.Error, BindingErrorType.Error, converted);
-                    }
+                    converted = notification.Value;
                 }
-                else
+
+                if (_fallbackValue != AvaloniaProperty.UnsetValue &&
+                    (converted == AvaloniaProperty.UnsetValue || converted is BindingNotification))
                 {
-                    converted = new BindingNotification(
-                        new InvalidCastException(
-                            $"Could not convert FallbackValue '{_fallbackValue}' to '{_targetType}'"),
-                        BindingErrorType.Error);
+                    var fallback = ConvertFallback();
+                    converted = Merge(converted, fallback);
                 }
+
+                return converted;
             }
+            else
+            {
+                return ConvertValue(notification);
+            }
+        }
 
-            if (notification == null)
+        private BindingNotification ConvertValue(BindingNotification notification)
+        {
+            if (notification.HasValue)
             {
-                return converted;
+                var converted = ConvertValue(notification.Value);
+                notification = Merge(notification, converted);
+            }
+            else if (_fallbackValue != AvaloniaProperty.UnsetValue)
+            {
+                var fallback = ConvertFallback();
+                notification = Merge(notification, fallback);
+            }
+
+            return notification;
+        }
+
+        private BindingNotification ConvertFallback()
+        {
+            object converted;
+
+            if (_fallbackValue == AvaloniaProperty.UnsetValue)
+            {
+                throw new AvaloniaInternalException("Cannot call ConvertFallback with no fallback value");
+            }
+
+            if (TypeUtilities.TryConvert(
+                _targetType,
+                _fallbackValue,
+                CultureInfo.InvariantCulture,
+                out converted))
+            {
+                return new BindingNotification(converted);
             }
             else
+            { 
+                return new BindingNotification(
+                    new InvalidCastException(
+                        $"Could not convert FallbackValue '{_fallbackValue}' to '{_targetType}'"),
+                    BindingErrorType.Error);
+            }
+        }
+
+        private BindingNotification Merge(object a, BindingNotification b)
+        {
+            var an = a as BindingNotification;
+
+            if (an != null)
             {
-                if (notification.HasValue)
-                {
-                    notification.Value = converted;
-                }
+                Merge(an, b);
+                return an;
+            }
+            else
+            {
+                return b;
+            }
+        }
 
-                return notification;
+        private BindingNotification Merge(BindingNotification a, object b)
+        {
+            var bn = b as BindingNotification;
+
+            if (bn != null)
+            {
+                Merge(a, bn);
             }
+            else
+            {
+                a.Value = b;
+                a.HasValue = true;
+            }
+
+            return a;
+        }
+
+        private BindingNotification Merge(BindingNotification a, BindingNotification b)
+        {
+            a.Value = b.Value;
+            a.HasValue = b.HasValue;
+
+            if (b.Error != null)
+            {
+                a.AddError(b.Error, b.ErrorType);
+            }
+
+            return a;
         }
     }
 }

tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_Direct.cs

@@ -284,7 +284,6 @@ namespace Avalonia.Base.UnitTests
             Assert.Equal("newvalue", target.Foo);
         }
 
-
         [Fact]
         public void UnsetValue_Is_Used_On_AddOwnered_Property()
         {

tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs

@@ -108,6 +108,92 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal(6.7, data.DoubleValue);
         }
 
+        [Fact]
+        public async void Should_Return_BindingNotification_With_FallbackValue_For_NonConvertibe_Target_Value()
+        {
+            Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
+
+            var data = new Class1 { StringValue = "foo" };
+            var target = new ExpressionSubject(
+                new ExpressionObserver(data, "StringValue"),
+                typeof(int),
+                42,
+                DefaultValueConverter.Instance);
+            var result = await target.Take(1);
+
+            Assert.Equal(
+                new BindingNotification(
+                    new InvalidCastException("Could not convert 'foo' to 'System.Int32'"),
+                    BindingErrorType.Error,
+                    42),
+                result);
+        }
+
+        [Fact]
+        public async void Should_Return_BindingNotification_With_FallbackValue_For_NonConvertibe_Target_Value_With_Data_Validation()
+        {
+            Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
+
+            var data = new Class1 { StringValue = "foo" };
+            var target = new ExpressionSubject(
+                new ExpressionObserver(data, "StringValue", true),
+                typeof(int),
+                42,
+                DefaultValueConverter.Instance);
+            var result = await target.Take(1);
+
+            Assert.Equal(
+                new BindingNotification(
+                    new InvalidCastException("Could not convert 'foo' to 'System.Int32'"),
+                    BindingErrorType.Error,
+                    42),
+                result);
+        }
+
+        [Fact]
+        public async void Should_Return_BindingNotification_For_Invalid_FallbackValue()
+        {
+            Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
+
+            var data = new Class1 { StringValue = "foo" };
+            var target = new ExpressionSubject(
+                new ExpressionObserver(data, "StringValue"),
+                typeof(int),
+                "bar",
+                DefaultValueConverter.Instance);
+            var result = await target.Take(1);
+
+            Assert.Equal(
+                new BindingNotification(
+                    new AggregateException(
+                        new InvalidCastException("Could not convert 'foo' to 'System.Int32'"),
+                        new InvalidCastException("Could not convert FallbackValue 'bar' to 'System.Int32'")),
+                    BindingErrorType.Error),
+                result);
+        }
+
+        [Fact]
+        public async void Should_Return_BindingNotification_For_Invalid_FallbackValue_With_Data_Validation()
+        {
+            Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
+
+            var data = new Class1 { StringValue = "foo" };
+            var target = new ExpressionSubject(
+                new ExpressionObserver(data, "StringValue", true),
+                typeof(int),
+                "bar",
+                DefaultValueConverter.Instance);
+            var result = await target.Take(1);
+
+            Assert.Equal(
+                new BindingNotification(
+                    new AggregateException(
+                        new InvalidCastException("Could not convert 'foo' to 'System.Int32'"),
+                        new InvalidCastException("Could not convert FallbackValue 'bar' to 'System.Int32'")),
+                    BindingErrorType.Error),
+                result);
+        }
+
         [Fact]
         public void Setting_Invalid_Double_String_Should_Not_Change_Target()
         {