From 81168ac09433147f8fe055bf7f8c618ea88d3fde Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 2 Oct 2017 21:55:26 -0500 Subject: [PATCH] Add error handling for methods with too many parameters. Add support for static methods. --- .../AlwaysEnabledDelegateCommand.cs | 5 +-- .../Data/Plugins/MethodAccessorPlugin.cs | 41 ++++++++++--------- .../Data/ExpressionObserverTests_Method.cs | 22 ++++++++++ 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/Markup/Avalonia.Markup/AlwaysEnabledDelegateCommand.cs b/src/Markup/Avalonia.Markup/AlwaysEnabledDelegateCommand.cs index c675b876ae..1d417a55a6 100644 --- a/src/Markup/Avalonia.Markup/AlwaysEnabledDelegateCommand.cs +++ b/src/Markup/Avalonia.Markup/AlwaysEnabledDelegateCommand.cs @@ -16,10 +16,7 @@ namespace Avalonia.Markup public event EventHandler CanExecuteChanged; - public bool CanExecute(object parameter) - { - return true; - } + public bool CanExecute(object parameter) => true; public void Execute(object parameter) { diff --git a/src/Markup/Avalonia.Markup/Data/Plugins/MethodAccessorPlugin.cs b/src/Markup/Avalonia.Markup/Data/Plugins/MethodAccessorPlugin.cs index ef4281c314..c602fa4f4d 100644 --- a/src/Markup/Avalonia.Markup/Data/Plugins/MethodAccessorPlugin.cs +++ b/src/Markup/Avalonia.Markup/Data/Plugins/MethodAccessorPlugin.cs @@ -9,24 +9,30 @@ namespace Avalonia.Markup.Data.Plugins { class MethodAccessorPlugin : IPropertyAccessorPlugin { - public bool Match(object obj, string propertyName) - => obj.GetType().GetRuntimeMethods().FirstOrDefault(x => x.Name == propertyName) != null; + public bool Match(object obj, string methodName) + => obj.GetType().GetRuntimeMethods().Any(x => x.Name == methodName); - public IPropertyAccessor Start(WeakReference reference, string propertyName) + public IPropertyAccessor Start(WeakReference reference, string methodName) { Contract.Requires(reference != null); - Contract.Requires(propertyName != null); + Contract.Requires(methodName != null); var instance = reference.Target; - var method = instance.GetType().GetRuntimeMethods().FirstOrDefault(x => x.Name == propertyName); + var method = instance.GetType().GetRuntimeMethods().FirstOrDefault(x => x.Name == methodName); if (method != null) { + if (method.GetParameters().Length + (method.ReturnType == typeof(void) ? 0 : 1) > 8) + { + var exception = new ArgumentException("Cannot create a binding accessor for a method with more than 8 parameters or more than 7 parameters if it has a non-void return type.", nameof(method)); + return new PropertyError(new BindingNotification(exception, BindingErrorType.Error)); + } + return new Accessor(reference, method); } else { - var message = $"Could not find CLR method '{propertyName}' on '{instance}'"; + var message = $"Could not find CLR method '{methodName}' on '{instance}'"; var exception = new MissingMemberException(message); return new PropertyError(new BindingNotification(exception, BindingErrorType.Error)); } @@ -41,9 +47,7 @@ namespace Avalonia.Markup.Data.Plugins var paramTypes = method.GetParameters().Select(param => param.ParameterType).ToArray(); var returnType = method.ReturnType; - - // TODO: Throw exception if more than 8 parameters or more than 7 + return type. - // Do this here or in the caller? Here probably + if (returnType == typeof(void)) { if (paramTypes.Length == 0) @@ -60,10 +64,8 @@ namespace Avalonia.Markup.Data.Plugins var genericTypeParameters = paramTypes.Concat(new[] { returnType }).ToArray(); PropertyType = Type.GetType($"System.Func`{genericTypeParameters.Length}").MakeGenericType(genericTypeParameters); } - - // TODO: Is this going to leak? - // TODO: Static methods? - Value = method.CreateDelegate(PropertyType, reference.Target); + + Value = method.IsStatic ? method.CreateDelegate(PropertyType) : method.CreateDelegate(PropertyType, reference.Target); } public override Type PropertyType { get; } @@ -73,19 +75,18 @@ namespace Avalonia.Markup.Data.Plugins public override bool SetValue(object value, BindingPriority priority) => false; protected override void SubscribeCore(IObserver observer) - { - SendCurrentValue(); - } - - private void SendCurrentValue() { try { - var value = Value; - Observer.OnNext(value); + Observer.OnNext(Value); } catch { } } + + private void SendCurrentValue() + { + + } } } } diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Method.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Method.cs index 9929e4e986..439e9dc542 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Method.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionObserverTests_Method.cs @@ -19,6 +19,11 @@ namespace Avalonia.Markup.UnitTests.Data public int MethodWithReturn() => 0; public int MethodWithReturnAndParameters(int i) => i; + + public static void StaticMethod() { } + + public static void TooManyParameters(int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8, int a9) { } + public static int TooManyParametersWithReturnType(int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8) => 1; } [Fact] @@ -37,6 +42,7 @@ namespace Avalonia.Markup.UnitTests.Data [InlineData(nameof(TestObject.MethodWithoutReturn), typeof(Action))] [InlineData(nameof(TestObject.MethodWithReturn), typeof(Func))] [InlineData(nameof(TestObject.MethodWithReturnAndParameters), typeof(Func))] + [InlineData(nameof(TestObject.StaticMethod), typeof(Action))] public async Task Should_Get_Method_WithCorrectDelegateType(string methodName, Type expectedType) { var data = new TestObject(); @@ -61,5 +67,21 @@ namespace Avalonia.Markup.UnitTests.Data GC.KeepAlive(data); } + + [Theory] + [InlineData(nameof(TestObject.TooManyParameters))] + [InlineData(nameof(TestObject.TooManyParametersWithReturnType))] + public async Task Should_Return_Error_Notification_If_Too_Many_Parameters(string methodName) + { + var data = new TestObject(); + var observer = new ExpressionObserver(data, methodName); + var result = await observer.Take(1); + + Assert.IsType(result); + + Assert.Equal(BindingErrorType.Error, ((BindingNotification)result).ErrorType); + + GC.KeepAlive(data); + } } }