Browse Source

Fix negation of null values. (#16101)

* Add tests for binding negation operator.

Expected results come from 11.0.x branch.

* Unset and null need to be distinct.

Also remove `ExpressionNode.Reset` as it was doing unneeded stuff and should just be the same as `SetSource(AvaloniaProperty.UnsetValue, null);`.

* Make ExpressionNode.OnSourceChanged accept null.

The previous design assumed that a source of `null` was an invalid input to all types of expression nodes. It turned out that there was a single exception to this rule: the `!` operator can in fact operate on a null value. With this new design we instead have to explicitly check for a null value in every override of `OnSourceChanged ` except in `LogicalNotNode`.

Due to this change, we also have to distinguish between `null` and `(unset)` in `ExpressionNode.SetSource` as well.

Fixes #16071

* Fix comment.
release/11.1.0-rc2
Steven Kirk 2 years ago
parent
commit
bbf11be8de
  1. 2
      src/Avalonia.Base/Data/BindingNotification.cs
  2. 14
      src/Avalonia.Base/Data/Core/BindingExpression.cs
  3. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/ArrayIndexerNode.cs
  4. 3
      src/Avalonia.Base/Data/Core/ExpressionNodes/AvaloniaPropertyAccessorNode.cs
  5. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/CollectionNodeBase.cs
  6. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/DataContextNode.cs
  7. 49
      src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs
  8. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/FuncTransformNode.cs
  9. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/LogicalAncestorElementNode.cs
  10. 4
      src/Avalonia.Base/Data/Core/ExpressionNodes/LogicalNotNode.cs
  11. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/MethodCommandNode.cs
  12. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/NamedElementNode.cs
  13. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/ParentDataContextNode.cs
  14. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/PropertyAccessorNode.cs
  15. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/DynamicPluginPropertyAccessorNode.cs
  16. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/DynamicPluginStreamNode.cs
  17. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/ReflectionIndexerNode.cs
  18. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/ReflectionTypeCastNode.cs
  19. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/StreamNode.cs
  20. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/TemplatedParentNode.cs
  21. 5
      src/Avalonia.Base/Data/Core/ExpressionNodes/VisualAncestorElementNode.cs
  22. 111
      tests/Avalonia.Markup.Xaml.UnitTests/Xaml/BindingTests.cs

2
src/Avalonia.Base/Data/BindingNotification.cs

@ -178,7 +178,7 @@ namespace Avalonia.Data
/// to <paramref name="value"/>. If <paramref name="value"/> is a
/// <see cref="BindingNotification"/> then the value will first be extracted.
/// </remarks>
public static object? UpdateValue(object o, object value)
public static object? UpdateValue(object? o, object value)
{
if (o is BindingNotification n)
{

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

@ -160,7 +160,7 @@ internal partial class BindingExpression : UntypedBindingExpressionBase, IDescri
var source = _nodes[0].Source;
for (var i = 0; i < _nodes.Count; ++i)
_nodes[i].SetSource(null, null);
_nodes[i].SetSource(AvaloniaProperty.UnsetValue, null);
_nodes[0].SetSource(source, null);
}
@ -253,10 +253,6 @@ internal partial class BindingExpression : UntypedBindingExpressionBase, IDescri
_nodes[nodeIndex + 1].SetSource(value, dataValidationError);
WriteTargetValueToSource();
}
else if (value is null)
{
OnNodeError(nodeIndex, "Value is null.");
}
else
{
_nodes[nodeIndex + 1].SetSource(value, dataValidationError);
@ -273,11 +269,11 @@ internal partial class BindingExpression : UntypedBindingExpressionBase, IDescri
/// <param name="error">The error message.</param>
internal void OnNodeError(int nodeIndex, string error)
{
// Set the source of all nodes after the one that errored to null. This needs to be done
// for each node individually because setting the source to null will not result in
// Set the source of all nodes after the one that errored to unset. This needs to be done
// for each node individually because setting the source to unset will not result in
// OnNodeValueChanged or OnNodeError being called.
for (var i = nodeIndex + 1; i < _nodes.Count; ++i)
_nodes[i].SetSource(null, null);
_nodes[i].SetSource(AvaloniaProperty.UnsetValue, null);
if (_mode == BindingMode.OneWayToSource)
return;
@ -394,7 +390,7 @@ internal partial class BindingExpression : UntypedBindingExpressionBase, IDescri
protected override void StopCore()
{
foreach (var node in _nodes)
node.Reset();
node.SetSource(AvaloniaProperty.UnsetValue, null);
if (_mode is BindingMode.TwoWay or BindingMode.OneWayToSource &&
TryGetTarget(out var target))

5
src/Avalonia.Base/Data/Core/ExpressionNodes/ArrayIndexerNode.cs

@ -44,8 +44,11 @@ internal sealed class ArrayIndexerNode : ExpressionNode, ISettableNode
return false;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is Array array)
SetValue(array.GetValue(_indexes));
else

3
src/Avalonia.Base/Data/Core/ExpressionNodes/AvaloniaPropertyAccessorNode.cs

@ -38,6 +38,9 @@ internal sealed class AvaloniaPropertyAccessorNode :
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is AvaloniaObject newObject)
{
WeakEvents.AvaloniaPropertyChanged.Subscribe(newObject, this);

5
src/Avalonia.Base/Data/Core/ExpressionNodes/CollectionNodeBase.cs

@ -22,8 +22,11 @@ internal abstract class CollectionNodeBase : ExpressionNode,
UpdateValueOrSetError(sender);
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
Subscribe(source);
UpdateValue(source);
}

5
src/Avalonia.Base/Data/Core/ExpressionNodes/DataContextNode.cs

@ -4,8 +4,11 @@ namespace Avalonia.Data.Core.ExpressionNodes;
internal sealed class DataContextNode : DataContextNodeBase
{
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is IDataContextProvider && source is AvaloniaObject ao)
{
ao.PropertyChanged += OnPropertyChanged;

49
src/Avalonia.Base/Data/Core/ExpressionNodes/ExpressionNode.cs

@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Text;
@ -60,16 +61,6 @@ internal abstract class ExpressionNode
BuildString(builder);
}
/// <summary>
/// Resets the node to its uninitialized state when the <see cref="Owner"/> is unsubscribed.
/// </summary>
public void Reset()
{
SetSource(null, null);
_source = null;
_value = AvaloniaProperty.UnsetValue;
}
/// <summary>
/// Sets the owner binding.
/// </summary>
@ -101,28 +92,26 @@ internal abstract class ExpressionNode
/// </param>
public void SetSource(object? source, Exception? dataValidationError)
{
var oldSource = Source;
if (source == AvaloniaProperty.UnsetValue)
source = null;
if (_source?.TryGetTarget(out var oldSource) != true)
oldSource = AvaloniaProperty.UnsetValue;
if (source == oldSource)
return;
if (oldSource is not null)
if (oldSource is not null && oldSource != AvaloniaProperty.UnsetValue)
Unsubscribe(oldSource);
_source = new(source);
if (source is null)
if (source == AvaloniaProperty.UnsetValue)
{
// If the source is null then the value is null. We explicitly do not want to call
// If the source is unset then the value is unset. We explicitly do not want to call
// OnSourceChanged as we don't want to raise errors for subsequent nodes in the
// binding change.
_source = null;
_value = AvaloniaProperty.UnsetValue;
}
else
{
_source = new(source);
try { OnSourceChanged(source, dataValidationError); }
catch (Exception e) { SetError(e); }
}
@ -242,6 +231,26 @@ internal abstract class ExpressionNode
}
}
/// <summary>
/// Called from <see cref="OnSourceChanged(object?, Exception?)"/> to validate that the source
/// is non-null and raise a node error if it is not.
/// </summary>
/// <param name="source">The expression node source.</param>
/// <returns>
/// True if the source is non-null; otherwise, false.
/// </returns>
protected bool ValidateNonNullSource([NotNullWhen(true)] object? source)
{
if (source is null)
{
Owner?.OnNodeError(Index - 1, "Value is null.");
_value = null;
return false;
}
return true;
}
/// <summary>
/// When implemented in a derived class, subscribes to the new source, and updates the current
/// <see cref="Value"/>.
@ -250,7 +259,7 @@ internal abstract class ExpressionNode
/// <param name="dataValidationError">
/// Any data validation error reported by the previous expression node.
/// </param>
protected abstract void OnSourceChanged(object source, Exception? dataValidationError);
protected abstract void OnSourceChanged(object? source, Exception? dataValidationError);
/// <summary>
/// When implemented in a derived class, unsubscribes from the previous source.

5
src/Avalonia.Base/Data/Core/ExpressionNodes/FuncTransformNode.cs

@ -21,8 +21,11 @@ internal sealed class FuncTransformNode : ExpressionNode
// We don't have enough information to add anything here.
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
SetValue(_transform(source));
}
}

