Try to improve logged binding errors.

- Don't log an error when the target for the root ExpressionNode is
null. This is usually because the `DataContext` hasn't been set up yet
and it spewed a load of useless error messages.
- Add a Description field to `ExpressionObserver` that can be used in
the case of e.g. #control bindings to record the whole expression (with
the "#control" part) rather than just the part tracked by the
`ExpressionObserver`.

src/Avalonia.Base/Avalonia.Base.csproj

@@ -43,7 +43,7 @@
     <Compile Include="..\Shared\SharedAssemblyInfo.cs">
       <Link>Properties\SharedAssemblyInfo.cs</Link>
     </Compile>
-    <Compile Include="Data\BindingBrokenException.cs" />
+    <Compile Include="Data\BindingChainNullException.cs" />
     <Compile Include="Data\BindingNotification.cs" />
     <Compile Include="Data\IndexerBinding.cs" />
     <Compile Include="Diagnostics\INotifyCollectionChangedDebug.cs" />

src/Avalonia.Base/Data/BindingBrokenException.cs

@@ -1,15 +0,0 @@
-// Copyright (c) The Avalonia Project. All rights reserved.
-// Licensed under the MIT license. See licence.md file in the project root for full license information.
-
-using System;
-
-namespace Avalonia.Data
-{
-    /// <summary>
-    /// An exception returned through <see cref="BindingNotification"/> signalling that a
-    /// requested binding expression could not be evaluated.
-    /// </summary>
-    public class BindingBrokenException : Exception
-    {
-    }
-}

src/Avalonia.Base/Data/BindingChainNullException.cs

@@ -0,0 +1,85 @@
+// Copyright (c) The Avalonia Project. All rights reserved.
+// Licensed under the MIT license. See licence.md file in the project root for full license information.
+
+using System;
+
+namespace Avalonia.Data
+{
+    /// <summary>
+    /// An exception returned through <see cref="BindingNotification"/> signalling that a
+    /// requested binding expression could not be evaluated because of a null in one of the links
+    /// of the binding chain.
+    /// </summary>
+    public class BindingChainNullException : Exception
+    {
+        private string _message;
+
+        /// <summary>
+        /// Initalizes a new instance of the <see cref="BindingChainNullException"/> class.
+        /// </summary>
+        public BindingChainNullException()
+        {
+        }
+
+        /// <summary>
+        /// Initalizes a new instance of the <see cref="BindingChainNullException"/> class.
+        /// </summary>
+        public BindingChainNullException(string message)
+        {
+            _message = message;
+        }
+
+        /// <summary>
+        /// Initalizes a new instance of the <see cref="BindingChainNullException"/> class.
+        /// </summary>
+        /// <param name="expression">The expression.</param>
+        /// <param name="expressionNullPoint">
+        /// The point in the expression at which the null was encountered.
+        /// </param>
+        public BindingChainNullException(string expression, string expressionNullPoint)
+        {
+            Expression = expression;
+            ExpressionNullPoint = expressionNullPoint;
+        }
+
+        /// <summary>
+        /// Gets the expression that could not be evaluated.
+        /// </summary>
+        public string Expression { get; protected set; }
+
+        /// <summary>
+        /// Gets the point in the expression at which the null was encountered.
+        /// </summary>
+        public string ExpressionNullPoint { get; protected set; }
+
+        /// <inheritdoc/>
+        public override string Message
+        {
+            get
+            {
+                if (_message == null)
+                {
+                    _message = BuildMessage();
+                }
+
+                return _message;
+            }
+        }
+
+        private string BuildMessage()
+        {
+            if (Expression != null && ExpressionNullPoint != null)
+            {
+                return $"'{ExpressionNullPoint}' is null in expression '{Expression}'.";
+            }
+            else if (ExpressionNullPoint != null)
+            {
+                return $"'{ExpressionNullPoint}' is null in expression.";
+            }
+            else
+            {
+                return "Null encountered in binding expression.";
+            }
+        }
+    }
+}

src/Avalonia.Base/Data/BindingNotification.cs

@@ -267,7 +267,7 @@ namespace Avalonia.Data
                 case BindingErrorType.None:
                     return $"{{Value: {Value}}}";
                 default:
