diff --git a/src/Avalonia.Base/Avalonia.Base.csproj b/src/Avalonia.Base/Avalonia.Base.csproj index 97dfd1230d..6674e45fd1 100644 --- a/src/Avalonia.Base/Avalonia.Base.csproj +++ b/src/Avalonia.Base/Avalonia.Base.csproj @@ -43,7 +43,7 @@ Properties\SharedAssemblyInfo.cs - + diff --git a/src/Avalonia.Base/Data/BindingBrokenException.cs b/src/Avalonia.Base/Data/BindingBrokenException.cs deleted file mode 100644 index 057629edb8..0000000000 --- a/src/Avalonia.Base/Data/BindingBrokenException.cs +++ /dev/null @@ -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 -{ - /// - /// An exception returned through signalling that a - /// requested binding expression could not be evaluated. - /// - public class BindingBrokenException : Exception - { - } -} diff --git a/src/Avalonia.Base/Data/BindingChainNullException.cs b/src/Avalonia.Base/Data/BindingChainNullException.cs new file mode 100644 index 0000000000..0e50a36d8a --- /dev/null +++ b/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 +{ + /// + /// An exception returned through signalling that a + /// requested binding expression could not be evaluated because of a null in one of the links + /// of the binding chain. + /// + public class BindingChainNullException : Exception + { + private string _message; + + /// + /// Initalizes a new instance of the class. + /// + public BindingChainNullException() + { + } + + /// + /// Initalizes a new instance of the class. + /// + public BindingChainNullException(string message) + { + _message = message; + } + + /// + /// Initalizes a new instance of the class. + /// + /// The expression. + /// + /// The point in the expression at which the null was encountered. + /// + public BindingChainNullException(string expression, string expressionNullPoint) + { + Expression = expression; + ExpressionNullPoint = expressionNullPoint; + } + + /// + /// Gets the expression that could not be evaluated. + /// + public string Expression { get; protected set; } + + /// + /// Gets the point in the expression at which the null was encountered. + /// + public string ExpressionNullPoint { get; protected set; } + + /// + 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."; + } + } + } +} diff --git a/src/Avalonia.Base/Data/BindingNotification.cs b/src/Avalonia.Base/Data/BindingNotification.cs index 0f587b969e..ecaf59e174 100644 --- a/src/Avalonia.Base/Data/BindingNotification.cs +++ b/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}}}"; } diff --git a/src/Avalonia.Controls/TextBox.cs b/src/Avalonia.Controls/TextBox.cs index dbea94f9a4..2c43f8f97e 100644 --- a/src/Avalonia.Controls/TextBox.cs +++ b/src/Avalonia.Controls/TextBox.cs @@ -499,7 +499,7 @@ namespace Avalonia.Controls var exceptions = aggregate == null ? (IEnumerable)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) { diff --git a/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs b/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs index 6b8b1282e5..832a25be27 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs @@ -238,10 +238,12 @@ namespace Avalonia.Markup.Xaml.Data { Contract.Requires(target != null); + var description = $"#{elementName}.{path}"; var result = new ExpressionObserver( ControlLocator.Track(target, elementName), path, - false); + false, + description); return result; } diff --git a/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj b/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj index 334add960e..9edfd2957d 100644 --- a/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj +++ b/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj @@ -41,7 +41,7 @@ Properties\SharedAssemblyInfo.cs - + diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs b/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs index d6f2d66adf..0e7777b732 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs +++ b/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; - bindingBroken?.Nodes.Add(Description); + var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainNullException; + 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); } diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs index f3f175e04c..819949b7b9 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs @@ -63,7 +63,14 @@ namespace Avalonia.Markup.Data /// The root object. /// The expression. /// Whether data validation should be enabled. - public ExpressionObserver(object root, string expression, bool enableDataValidation = false) + /// + /// A description of the expression. If null, will be used. + /// + public ExpressionObserver( + object root, + string expression, + bool enableDataValidation = false, + string description = null) { Contract.Requires(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 /// An observable which provides the root object. /// The expression. /// Whether data validation should be enabled. + /// + /// A description of the expression. If null, will be used. + /// public ExpressionObserver( IObservable rootObservable, string expression, - bool enableDataValidation = false) + bool enableDataValidation = false, + string description = null) { Contract.Requires(rootObservable != null); Contract.Requires(expression != null); Expression = expression; + Description = description ?? expression; _node = Parse(expression, enableDataValidation); _finished = new Subject(); _root = rootObservable; @@ -104,17 +117,22 @@ namespace Avalonia.Markup.Data /// The expression. /// An observable which triggers a re-read of the getter. /// Whether data validation should be enabled. + /// + /// A description of the expression. If null, will be used. + /// public ExpressionObserver( Func rootGetter, string expression, IObservable update, - bool enableDataValidation = false) + bool enableDataValidation = false, + string description = null) { Contract.Requires(rootGetter != null); Contract.Requires(expression != null); Contract.Requires(update != null); Expression = expression; + Description = description ?? expression; _node = Parse(expression, enableDataValidation); _finished = new Subject(); @@ -138,6 +156,11 @@ namespace Avalonia.Markup.Data return (Leaf as PropertyAccessorNode)?.SetTargetValue(value, priority) ?? false; } + /// + /// Gets a description of the expression being observed. + /// + public string Description { get; } + /// /// Gets the expression being observed. /// @@ -149,9 +172,6 @@ namespace Avalonia.Markup.Data /// public Type ResultType => (Leaf as PropertyAccessorNode)?.PropertyType; - /// - string IDescription.Description => Expression; - /// /// Gets the leaf node. /// @@ -215,15 +235,23 @@ namespace Avalonia.Markup.Data } else { - var notification = o as BindingNotification; - var broken = notification.Error as MarkupBindingBrokenException; + var broken = BindingNotification.ExtractError(o) as MarkupBindingChainNullException; 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 notification; + return o; } } diff --git a/src/Markup/Avalonia.Markup/Data/MarkupBindingBrokenException.cs b/src/Markup/Avalonia.Markup/Data/MarkupBindingBrokenException.cs deleted file mode 100644 index 33aef139a0..0000000000 --- a/src/Markup/Avalonia.Markup/Data/MarkupBindingBrokenException.cs +++ /dev/null @@ -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 Nodes { get; } = new List(); - - 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}'."; - } - } - } -} diff --git a/src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs b/src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs new file mode 100644 index 0000000000..a549d6ebb6 --- /dev/null +++ b/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 _nodes = new List(); + + 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; + } + } +} diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs index 1815d82c12..fb98144647 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs +++ b/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); diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs index bd7f28f620..aa9ee7d58b 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs +++ b/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( - new BindingNotification( - new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."), - BindingErrorType.Error, - AvaloniaProperty.UnsetValue), - result); + Assert.Equal(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( - new BindingNotification( - new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."), - BindingErrorType.Error, - AvaloniaProperty.UnsetValue), - result); + Assert.Equal(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( - new BindingNotification( - new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."), - BindingErrorType.Error, - AvaloniaProperty.UnsetValue), - result); + Assert.Equal(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( - new BindingNotification( - new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."), - BindingErrorType.Error, - AvaloniaProperty.UnsetValue), - result); + Assert.Equal(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 target = new ExpressionObserver(data, "Next.Bar"); + var data = new Class1 + { + Next = new Class2 + { + Next = new Class2 + { + Bar = "bar" + } + } + }; + + var target = new ExpressionObserver(data, "Next.Next.Bar"); var result = new List(); 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("bar", result[0]); - Assert.IsType(result[1]); - Assert.Equal("baz", result[2]); + Assert.Equal( + new object[] + { + "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( - new MarkupBindingBrokenException("'Foo' is null in expression 'Foo'."), - BindingErrorType.Error, - AvaloniaProperty.UnsetValue), + 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 diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ControlBindingTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ControlBindingTests.cs index ed7a6bddc3..df62a1ed41 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Xaml/ControlBindingTests.cs +++ b/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'. | " + - "Could not convert FallbackValue 'bar' to 'System.Double'") + (string)pv[2] == "Could not convert FallbackValue 'bar' to 'System.Double'") { called = true; }