5
src/Avalonia.Base/Data/Core/ExpressionNodes/LogicalAncestorElementNode.cs

@ -56,8 +56,11 @@ internal sealed class LogicalAncestorElementNode : SourceNode
return target is ILogical logical && logical.IsAttachedToLogicalTree;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is ILogical logical)
{
var locator = ControlLocator.Track(logical, _ancestorLevel, _ancestorType);

4
src/Avalonia.Base/Data/Core/ExpressionNodes/LogicalNotNode.cs

@ -28,14 +28,12 @@ internal sealed class LogicalNotNode : ExpressionNode, ISettableNode
return false;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
var v = BindingNotification.ExtractValue(source);
if (TryConvert(v, out var value))
{
SetValue(BindingNotification.UpdateValue(source, !value), dataValidationError);
}
else
SetError($"Unable to convert '{source}' to bool.");
}

5
src/Avalonia.Base/Data/Core/ExpressionNodes/MethodCommandNode.cs

@ -39,8 +39,11 @@ internal sealed class MethodCommandNode : ExpressionNode, IWeakEventSubscriber<P
builder.Append("()");
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is INotifyPropertyChanged newInpc)
WeakEvents.ThreadSafePropertyChanged.Subscribe(newInpc, this);

5
src/Avalonia.Base/Data/Core/ExpressionNodes/NamedElementNode.cs