-                    return HasValue ? 
+                    return HasValue ?
                         $"{{{ErrorType}: {Error}, Fallback: {Value}}}" : 
                         $"{{{ErrorType}: {Error}}}";
             }

src/Avalonia.Controls/TextBox.cs

@@ -499,7 +499,7 @@ namespace Avalonia.Controls
                 var exceptions = aggregate == null ?
                     (IEnumerable<Exception>)new[] { exception } :
                     aggregate.InnerExceptions;
-                var filtered = exceptions.Where(x => !(x is BindingBrokenException)).ToList();
+                var filtered = exceptions.Where(x => !(x is BindingChainNullException)).ToList();
 
                 if (filtered.Count > 0)
                 {

src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs

@@ -238,10 +238,12 @@ namespace Avalonia.Markup.Xaml.Data
         {
             Contract.Requires<ArgumentNullException>(target != null);
 
+            var description = $"#{elementName}.{path}";
             var result = new ExpressionObserver(
                 ControlLocator.Track(target, elementName),
                 path,
-                false);
+                false,
+                description);
             return result;
         }
 

src/Markup/Avalonia.Markup/Avalonia.Markup.csproj

@@ -41,7 +41,7 @@
     <Compile Include="..\..\Shared\SharedAssemblyInfo.cs">
       <Link>Properties\SharedAssemblyInfo.cs</Link>
     </Compile>
-    <Compile Include="Data\MarkupBindingBrokenException.cs" />
+    <Compile Include="Data\MarkupBindingChainNullException.cs" />
     <Compile Include="Data\CommonPropertyNames.cs" />
     <Compile Include="Data\EmptyExpressionNode.cs" />
     <Compile Include="Data\ExpressionNodeBuilder.cs" />

src/Markup/Avalonia.Markup/Data/ExpressionNode.cs

@@ -92,8 +92,8 @@ namespace Avalonia.Markup.Data
 
         protected virtual void NextValueChanged(object value)
         {
-            var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingBrokenException;
+            var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainNullException;
-            bindingBroken?.Nodes.Add(Description);
+            bindingBroken?.AddNode(Description);
             _observer.OnNext(value);
         }
 
@@ -181,7 +181,7 @@ namespace Avalonia.Markup.Data
         private BindingNotification TargetNullNotification()
         {
             return new BindingNotification(
-                new MarkupBindingBrokenException(this),
+                new MarkupBindingChainNullException(),
                 BindingErrorType.Error,
                 AvaloniaProperty.UnsetValue);
         }

src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs

@@ -63,7 +63,14 @@ namespace Avalonia.Markup.Data
         /// <param name="root">The root object.</param>
         /// <param name="expression">The expression.</param>
         /// <param name="enableDataValidation">Whether data validation should be enabled.</param>
-        public ExpressionObserver(object root, string expression, bool enableDataValidation = false)
+        /// <param name="description">
+        /// A description of the expression. If null, <paramref name="expression"/> will be used.
+        /// </param>
+        public ExpressionObserver(
+            object root,
+            string expression,
+            bool enableDataValidation = false,
+            string description = null)
         {
             Contract.Requires<ArgumentNullException>(expression != null);
 
@@ -73,6 +80,7 @@ namespace Avalonia.Markup.Data
             }
 
             Expression = expression;
+            Description = description ?? expression;
             _node = Parse(expression, enableDataValidation);
             _root = new WeakReference(root);
         }
@@ -83,15 +91,20 @@ namespace Avalonia.Markup.Data
         /// <param name="rootObservable">An observable which provides the root object.</param>
         /// <param name="expression">The expression.</param>
         /// <param name="enableDataValidation">Whether data validation should be enabled.</param>
