From 715149b1f5b5b11696b00adbd0b4ed71abeb4f29 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 10 Oct 2016 22:35:44 +0200 Subject: [PATCH] Improve stream operator error message. When a stream operator is applied to an unsupported type. --- src/Avalonia.Base/Avalonia.Base.csproj | 2 +- ...lException.cs => BindingChainException.cs} | 57 ++++++++----------- src/Avalonia.Controls/TextBox.cs | 2 +- .../Avalonia.Markup/Avalonia.Markup.csproj | 2 +- .../Avalonia.Markup/Data/ExpressionNode.cs | 4 +- .../Data/ExpressionObserver.cs | 2 +- .../Data/MarkupBindingChainException.cs | 42 ++++++++++++++ .../Data/MarkupBindingChainNullException.cs | 33 ----------- src/Markup/Avalonia.Markup/Data/StreamNode.cs | 2 +- .../ExpressionObserverTests_DataValidation.cs | 2 +- .../ExpressionObserverTests_Observable.cs | 25 ++++++++ .../Data/ExpressionObserverTests_Property.cs | 4 +- 12 files changed, 102 insertions(+), 75 deletions(-) rename src/Avalonia.Base/Data/{BindingChainNullException.cs => BindingChainException.cs} (52%) create mode 100644 src/Markup/Avalonia.Markup/Data/MarkupBindingChainException.cs delete mode 100644 src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs diff --git a/src/Avalonia.Base/Avalonia.Base.csproj b/src/Avalonia.Base/Avalonia.Base.csproj index bc52e31d2c..63f500270d 100644 --- a/src/Avalonia.Base/Avalonia.Base.csproj +++ b/src/Avalonia.Base/Avalonia.Base.csproj @@ -44,7 +44,7 @@ Properties\SharedAssemblyInfo.cs - + diff --git a/src/Avalonia.Base/Data/BindingChainNullException.cs b/src/Avalonia.Base/Data/BindingChainException.cs similarity index 52% rename from src/Avalonia.Base/Data/BindingChainNullException.cs rename to src/Avalonia.Base/Data/BindingChainException.cs index 0e50a36d8a..97b0d3ba8b 100644 --- a/src/Avalonia.Base/Data/BindingChainNullException.cs +++ b/src/Avalonia.Base/Data/BindingChainException.cs @@ -10,36 +10,39 @@ namespace Avalonia.Data /// requested binding expression could not be evaluated because of a null in one of the links /// of the binding chain. /// - public class BindingChainNullException : Exception + public class BindingChainException : Exception { private string _message; /// - /// Initalizes a new instance of the class. + /// Initalizes a new instance of the class. /// - public BindingChainNullException() + public BindingChainException() { } /// - /// Initalizes a new instance of the class. + /// Initalizes a new instance of the class. /// - public BindingChainNullException(string message) + /// The error message. + public BindingChainException(string message) { _message = message; } /// - /// Initalizes a new instance of the class. + /// Initalizes a new instance of the class. /// + /// The error message. /// The expression. - /// - /// The point in the expression at which the null was encountered. + /// + /// The point in the expression at which the error was encountered. /// - public BindingChainNullException(string expression, string expressionNullPoint) + public BindingChainException(string message, string expression, string errorPoint) { + _message = message; Expression = expression; - ExpressionNullPoint = expressionNullPoint; + ExpressionErrorPoint = errorPoint; } /// @@ -48,37 +51,27 @@ namespace Avalonia.Data public string Expression { get; protected set; } /// - /// Gets the point in the expression at which the null was encountered. + /// Gets the point in the expression at which the error occured. /// - public string ExpressionNullPoint { get; protected set; } + public string ExpressionErrorPoint { get; protected set; } /// public override string Message { get { - if (_message == null) + if (Expression != null && ExpressionErrorPoint != null) { - _message = BuildMessage(); + return $"{_message} in expression '{Expression}' at '{ExpressionErrorPoint}'."; + } + else if (ExpressionErrorPoint != null) + { + return $"{_message} in expression '{ExpressionErrorPoint}'."; + } + else + { + return $"{_message} in expression."; } - - 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.Controls/TextBox.cs b/src/Avalonia.Controls/TextBox.cs index ed92899cc6..bc8da38724 100644 --- a/src/Avalonia.Controls/TextBox.cs +++ b/src/Avalonia.Controls/TextBox.cs @@ -547,7 +547,7 @@ namespace Avalonia.Controls var exceptions = aggregate == null ? (IEnumerable)new[] { exception } : aggregate.InnerExceptions; - var filtered = exceptions.Where(x => !(x is BindingChainNullException)).ToList(); + var filtered = exceptions.Where(x => !(x is BindingChainException)).ToList(); if (filtered.Count > 0) { diff --git a/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj b/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj index 616891c0ea..d97ab89a09 100644 --- a/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj +++ b/src/Markup/Avalonia.Markup/Avalonia.Markup.csproj @@ -43,7 +43,7 @@ Properties\SharedAssemblyInfo.cs - + diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs b/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs index 9d590b0e19..93f20e4c77 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionNode.cs @@ -88,7 +88,7 @@ namespace Avalonia.Markup.Data protected virtual void NextValueChanged(object value) { - var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainNullException; + var bindingBroken = BindingNotification.ExtractError(value) as MarkupBindingChainException; bindingBroken?.AddNode(Description); _observer.OnNext(value); } @@ -152,7 +152,7 @@ namespace Avalonia.Markup.Data private BindingNotification TargetNullNotification() { return new BindingNotification( - new MarkupBindingChainNullException(), + new MarkupBindingChainException("Null value"), BindingErrorType.Error, AvaloniaProperty.UnsetValue); } diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs index 819949b7b9..90c1aa2894 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs @@ -235,7 +235,7 @@ namespace Avalonia.Markup.Data } else { - var broken = BindingNotification.ExtractError(o) as MarkupBindingChainNullException; + var broken = BindingNotification.ExtractError(o) as MarkupBindingChainException; if (broken != null) { diff --git a/src/Markup/Avalonia.Markup/Data/MarkupBindingChainException.cs b/src/Markup/Avalonia.Markup/Data/MarkupBindingChainException.cs new file mode 100644 index 0000000000..dab5756976 --- /dev/null +++ b/src/Markup/Avalonia.Markup/Data/MarkupBindingChainException.cs @@ -0,0 +1,42 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Avalonia.Data; + +namespace Avalonia.Markup.Data +{ + internal class MarkupBindingChainException : BindingChainException + { + private IList _nodes = new List(); + + public MarkupBindingChainException(string message) + : base(message) + { + } + + public MarkupBindingChainException(string message, string node) + : base(message) + { + AddNode(node); + } + + public MarkupBindingChainException(string message, string expression, string expressionNullPoint) + : base(message, 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; + ExpressionErrorPoint = string.Join(".", _nodes.Reverse()) + .Replace(".!", "!") + .Replace(".[", "[") + .Replace(".^", "^"); + _nodes = null; + } + } +} diff --git a/src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs b/src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs deleted file mode 100644 index a549d6ebb6..0000000000 --- a/src/Markup/Avalonia.Markup/Data/MarkupBindingChainNullException.cs +++ /dev/null @@ -1,33 +0,0 @@ -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/src/Markup/Avalonia.Markup/Data/StreamNode.cs b/src/Markup/Avalonia.Markup/Data/StreamNode.cs index 7513b4323f..7a5cfe5009 100644 --- a/src/Markup/Avalonia.Markup/Data/StreamNode.cs +++ b/src/Markup/Avalonia.Markup/Data/StreamNode.cs @@ -24,7 +24,7 @@ namespace Avalonia.Markup.Data // TODO: Improve error. return Observable.Return(new BindingNotification( - new InvalidCastException("Value could not be streamed."), + new MarkupBindingChainException("Stream operator applied to unsupported type", Description), BindingErrorType.Error)); } } diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_DataValidation.cs index 546cfe015f..3b5ca26db1 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 MarkupBindingChainNullException("Inner.MustBePositive", "Inner"), + new MarkupBindingChainException("Null value", "Inner.MustBePositive", "Inner"), BindingErrorType.Error, AvaloniaProperty.UnsetValue), }, result); diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs index 5e1a392f96..640d82fa19 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Observable.cs @@ -110,6 +110,31 @@ namespace Avalonia.Markup.UnitTests.Data } } + [Fact] + public void Should_Return_BindingNotification_If_Stream_Operator_Applied_To_Not_Supported_Type() + { + using (var sync = UnitTestSynchronizationContext.Begin()) + { + var data = new Class2("foo"); + var target = new ExpressionObserver(data, "Foo^", true); + var result = new List(); + + var sub = target.Subscribe(x => result.Add(x)); + sync.ExecutePostedCallbacks(); + + Assert.Equal( + new[] + { + new BindingNotification( + new MarkupBindingChainException("Stream operator applied to unsupported type", "Foo^", "Foo^"), + BindingErrorType.Error) + }, + result); + + sub.Dispose(); + } + } + private class Class1 : NotifyingBase { public Subject Next { get; } = new Subject(); diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs index aa9ee7d58b..bdcd39d997 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Property.cs @@ -146,7 +146,7 @@ namespace Avalonia.Markup.UnitTests.Data new[] { new BindingNotification( - new MarkupBindingChainNullException("Foo.Bar.Baz", "Foo"), + new MarkupBindingChainException("Null value", "Foo.Bar.Baz", "Foo"), BindingErrorType.Error, AvaloniaProperty.UnsetValue), }, @@ -274,7 +274,7 @@ namespace Avalonia.Markup.UnitTests.Data { "bar", new BindingNotification( - new MarkupBindingChainNullException("Next.Next.Bar", "Next.Next"), + new MarkupBindingChainException("Null value", "Next.Next.Bar", "Next.Next"), BindingErrorType.Error, AvaloniaProperty.UnsetValue), "bar"