From e8c32bf801a6f36270155657b4665821019f986a Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 17 May 2018 17:39:46 -0500 Subject: [PATCH 01/34] Preliminary support. Indexers require more work since the compiler doesn't generate IndexExpressions (they weren't in S.L.Expressions v1 so they aren't auto-genned). --- .../Data/ExpressionNodeBuilder.cs | 8 + .../Data/ExpressionObserver.cs | 92 ++++++++-- .../Data/ExpressionParseException.cs | 4 +- .../Data/IndexerExpressionNode.cs | 61 +++++++ .../Avalonia.Markup/Data/IndexerNode.cs | 80 ++------- .../Avalonia.Markup/Data/IndexerNodeBase.cs | 87 ++++++++++ .../Data/Parsers/ExpressionTreeParser.cs | 34 ++++ .../Parsers/ExpressionVisitorNodeBuilder.cs | 158 ++++++++++++++++++ .../ExpressionObserverTests_ExpressionTree.cs | 111 ++++++++++++ 9 files changed, 555 insertions(+), 80 deletions(-) create mode 100644 src/Markup/Avalonia.Markup/Data/IndexerExpressionNode.cs create mode 100644 src/Markup/Avalonia.Markup/Data/IndexerNodeBase.cs create mode 100644 src/Markup/Avalonia.Markup/Data/Parsers/ExpressionTreeParser.cs create mode 100644 src/Markup/Avalonia.Markup/Data/Parsers/ExpressionVisitorNodeBuilder.cs create mode 100644 tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_ExpressionTree.cs diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionNodeBuilder.cs b/src/Markup/Avalonia.Markup/Data/ExpressionNodeBuilder.cs index 013299c1d7..e19259c6ed 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionNodeBuilder.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionNodeBuilder.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; +using System.Linq.Expressions; using Avalonia.Markup.Data.Parsers; namespace Avalonia.Markup.Data @@ -26,5 +27,12 @@ namespace Avalonia.Markup.Data return node; } + + public static ExpressionNode Build(LambdaExpression expression, bool enableValidation = false) + { + var parser = new ExpressionTreeParser(enableValidation); + + return parser.Parse(expression); + } } } diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs index dd9718a0f6..127b338008 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionObserver.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq.Expressions; using System.Reactive; using System.Reactive.Disposables; using System.Reactive.Linq; @@ -72,20 +73,36 @@ namespace Avalonia.Markup.Data string expression, bool enableDataValidation = false, string description = null) + : this(root, Parse(expression, enableDataValidation), description ?? expression) { Contract.Requires(expression != null); + Expression = expression; + } + private ExpressionObserver( + object root, + ExpressionNode node, + string description = null) + { if (root == AvaloniaProperty.UnsetValue) { root = null; } - Expression = expression; - Description = description ?? expression; - _node = Parse(expression, enableDataValidation); + _node = node; + Description = description; _root = new WeakReference(root); } + public static ExpressionObserver CreateFromExpression( + T root, + Expression> expression, + bool enableDataValidation = false, + string description = null) + { + return new ExpressionObserver(root, Parse(expression, enableDataValidation), description ?? expression.ToString()); + } + /// /// Initializes a new instance of the class. /// @@ -100,15 +117,38 @@ namespace Avalonia.Markup.Data string expression, bool enableDataValidation = false, string description = null) + : this(rootObservable, Parse(expression, enableDataValidation), description ?? expression) { Contract.Requires(rootObservable != null); Contract.Requires(expression != null); Expression = expression; - Description = description ?? expression; - _node = Parse(expression, enableDataValidation); - _finished = new Subject(); + } + + private ExpressionObserver( + IObservable rootObservable, + ExpressionNode node, + string description) + { + Contract.Requires(rootObservable != null); + + _node = node; + Description = description; _root = rootObservable; + _finished = new Subject(); + } + + public static ExpressionObserver CreateFromExpression( + IObservable rootObservable, + Expression> expression, + bool enableDataValidation = false, + string description = null) + { + Contract.Requires(rootObservable != null); + return new ExpressionObserver( + rootObservable.Select(o => (object)o), + Parse(expression, enableDataValidation), + description ?? expression.ToString()); } /// @@ -127,19 +167,44 @@ namespace Avalonia.Markup.Data IObservable update, bool enableDataValidation = false, string description = null) + : this(rootGetter, Parse(expression, enableDataValidation), update, description ?? expression) { - Contract.Requires(rootGetter != null); Contract.Requires(expression != null); - Contract.Requires(update != null); Expression = expression; - Description = description ?? expression; - _node = Parse(expression, enableDataValidation); - _finished = new Subject(); + } + + private ExpressionObserver( + Func rootGetter, + ExpressionNode node, + IObservable update, + string description) + { + Contract.Requires(rootGetter != null); + Contract.Requires(update != null); + Description = description; + _node = node; + _finished = new Subject(); _node.Target = new WeakReference(rootGetter()); _root = update.Select(x => rootGetter()); } + + public static ExpressionObserver CreateFromExpression( + Func rootGetter, + Expression> expression, + IObservable update, + bool enableDataValidation = false, + string description = null) + { + Contract.Requires(rootGetter != null); + + return new ExpressionObserver( + () => rootGetter(), + Parse(expression, enableDataValidation), + update, + description ?? expression.ToString()); + } /// /// Attempts to set the value of a property expression. @@ -238,6 +303,11 @@ namespace Avalonia.Markup.Data } } + private static ExpressionNode Parse(LambdaExpression expression, bool enableDataValidation) + { + return ExpressionNodeBuilder.Build(expression, enableDataValidation); + } + private static object ToWeakReference(object o) { return o is BindingNotification ? o : new WeakReference(o); diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionParseException.cs b/src/Markup/Avalonia.Markup/Data/ExpressionParseException.cs index d06bdd1e52..3ef225e70a 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionParseException.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionParseException.cs @@ -17,8 +17,8 @@ namespace Avalonia.Markup.Data /// /// The column position of the error. /// The exception message. - public ExpressionParseException(int column, string message) - : base(message) + public ExpressionParseException(int column, string message, Exception innerException = null) + : base(message, innerException) { Column = column; } diff --git a/src/Markup/Avalonia.Markup/Data/IndexerExpressionNode.cs b/src/Markup/Avalonia.Markup/Data/IndexerExpressionNode.cs new file mode 100644 index 0000000000..b296badb86 --- /dev/null +++ b/src/Markup/Avalonia.Markup/Data/IndexerExpressionNode.cs @@ -0,0 +1,61 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq.Expressions; +using System.Text; +using Avalonia.Data; + +namespace Avalonia.Markup.Data +{ + class IndexerExpressionNode : IndexerNodeBase + { + private readonly ParameterExpression parameter; + private readonly IndexExpression expression; + private readonly Delegate setDelegate; + private readonly Delegate getDelegate; + private readonly Delegate firstArgumentDelegate; + + public IndexerExpressionNode(IndexExpression expression) + { + parameter = Expression.Parameter(expression.Object.Type); + this.expression = expression.Update(parameter, expression.Arguments); + + getDelegate = Expression.Lambda(this.expression, parameter).Compile(); + + var valueParameter = Expression.Parameter(expression.Type); + + setDelegate = Expression.Lambda(Expression.Assign(this.expression, valueParameter), parameter, valueParameter).Compile(); + + firstArgumentDelegate = Expression.Lambda(this.expression.Arguments[0], parameter).Compile(); + } + + public override Type PropertyType => expression.Type; + + public override string Description => expression.ToString(); + + public override bool SetTargetValue(object value, BindingPriority priority) + { + try + { + setDelegate.DynamicInvoke(Target.Target, value); + return true; + } + catch (Exception) + { + return false; + } + } + + protected override object GetValue(object target) + { + return getDelegate.DynamicInvoke(target); + } + + protected override bool ShouldUpdate(object sender, PropertyChangedEventArgs e) + { + return expression.Indexer.Name == e.PropertyName; + } + + protected override int? TryGetFirstArgumentAsInt() => firstArgumentDelegate.DynamicInvoke(Target.Target) as int?; + } +} diff --git a/src/Markup/Avalonia.Markup/Data/IndexerNode.cs b/src/Markup/Avalonia.Markup/Data/IndexerNode.cs index 4e2914a148..09ce2b85e9 100644 --- a/src/Markup/Avalonia.Markup/Data/IndexerNode.cs +++ b/src/Markup/Avalonia.Markup/Data/IndexerNode.cs @@ -15,7 +15,7 @@ using Avalonia.Data; namespace Avalonia.Markup.Data { - internal class IndexerNode : ExpressionNode, ISettableNode + internal class IndexerNode : IndexerNodeBase { public IndexerNode(IList arguments) { @@ -24,35 +24,7 @@ namespace Avalonia.Markup.Data public override string Description => "[" + string.Join(",", Arguments) + "]"; - protected override IObservable StartListeningCore(WeakReference reference) - { - var target = reference.Target; - var incc = target as INotifyCollectionChanged; - var inpc = target as INotifyPropertyChanged; - var inputs = new List>(); - - if (incc != null) - { - inputs.Add(WeakObservable.FromEventPattern( - incc, - nameof(incc.CollectionChanged)) - .Where(x => ShouldUpdate(x.Sender, x.EventArgs)) - .Select(_ => GetValue(target))); - } - - if (inpc != null) - { - inputs.Add(WeakObservable.FromEventPattern( - inpc, - nameof(inpc.PropertyChanged)) - .Where(x => ShouldUpdate(x.Sender, x.EventArgs)) - .Select(_ => GetValue(target))); - } - - return Observable.Merge(inputs).StartWith(GetValue(target)); - } - - public bool SetTargetValue(object value, BindingPriority priority) + public override bool SetTargetValue(object value, BindingPriority priority) { var typeInfo = Target.Target.GetType().GetTypeInfo(); var list = Target.Target as IList; @@ -154,9 +126,9 @@ namespace Avalonia.Markup.Data public IList Arguments { get; } - public Type PropertyType => GetIndexer(Target.Target.GetType().GetTypeInfo())?.PropertyType; + public override Type PropertyType => GetIndexer(Target.Target.GetType().GetTypeInfo())?.PropertyType; - private object GetValue(object target) + protected override object GetValue(object target) { var typeInfo = target.GetType().GetTypeInfo(); var list = target as IList; @@ -309,45 +281,19 @@ namespace Avalonia.Markup.Data } } - private bool ShouldUpdate(object sender, NotifyCollectionChangedEventArgs e) + protected override bool ShouldUpdate(object sender, PropertyChangedEventArgs e) { - if (sender is IList) - { - object indexObject; - - if (!TypeUtilities.TryConvert(typeof(int), Arguments[0], CultureInfo.InvariantCulture, out indexObject)) - { - return false; - } - - var index = (int)indexObject; - - switch (e.Action) - { - case NotifyCollectionChangedAction.Add: - return index >= e.NewStartingIndex; - case NotifyCollectionChangedAction.Remove: - return index >= e.OldStartingIndex; - case NotifyCollectionChangedAction.Replace: - return index >= e.NewStartingIndex && - index < e.NewStartingIndex + e.NewItems.Count; - case NotifyCollectionChangedAction.Move: - return (index >= e.NewStartingIndex && - index < e.NewStartingIndex + e.NewItems.Count) || - (index >= e.OldStartingIndex && - index < e.OldStartingIndex + e.OldItems.Count); - case NotifyCollectionChangedAction.Reset: - return true; - } - } - - return true; // Implementation defined meaning for the index, so just try to update anyway + var typeInfo = sender.GetType().GetTypeInfo(); + return typeInfo.GetDeclaredProperty(e.PropertyName)?.GetIndexParameters().Any() ?? false; } - private bool ShouldUpdate(object sender, PropertyChangedEventArgs e) + protected override int? TryGetFirstArgumentAsInt() { - var typeInfo = sender.GetType().GetTypeInfo(); - return typeInfo.GetDeclaredProperty(e.PropertyName)?.GetIndexParameters().Any() ?? false; + if (TypeUtilities.TryConvert(typeof(int), Arguments[0], CultureInfo.InvariantCulture, out var value)) + { + return (int?)value; + } + return null; } } } diff --git a/src/Markup/Avalonia.Markup/Data/IndexerNodeBase.cs b/src/Markup/Avalonia.Markup/Data/IndexerNodeBase.cs new file mode 100644 index 0000000000..d3a4a818fe --- /dev/null +++ b/src/Markup/Avalonia.Markup/Data/IndexerNodeBase.cs @@ -0,0 +1,87 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; +using System.Globalization; +using System.Linq; +using System.Reactive.Linq; +using System.Reflection; +using System.Text; +using Avalonia.Data; +using Avalonia.Utilities; + +namespace Avalonia.Markup.Data +{ + abstract class IndexerNodeBase : ExpressionNode, ISettableNode + { + protected override IObservable StartListeningCore(WeakReference reference) + { + var target = reference.Target; + var inputs = new List>(); + + if (target is INotifyCollectionChanged incc) + { + inputs.Add(WeakObservable.FromEventPattern( + incc, + nameof(incc.CollectionChanged)) + .Where(x => ShouldUpdate(x.Sender, x.EventArgs)) + .Select(_ => GetValue(target))); + } + + if (target is INotifyPropertyChanged inpc) + { + inputs.Add(WeakObservable.FromEventPattern( + inpc, + nameof(inpc.PropertyChanged)) + .Where(x => ShouldUpdate(x.Sender, x.EventArgs)) + .Select(_ => GetValue(target))); + } + + return inputs.Merge().StartWith(GetValue(target)); + } + + public abstract bool SetTargetValue(object value, BindingPriority priority); + + public abstract Type PropertyType { get; } + + protected abstract object GetValue(object target); + + protected abstract int? TryGetFirstArgumentAsInt(); + + private bool ShouldUpdate(object sender, NotifyCollectionChangedEventArgs e) + { + if (sender is IList) + { + var index = TryGetFirstArgumentAsInt(); + + if (index == null) + { + return false; + } + + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + return index >= e.NewStartingIndex; + case NotifyCollectionChangedAction.Remove: + return index >= e.OldStartingIndex; + case NotifyCollectionChangedAction.Replace: + return index >= e.NewStartingIndex && + index < e.NewStartingIndex + e.NewItems.Count; + case NotifyCollectionChangedAction.Move: + return (index >= e.NewStartingIndex && + index < e.NewStartingIndex + e.NewItems.Count) || + (index >= e.OldStartingIndex && + index < e.OldStartingIndex + e.OldItems.Count); + case NotifyCollectionChangedAction.Reset: + return true; + } + } + + return true; // Implementation defined meaning for the index, so just try to update anyway + } + + protected abstract bool ShouldUpdate(object sender, PropertyChangedEventArgs e); + } +} diff --git a/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionTreeParser.cs b/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionTreeParser.cs new file mode 100644 index 0000000000..9e225ffcc5 --- /dev/null +++ b/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionTreeParser.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Linq.Expressions; +using System.Text; + +namespace Avalonia.Markup.Data.Parsers +{ + class ExpressionTreeParser + { + private readonly bool enableDataValidation; + + public ExpressionTreeParser(bool enableDataValidation) + { + this.enableDataValidation = enableDataValidation; + } + + public ExpressionNode Parse(Expression expr) + { + var visitor = new ExpressionVisitorNodeBuilder(enableDataValidation); + + visitor.Visit(expr); + + var nodes = visitor.Nodes; + + for (int n = 0; n < nodes.Count - 1; ++n) + { + nodes[n].Next = nodes[n + 1]; + } + + return nodes.FirstOrDefault() ?? new EmptyExpressionNode(); + } + } +} diff --git a/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionVisitorNodeBuilder.cs b/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionVisitorNodeBuilder.cs new file mode 100644 index 0000000000..0126d31098 --- /dev/null +++ b/src/Markup/Avalonia.Markup/Data/Parsers/ExpressionVisitorNodeBuilder.cs @@ -0,0 +1,158 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Linq.Expressions; +using System.Reflection; +using System.Text; + +namespace Avalonia.Markup.Data.Parsers +{ + class ExpressionVisitorNodeBuilder : ExpressionVisitor + { + private static PropertyInfo AvaloniaObjectIndexer; + + private readonly bool enableDataValidation; + + static ExpressionVisitorNodeBuilder() + { + AvaloniaObjectIndexer = typeof(AvaloniaObject).GetProperty("Item", new[] { typeof(AvaloniaProperty) }); + } + + public List Nodes { get; } + + public ExpressionVisitorNodeBuilder(bool enableDataValidation) + { + this.enableDataValidation = enableDataValidation; + Nodes = new List(); + } + + protected override Expression VisitUnary(UnaryExpression node) + { + if (node.NodeType != ExpressionType.Not || node.Type != typeof(bool)) + { + throw new ExpressionParseException(0, $"Invalid unary operation {node.NodeType} in binding expression"); + } + + Nodes.Add(new LogicalNotNode()); + + return base.VisitUnary(node); + } + + protected override Expression VisitMember(MemberExpression node) + { + Nodes.Add(new PropertyAccessorNode(node.Member.Name, enableDataValidation)); + return base.VisitMember(node); + } + + protected override Expression VisitIndex(IndexExpression node) + { + if (node.Indexer == AvaloniaObjectIndexer) + { + var property = GetArgumentExpressionValue(node.Arguments[0]); + Nodes.Add(new PropertyAccessorNode($"{property.OwnerType.Name}.{property.Name}", enableDataValidation)); + } + else + { + Nodes.Add(new IndexerExpressionNode(node)); + } + + return node; + } + + private T GetArgumentExpressionValue(Expression expr) + { + try + { + return Expression.Lambda>(expr).Compile(preferInterpretation: true)(); + } + catch (InvalidOperationException ex) + { + throw new ExpressionParseException(0, "Unable to parse indexer value.", ex); + } + } + + protected override Expression VisitBinary(BinaryExpression node) + { + if (node.NodeType == ExpressionType.ArrayIndex) + { + return base.VisitBinary(node); + } + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitBlock(BlockExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override CatchBlock VisitCatchBlock(CatchBlock node) + { + throw new ExpressionParseException(0, $"Catch blocks are not allowed in binding expressions."); + } + + protected override Expression VisitConditional(ConditionalExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitDynamic(DynamicExpression node) + { + throw new ExpressionParseException(0, $"Dynamic expressions are not allowed in binding expressions."); + } + + protected override ElementInit VisitElementInit(ElementInit node) + { + throw new ExpressionParseException(0, $"Element init expressions are not valid in a binding expression."); + } + + protected override Expression VisitGoto(GotoExpression node) + { + throw new ExpressionParseException(0, $"Goto expressions not supported in binding expressions."); + } + + protected override Expression VisitInvocation(InvocationExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitLabel(LabelExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitListInit(ListInitExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitLoop(LoopExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override MemberAssignment VisitMemberAssignment(MemberAssignment node) + { + throw new ExpressionParseException(0, $"Member assignments not supported in binding expressions."); + } + + protected override Expression VisitMethodCall(MethodCallExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitSwitch(SwitchExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitTry(TryExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + + protected override Expression VisitTypeBinary(TypeBinaryExpression node) + { + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); + } + } +} diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_ExpressionTree.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_ExpressionTree.cs new file mode 100644 index 0000000000..3fdd9ffc10 --- /dev/null +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_ExpressionTree.cs @@ -0,0 +1,111 @@ +using System; +using System.Collections.Generic; +using System.Reactive.Linq; +using System.Text; +using System.Threading.Tasks; +using Avalonia.Markup.Data; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Markup.UnitTests.Data +{ + public class ExpressionObserverTests_ExpressionTree + { + [Fact] + public async Task IdentityExpression_Creates_IdentityObserver() + { + var target = new object(); + + var observer = ExpressionObserver.CreateFromExpression(target, o => o); + + Assert.Equal(target, await observer.Take(1)); + } + + [Fact] + public async Task Property_Access_Expression_Observes_Property() + { + var target = new Class1(); + + var observer = ExpressionObserver.CreateFromExpression(target, o => o.Foo); + + Assert.Null(await observer.Take(1)); + + using (observer.Subscribe(_ => {})) + { + target.Foo = "Test"; + } + + Assert.Equal("Test", await observer.Take(1)); + + GC.KeepAlive(target); + } + + [Fact] + public void Property_Acccess_Expression_Can_Set_Property() + { + var data = new Class1(); + var target = ExpressionObserver.CreateFromExpression(data, o => o.Foo); + + using (target.Subscribe(_ => { })) + { + Assert.True(target.SetValue("baz")); + } + + GC.KeepAlive(data); + } + + [Fact] + public async Task Indexer_Accessor_Can_Read_Value() + { + var data = new[] { 1, 2, 3, 4 }; + + var target = ExpressionObserver.CreateFromExpression(data, o => o[0]); + + Assert.Equal(data[0], await target.Take(1)); + } + + [Fact] + public async Task Indexer_Accessor_Can_Read_Complex_Index() + { + var data = new Dictionary(); + + var key = new object(); + + data.Add(key, new object()); + + var target = ExpressionObserver.CreateFromExpression(data, o => o[key]); + + Assert.Equal(data[key], await target.Take(1)); + } + + [Fact] + public void Indexer_Can_Set_Value() + { + var data = new[] { 1, 2, 3, 4 }; + + var target = ExpressionObserver.CreateFromExpression(data, o => o[0]); + + using (target.Subscribe(_ => { })) + { + Assert.True(target.SetValue(2)); + } + + GC.KeepAlive(data); + } + + private class Class1 : NotifyingBase + { + private string _foo; + + public string Foo + { + get { return _foo; } + set + { + _foo = value; + RaisePropertyChanged(nameof(Foo)); + } + } + } + } +} From bf6375fe266f3825e59eaf5f020c9bfbe955f45a Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Tue, 29 May 2018 16:49:08 -0500 Subject: [PATCH 02/34] Fix indexer and casting expressions. --- .../Parsers/ExpressionVisitorNodeBuilder.cs | 49 ++++++++++-- .../ExpressionObserverTests_ExpressionTree.cs | 80 +++++++++++++++++++ 2 files changed, 122 insertions(+), 7 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs b/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs index 8564bf5111..5affe227e1 100644 --- a/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs +++ b/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -28,24 +29,43 @@ namespace Avalonia.Data.Core.Parsers protected override Expression VisitUnary(UnaryExpression node) { - if (node.NodeType != ExpressionType.Not || node.Type != typeof(bool)) + if (node.NodeType == ExpressionType.Not && node.Type == typeof(bool)) { - throw new ExpressionParseException(0, $"Invalid unary operation {node.NodeType} in binding expression"); + Nodes.Add(new LogicalNotNode()); + } + else if (node.NodeType == ExpressionType.Convert) + { + if (node.Operand.Type.IsAssignableFrom(node.Type)) + { + // Ignore inheritance casts + } + else + { + throw new ExpressionParseException(0, $"Cannot parse non-inheritance casts in a binding expression."); + } + } + else if (node.NodeType == ExpressionType.TypeAs) + { + // Ignore as operator. + } + else + { + throw new ExpressionParseException(0, $"Unable to parse unary operator {node.NodeType} in a binding expression"); } - - Nodes.Add(new LogicalNotNode()); return base.VisitUnary(node); } protected override Expression VisitMember(MemberExpression node) { + var visited = base.VisitMember(node); Nodes.Add(new PropertyAccessorNode(node.Member.Name, enableDataValidation)); - return base.VisitMember(node); + return visited; } protected override Expression VisitIndex(IndexExpression node) { + var visited = base.VisitIndex(node); if (node.Indexer == AvaloniaObjectIndexer) { var property = GetArgumentExpressionValue(node.Arguments[0]); @@ -56,7 +76,7 @@ namespace Avalonia.Data.Core.Parsers Nodes.Add(new IndexerExpressionNode(node)); } - return node; + return visited; } private T GetArgumentExpressionValue(Expression expr) @@ -75,7 +95,8 @@ namespace Avalonia.Data.Core.Parsers { if (node.NodeType == ExpressionType.ArrayIndex) { - return base.VisitBinary(node); + base.VisitBinary(node); + return Visit(Expression.MakeIndex(node.Left, null, new[] { node.Right })); } throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); } @@ -137,9 +158,23 @@ namespace Avalonia.Data.Core.Parsers protected override Expression VisitMethodCall(MethodCallExpression node) { + base.VisitMethodCall(node); + var property = TryGetPropertyFromMethod(node.Method); + + if (property != null) + { + return Visit(Expression.MakeIndex(node.Object, property, node.Arguments)); + } + throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); } + private PropertyInfo TryGetPropertyFromMethod(MethodInfo method) + { + var type = method.DeclaringType; + return type.GetRuntimeProperties().FirstOrDefault(prop => prop.GetMethod == method); + } + protected override Expression VisitSwitch(SwitchExpression node) { throw new ExpressionParseException(0, $"Invalid expression type in binding expression: {node.NodeType}."); diff --git a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_ExpressionTree.cs b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_ExpressionTree.cs index 1ec4bdb4f5..ebf3ca2a49 100644 --- a/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_ExpressionTree.cs +++ b/tests/Avalonia.Base.UnitTests/Data/Core/ExpressionObserverTests_ExpressionTree.cs @@ -19,6 +19,7 @@ namespace Avalonia.Base.UnitTests.Data.Core var observer = ExpressionObserver.CreateFromExpression(target, o => o); Assert.Equal(target, await observer.Take(1)); + GC.KeepAlive(target); } [Fact] @@ -62,6 +63,18 @@ namespace Avalonia.Base.UnitTests.Data.Core var target = ExpressionObserver.CreateFromExpression(data, o => o[0]); Assert.Equal(data[0], await target.Take(1)); + GC.KeepAlive(data); + } + + [Fact] + public async Task Indexer_List_Accessor_Can_Read_Value() + { + var data = new List { 1, 2, 3, 4 }; + + var target = ExpressionObserver.CreateFromExpression(data, o => o[0]); + + Assert.Equal(data[0], await target.Take(1)); + GC.KeepAlive(data); } [Fact] @@ -76,6 +89,8 @@ namespace Avalonia.Base.UnitTests.Data.Core var target = ExpressionObserver.CreateFromExpression(data, o => o[key]); Assert.Equal(data[key], await target.Take(1)); + + GC.KeepAlive(data); } [Fact] @@ -93,6 +108,62 @@ namespace Avalonia.Base.UnitTests.Data.Core GC.KeepAlive(data); } + [Fact] + public async Task Inheritance_Casts_Should_Be_Ignored() + { + NotifyingBase test = new Class1 { Foo = "Test" }; + + var target = ExpressionObserver.CreateFromExpression(test, o => ((Class1)o).Foo); + + Assert.Equal("Test", await target.Take(1)); + + GC.KeepAlive(test); + } + + [Fact] + public void Convert_Casts_Should_Error() + { + var test = 1; + + Assert.Throws(() => ExpressionObserver.CreateFromExpression(test, o => (double)o)); + } + + [Fact] + public async Task As_Operator_Should_Be_Ignored() + { + NotifyingBase test = new Class1 { Foo = "Test" }; + + var target = ExpressionObserver.CreateFromExpression(test, o => (o as Class1).Foo); + + Assert.Equal("Test", await target.Take(1)); + + GC.KeepAlive(test); + } + + [Fact] + public async Task Avalonia_Property_Indexer_Reads_Avalonia_Property_Value() + { + var test = new Class2(); + + var target = ExpressionObserver.CreateFromExpression(test, o => o[Class2.FooProperty]); + + Assert.Equal("foo", await target.Take(1)); + + GC.KeepAlive(test); + } + + [Fact] + public async Task Complex_Expression_Correctly_Parsed() + { + var test = new Class1 { Foo = "Test" }; + + var target = ExpressionObserver.CreateFromExpression(test, o => o.Foo.Length); + + Assert.Equal(test.Foo.Length, await target.Take(1)); + + GC.KeepAlive(test); + } + private class Class1 : NotifyingBase { private string _foo; @@ -107,5 +178,14 @@ namespace Avalonia.Base.UnitTests.Data.Core } } } + + + private class Class2 : AvaloniaObject + { + public static readonly StyledProperty FooProperty = + AvaloniaProperty.Register("Foo", defaultValue: "foo"); + + public string ClrProperty { get; } = "clr-property"; + } } } From d2823110318865693625463f92eaf42c64ed98c4 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 31 May 2018 18:39:03 -0500 Subject: [PATCH 03/34] Fix bug in ExpressionVisitorNodeBuilder. --- .../Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs b/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs index 5affe227e1..433cfd1889 100644 --- a/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs +++ b/src/Avalonia.Base/Data/Core/Parsers/ExpressionVisitorNodeBuilder.cs @@ -65,7 +65,8 @@ namespace Avalonia.Data.Core.Parsers protected override Expression VisitIndex(IndexExpression node) { - var visited = base.VisitIndex(node); + Visit(node.Object); + if (node.Indexer == AvaloniaObjectIndexer) { var property = GetArgumentExpressionValue(node.Arguments[0]); @@ -76,7 +77,7 @@ namespace Avalonia.Data.Core.Parsers Nodes.Add(new IndexerExpressionNode(node)); } - return visited; + return node; } private T GetArgumentExpressionValue(Expression expr) @@ -158,7 +159,6 @@ namespace Avalonia.Data.Core.Parsers protected override Expression VisitMethodCall(MethodCallExpression node) { - base.VisitMethodCall(node); var property = TryGetPropertyFromMethod(node.Method); if (property != null) From 4f26d4fc0865c71a346f526dd13b300df546aa63 Mon Sep 17 00:00:00 2001 From: Dan Walmsley Date: Thu, 7 Jun 2018 15:32:57 +0100 Subject: [PATCH 04/34] dont highlight selected menuitems. --- src/Avalonia.Themes.Default/MenuItem.xaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Avalonia.Themes.Default/MenuItem.xaml b/src/Avalonia.Themes.Default/MenuItem.xaml index efb31175fa..319ff189f0 100644 --- a/src/Avalonia.Themes.Default/MenuItem.xaml +++ b/src/Avalonia.Themes.Default/MenuItem.xaml @@ -122,11 +122,6 @@ - -