+        /// <param name="description">
+        /// A description of the expression. If null, <paramref name="expression"/> will be used.
+        /// </param>
         public ExpressionObserver(
             IObservable<object> rootObservable,
             string expression,
-            bool enableDataValidation = false)
+            bool enableDataValidation = false,
+            string description = null)
         {
             Contract.Requires<ArgumentNullException>(rootObservable != null);
             Contract.Requires<ArgumentNullException>(expression != null);
 
             Expression = expression;
+            Description = description ?? expression;
             _node = Parse(expression, enableDataValidation);
             _finished = new Subject<Unit>();
             _root = rootObservable;
@@ -104,17 +117,22 @@ namespace Avalonia.Markup.Data
         /// <param name="expression">The expression.</param>
         /// <param name="update">An observable which triggers a re-read of the getter.</param>
         /// <param name="enableDataValidation">Whether data validation should be enabled.</param>
+        /// <param name="description">
+        /// A description of the expression. If null, <paramref name="expression"/> will be used.
+        /// </param>
         public ExpressionObserver(
             Func<object> rootGetter,
             string expression,
             IObservable<Unit> update,
-            bool enableDataValidation = false)
+            bool enableDataValidation = false,
+            string description = null)
         {
             Contract.Requires<ArgumentNullException>(rootGetter != null);
             Contract.Requires<ArgumentNullException>(expression != null);
             Contract.Requires<ArgumentNullException>(update != null);
 
             Expression = expression;
+            Description = description ?? expression;
             _node = Parse(expression, enableDataValidation);
             _finished = new Subject<Unit>();
 
@@ -138,6 +156,11 @@ namespace Avalonia.Markup.Data
             return (Leaf as PropertyAccessorNode)?.SetTargetValue(value, priority) ?? false;
         }
 
+        /// <summary>
+        /// Gets a description of the expression being observed.
+        /// </summary>
+        public string Description { get; }
+
         /// <summary>
         /// Gets the expression being observed.
         /// </summary>
@@ -149,9 +172,6 @@ namespace Avalonia.Markup.Data
         /// </summary>
         public Type ResultType => (Leaf as PropertyAccessorNode)?.PropertyType;
 
-        /// <inheritdoc/>
-        string IDescription.Description => Expression;
-
         /// <summary>
         /// Gets the leaf node.
         /// </summary>
@@ -215,15 +235,23 @@ namespace Avalonia.Markup.Data
             }
             else
             {
-                var notification = o as BindingNotification;
+                var broken = BindingNotification.ExtractError(o) as MarkupBindingChainNullException;
-                var broken = notification.Error as MarkupBindingBrokenException;
 
                 if (broken != null)
                 {
-                    broken.Expression = Expression;
+                    // We've received notification of a broken expression due to a null value
+                    // somewhere in the chain. If this null value occurs at the first node then we
+                    // ignore it, as its likely that e.g. the DataContext has not yet been set up.
+                    if (broken.HasNodes)
+                    {
+                        broken.Commit(Description);
+                    }
+                    else
+                    {
+                        o = AvaloniaProperty.UnsetValue;
+                    }
                 }
-
+                return o;
-                return notification;
             }
         }
 

src/Markup/Avalonia.Markup/Data/MarkupBindingBrokenException.cs

@@ -1,65 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using Avalonia.Data;
-
-namespace Avalonia.Markup.Data
-{
-    public class MarkupBindingBrokenException : BindingBrokenException
-    {
-        private string _message;
-
-        public MarkupBindingBrokenException()
-        {
-        }
-
-        public MarkupBindingBrokenException(string message)
-        {
-            _message = message;
-        }
-
-        internal MarkupBindingBrokenException(ExpressionNode node)
-        {
-            Nodes.Add(node.Description);
-        }
-
-        public override string Message
-        {
-            get
-            {
-                if (_message != null)
-                {
-                    return _message;
-                }
-                else
-                {
-                    return _message = BuildMessage();
-                }
-            }
-        }
-
-        internal string Expression { get; set; }
-        internal IList<string> Nodes { get; } = new List<string>();
-
-        private string BuildMessage()
-        {
-            if (Nodes.Count == 0)
-            {
-                return "The binding chain was broken.";
-            }
-            else if (Nodes.Count == 1)
-            {
-                return $"'{Nodes[0]}' is null in expression '{Expression}'.";
-            }
-            else
-            {
-                var brokenPath = string.Join(".", Nodes.Skip(1).Reverse())
-                    .Replace(".!", "!")
-                    .Replace(".[", "[");
-                return $"'{brokenPath}' is null in expression '{Expression}'.";
-            }
-        }
-    }
-}

src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs

@@ -0,0 +1,33 @@
+using System.Collections.Generic;
+using System.Linq;
+using Avalonia.Data;
+
+namespace Avalonia.Markup.Data
+{
+    internal class MarkupBindingChainNullException : BindingChainNullException
+    {
+        private IList<string> _nodes = new List<string>();
+
+        public MarkupBindingChainNullException()
+        {
+        }
+
+        public MarkupBindingChainNullException(string expression, string expressionNullPoint)
+            : base(expression, expressionNullPoint)
+        {
+            _nodes = null;
+        }
+
+        public bool HasNodes => _nodes.Count > 0;
+        public void AddNode(string node) => _nodes.Add(node);
+
+        public void Commit(string expression)
+        {
+            Expression = expression;
+            ExpressionNullPoint = string.Join(".", _nodes.Reverse())
+                .Replace(".!", "!")
+                .Replace(".[", "[");
+            _nodes = null;
+        }
+    }
+}

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

@@ -143,7 +143,7 @@ namespace Avalonia.Markup.UnitTests.Data
             Assert.Equal(new[]
             {
                 new BindingNotification(
-                    new MarkupBindingBrokenException("'Inner' is null in expression 'Inner.MustBePositive'."),
+                    new MarkupBindingChainNullException("Inner.MustBePositive", "Inner"),
                     BindingErrorType.Error,
                     AvaloniaProperty.UnsetValue),
             }, result);

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

@@ -58,63 +58,43 @@ namespace Avalonia.Markup.UnitTests.Data
         }
 
         [Fact]
-        public async void Should_Return_BindingNotification_Error_For_Root_Null()
+        public async void Should_Return_UnsetValue_For_Root_Null()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(default(object), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(
+            Assert.Equal(AvaloniaProperty.UnsetValue, result);
-                new BindingNotification(
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                    BindingErrorType.Error,
-                    AvaloniaProperty.UnsetValue),
-                result);
         }
 
         [Fact]
-        public async void Should_Return_BindingNotification_Error_For_Root_UnsetValue()
+        public async void Should_Return_UnsetValue_For_Root_UnsetValue()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(AvaloniaProperty.UnsetValue, "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(
+            Assert.Equal(AvaloniaProperty.UnsetValue, result);
-                new BindingNotification(
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                    BindingErrorType.Error,
-                    AvaloniaProperty.UnsetValue),
-                result);
         }
 
         [Fact]
-        public async void Should_Return_BindingNotification_Error_For_Observable_Root_Null()
+        public async void Should_Return_UnsetValue_For_Observable_Root_Null()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(Observable.Return(default(object)), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(
+            Assert.Equal(AvaloniaProperty.UnsetValue, result);
-                new BindingNotification(
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                    BindingErrorType.Error,
-                    AvaloniaProperty.UnsetValue),
-                result);
         }
 
         [Fact]
-        public async void Should_Return_BindingNotification_Error_For_Observable_Root_UnsetValue()
+        public async void Should_Return_UnsetValue_For_Observable_Root_UnsetValue()
         {
             var data = new Class3 { Foo = "foo" };
             var target = new ExpressionObserver(Observable.Return(AvaloniaProperty.UnsetValue), "Foo");
             var result = await target.Take(1);
 
-            Assert.Equal(
+            Assert.Equal(AvaloniaProperty.UnsetValue, result);
-                new BindingNotification(
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                    BindingErrorType.Error,
-                    AvaloniaProperty.UnsetValue),
-                result);
         }
 
         [Fact]
@@ -166,7 +146,7 @@ namespace Avalonia.Markup.UnitTests.Data
                 new[]
                 {
                     new BindingNotification(
-                        new MarkupBindingBrokenException("'Foo' is null in expression 'Foo.Bar.Baz'."),
+                    new MarkupBindingChainNullException("Foo.Bar.Baz", "Foo"),
                         BindingErrorType.Error,
                         AvaloniaProperty.UnsetValue),
                 },