@ -30,8 +30,11 @@ internal sealed class NamedElementNode : SourceNode
return target is not ILogical logical || logical.IsAttachedToLogicalTree;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (_nameScope.TryGetTarget(out var scope))
_subscription = NameScopeLocator.Track(scope, _name).Subscribe(SetValue);
else

5
src/Avalonia.Base/Data/Core/ExpressionNodes/ParentDataContextNode.cs

@ -11,8 +11,11 @@ internal sealed class ParentDataContextNode : DataContextNodeBase
private static readonly AvaloniaObject s_unset = new();
private AvaloniaObject? _parent = s_unset;
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is AvaloniaObject newElement)
newElement.PropertyChanged += OnPropertyChanged;

5
src/Avalonia.Base/Data/Core/ExpressionNodes/PropertyAccessorNode.cs

@ -48,8 +48,11 @@ internal sealed class PropertyAccessorNode : ExpressionNode, IPropertyAccessorNo
}
[UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "<Pending>")]
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
var reference = new WeakReference<object?>(source);
if (_plugin.Start(reference, PropertyName) is { } accessor)

5
src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/DynamicPluginPropertyAccessorNode.cs

@ -42,8 +42,11 @@ internal sealed class DynamicPluginPropertyAccessorNode : ExpressionNode, IPrope
return _accessor?.SetValue(value, BindingPriority.LocalValue) ?? false;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
var reference = new WeakReference<object?>(source);
if (GetPlugin(source) is { } plugin &&

5
src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/DynamicPluginStreamNode.cs

@ -16,8 +16,11 @@ internal sealed class DynamicPluginStreamNode : ExpressionNode
builder.Append('^');
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
var reference = new WeakReference<object?>(source);
if (GetPlugin(reference) is { } plugin &&

5
src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/ReflectionIndexerNode.cs

@ -52,8 +52,11 @@ internal sealed class ReflectionIndexerNode : CollectionNodeBase, ISettableNode
return true;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
_indexes = null;
if (GetIndexer(source.GetType(), out _getter, out _setter))

5
src/Avalonia.Base/Data/Core/ExpressionNodes/Reflection/ReflectionTypeCastNode.cs

@ -19,8 +19,11 @@ internal sealed class ReflectionTypeCastNode : ExpressionNode
builder.Append(')');
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (_targetType.IsInstanceOfType(source))
SetValue(source);
else

5
src/Avalonia.Base/Data/Core/ExpressionNodes/StreamNode.cs

@ -23,8 +23,11 @@ internal sealed class StreamNode : ExpressionNode, IObserver<object?>
void IObserver<object?>.OnError(Exception error) { }
void IObserver<object?>.OnNext(object? value) => SetValue(value);
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (_plugin.Start(new(source)) is { } accessor)
{
_subscription = accessor.Subscribe(this);

5
src/Avalonia.Base/Data/Core/ExpressionNodes/TemplatedParentNode.cs

@ -22,8 +22,11 @@ internal sealed class TemplatedParentNode : SourceNode
throw new InvalidOperationException("Cannot find a StyledElement to get a TemplatedParent.");
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is StyledElement newElement)
{
newElement.PropertyChanged += OnPropertyChanged;

5
src/Avalonia.Base/Data/Core/ExpressionNodes/VisualAncestorElementNode.cs

@ -56,8 +56,11 @@ internal sealed class VisualAncestorElementNode : SourceNode
return target is Visual visual && visual.IsAttachedToVisualTree;
}
protected override void OnSourceChanged(object source, Exception? dataValidationError)
protected override void OnSourceChanged(object? source, Exception? dataValidationError)
{
if (!ValidateNonNullSource(source))
return;
if (source is Visual visual)
{
var locator = VisualLocator.Track(visual, _ancestorLevel, _ancestorType);

111
tests/Avalonia.Markup.Xaml.UnitTests/Xaml/BindingTests.cs

@ -1,4 +1,6 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Globalization;
using System.Reactive.Subjects;
using Avalonia.Controls;
@ -426,11 +428,120 @@ namespace Avalonia.Markup.Xaml.UnitTests.Xaml
}
}
[Theory]
[MemberData(nameof(NegationData))]
public void Negating_Object_Returns_Correct_Value(object value, bool? expected)
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var xaml = @"
<Window xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
Tag='{Binding !Object}'>
</Window>";
var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml);
var viewModel = new WindowViewModel { Object = value };
window.DataContext = viewModel;
window.ApplyTemplate();
Assert.Equal(expected, window.Tag);
}
}
[Theory]
[MemberData(nameof(NegationData))]
public void Double_Negating_Object_Returns_Correct_Value(object value, bool? negated)
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var xaml = @"
<Window xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
Tag='{Binding !!Object}'>
</Window>";
var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml);
var viewModel = new WindowViewModel { Object = value };
window.DataContext = viewModel;
window.ApplyTemplate();
var expected = negated.HasValue ? !negated : null;
Assert.Equal(expected, window.Tag);
}
}
[Theory]
[MemberData(nameof(NegationData))]
public void Negating_Object_Returns_Correct_Value_When_Bound_To_Bool(object value, bool? expected)
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var xaml = @"
<Window xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
IsVisible='{Binding !Object}'>
</Window>";
var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml);
var viewModel = new WindowViewModel { Object = value };
window.DataContext = viewModel;
window.ApplyTemplate();
Assert.Equal(expected ?? false, window.IsVisible);
}
}
[Theory]
[MemberData(nameof(NegationData))]
public void Double_Negating_Object_Returns_Correct_Value_When_Bound_To_Bool(object value, bool? negated)
{
using (UnitTestApplication.Start(TestServices.StyledWindow))
{
var xaml = @"
<Window xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'
IsVisible='{Binding !!Object}'>
</Window>";
var window = (Window)AvaloniaRuntimeXamlLoader.Load(xaml);
var viewModel = new WindowViewModel { Object = value };
window.DataContext = viewModel;
window.ApplyTemplate();
var expected = negated.HasValue ? !negated : false;
Assert.Equal(expected, window.IsVisible);
}
}
public static IEnumerable<object[]> NegationData()
{
yield return new object[] { true, false };
yield return new object[] { false, true };
yield return new object[] { null, true };
yield return new object[] { new object(), null };
yield return new object[] { "foo", null };
yield return new object[] { "true", false };
yield return new object[] { "false", true };
yield return new object[] { 0, true };
yield return new object[] { 1, false };
yield return new object[] { 2, false };
yield return new object[] { -1, false };
yield return new object[] { 0.0, true };
yield return new object[] { 1.0, false };
yield return new object[] { 2.0, false };
yield return new object[] { -1.0, false };
yield return new object[] { double.NaN, false };
yield return new object[] { double.PositiveInfinity, false };
yield return new object[] { double.NegativeInfinity, false };
}
private class WindowViewModel
{
public bool ShowInTaskbar { get; set; }
public string Greeting1 { get; set; } = "Hello";
public string Greeting2 { get; set; } = "World";
public object Object { get; set; }
}
public class CultureAppender : IValueConverter

Loading…
Cancel
Save