@@ -270,24 +250,34 @@ namespace Avalonia.Markup.UnitTests.Data
         [Fact]
         public void Should_Track_Property_Chain_Breaking_With_Null_Then_Mending()
         {
-            var data = new Class1 { Next = new Class2 { Bar = "bar" } };
+            var data = new Class1
-            var target = new ExpressionObserver(data, "Next.Bar");
+            {
+                Next = new Class2
+                {
+                    Next = new Class2
+                    {
+                        Bar = "bar"
+                    }
+                }
+            };
+
+            var target = new ExpressionObserver(data, "Next.Next.Bar");
             var result = new List<object>();
 
             var sub = target.Subscribe(x => result.Add(x));
             var old = data.Next;
-            data.Next = null;
             data.Next = new Class2 { Bar = "baz" };
+            data.Next = old;
 
             Assert.Equal(
                 new object[] 
                 {
                     "bar",
                     new BindingNotification(
-                        new MarkupBindingBrokenException("'Next' is null in expression 'Next.Bar'."),
+                        new MarkupBindingChainNullException("Next.Next.Bar", "Next.Next"),
                         BindingErrorType.Error,
                         AvaloniaProperty.UnsetValue),
-                    "baz"
+                    "bar"
                 }, 
                 result);
 
@@ -299,7 +289,7 @@ namespace Avalonia.Markup.UnitTests.Data
         }
 
         [Fact]
-        public void Should_Track_Property_Chain_Breaking_With_Object_Then_Mending()
+        public void Should_Track_Property_Chain_Breaking_With_Missing_Member_Then_Mending()
         {
             var data = new Class1 { Next = new Class2 { Bar = "bar" } };
             var target = new ExpressionObserver(data, "Next.Bar");
@@ -311,10 +301,16 @@ namespace Avalonia.Markup.UnitTests.Data
             data.Next = breaking;
             data.Next = new Class2 { Bar = "baz" };
 
-            Assert.Equal(3, result.Count);
+            Assert.Equal(
-            Assert.Equal("bar", result[0]);
+                new object[]
-            Assert.IsType<BindingNotification>(result[1]);
+                {
-            Assert.Equal("baz", result[2]);
+                    "bar",
+                    new BindingNotification(
+                        new MissingMemberException("Could not find CLR property 'Bar' on 'Avalonia.Markup.UnitTests.Data.ExpressionObserverTests_Property+WithoutBar'"),
+                        BindingErrorType.Error),
+                    "baz",
+                },
+                result);
 
             sub.Dispose();
 
@@ -475,20 +471,6 @@ namespace Avalonia.Markup.UnitTests.Data
             }
         }
 
-        [Fact]
-        public async void Should_Handle_Null_Root()
-        {
-            var target = new ExpressionObserver((object)null, "Foo");
-            var result = await target.Take(1);
-
-            Assert.Equal(
-                new BindingNotification(
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                    BindingErrorType.Error,
-                    AvaloniaProperty.UnsetValue),
-                result);
-        }
-
         [Fact]
         public void Can_Replace_Root()
         {
@@ -510,10 +492,7 @@ namespace Avalonia.Markup.UnitTests.Data
                 {
                     "foo",
                     "bar",
-                    new BindingNotification(
+                    AvaloniaProperty.UnsetValue,
-                    new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."),
-                        BindingErrorType.Error,
-                        AvaloniaProperty.UnsetValue),
                 }, 
                 result);
 
@@ -580,6 +559,7 @@ namespace Avalonia.Markup.UnitTests.Data
         private class Class2 : NotifyingBase, INext
         {
             private string _bar;
+            private INext _next;
 
             public string Bar
             {
@@ -590,6 +570,16 @@ namespace Avalonia.Markup.UnitTests.Data
                     RaisePropertyChanged(nameof(Bar));
                 }
             }
+
+            public INext Next
+            {
+                get { return _next; }
+                set
+                {
+                    _next = value;
+                    RaisePropertyChanged(nameof(Next));
+                }
+            }
         }
 
         private class Class3 : Class1

tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ControlBindingTests.cs

@@ -43,8 +43,7 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml
                     pv.Length == 3 &&
                     pv[0] is ProgressBar &&
                     object.ReferenceEquals(pv[1], ProgressBar.ValueProperty) &&
-                    (string)pv[2] == "'Value' is null in expression 'Value'. | " + 
+                    (string)pv[2] == "Could not convert FallbackValue 'bar' to 'System.Double'")
-                                     "Could not convert FallbackValue 'bar' to 'System.Double'")
                 {
                     called = true;
                 }