From 22e72b63edac6ccd1fa818b1f450e62c43f6cc06 Mon Sep 17 00:00:00 2001
From: Kent Boogaart <kent.boogaart@gmail.com>
Date: Sun, 18 Sep 2016 18:51:16 +0930
Subject: [PATCH] emove BindCommand implementation taking Func<TParam>.

This commit fixes #1081 by removing the implementation of BindCommand that
takes a Func<TParam>. This implementation has never worked - the command
parameter was never passed into the IObservable<object> that the binding
infrastructure uses to resolve the command parameter. Moreover, it never
could work in the fashion advertised. Since it is just a function, there
is no way of knowing when to re-evaluate it. Thus, the observable passed
into the binding infrastructure can only evaluate the function once, which
makes the entire overload pointless.
---
 src/ReactiveUI.Tests/CommandBindingTests.cs |  66 ++++++++++--
 src/ReactiveUI/CommandBinding.cs            | 114 ++++++++------------
 2 files changed, 103 insertions(+), 77 deletions(-)

diff --git a/src/ReactiveUI.Tests/CommandBindingTests.cs b/src/ReactiveUI.Tests/CommandBindingTests.cs
index b94d5b243c..b4f32d1256 100755
--- a/src/ReactiveUI.Tests/CommandBindingTests.cs
+++ b/src/ReactiveUI.Tests/CommandBindingTests.cs
@@ -19,7 +19,7 @@ public FakeViewModel()
             Cmd = ReactiveCommand.Create(() => { });
         }
     }
- 
+
     public class FakeView : IViewFor<FakeViewModel>
     {
         public TextBox TheTextBox { get; protected set; }
@@ -138,6 +138,13 @@ public CommandBindViewModel()
         }
 
         public FakeNestedViewModel NestedViewModel { get; set; }
+
+        private int value;
+        public int Value
+        {
+            get { return value; }
+            set { this.RaiseAndSetIfChanged(ref this.value, value); }
+        }
     }
 
     public class FakeNestedViewModel : ReactiveObject
@@ -150,23 +157,30 @@ public FakeNestedViewModel()
         public ReactiveCommand NestedCommand { get; protected set; }
     }
 
+    public class CustomClickButton : Button
+    {
+        public event EventHandler<EventArgs> CustomClick;
+
+        public void RaiseCustomClick() =>
+            this.CustomClick?.Invoke(this, EventArgs.Empty);
+    }
 
     public class CommandBindView : IViewFor<CommandBindViewModel>
     {
-        object IViewFor.ViewModel { 
+        object IViewFor.ViewModel {
             get { return ViewModel; }
-            set { ViewModel = (CommandBindViewModel)value; } 
+            set { ViewModel = (CommandBindViewModel)value; }
         }
 
         public CommandBindViewModel ViewModel { get; set; }
 
-        public Button Command1 { get; protected set; }
+        public CustomClickButton Command1 { get; protected set; }
 
         public Image Command2 { get; protected set; }
 
         public CommandBindView()
         {
-            Command1 = new Button();
+            Command1 = new CustomClickButton();
             Command2 = new Image();
         }
     }
@@ -191,7 +205,7 @@ public void CommandBindByNameWireup()
             disp.Dispose();
             Assert.Null(view.Command1.Command);
         }
-        
+
         [Fact]
         public void CommandBindNestedCommandWireup()
         {
@@ -300,5 +314,45 @@ public void CommandBindToExplicitEventWireup()
             Assert.Equal(1, invokeCount);
         }
 #endif
+
+        [Fact]
+        public void CommandBindWithParameterExpression()
+        {
+            var vm = new CommandBindViewModel();
+            var view = new CommandBindView() { ViewModel = vm };
+
+            var received = 0;
+            var cmd = ReactiveCommand.Create<int>(i => { received = i; });
+            vm.Command1 = cmd;
+
+            var disp = view.BindCommand(vm, x => x.Command1, x => x.Command1, x => x.Value, nameof(CustomClickButton.CustomClick));
+
+            vm.Value = 42;
+            view.Command1.RaiseCustomClick();
+            Assert.Equal(42, received);
+
+            vm.Value = 13;
+            view.Command1.RaiseCustomClick();
+            Assert.Equal(13, received);
+        }
+
+        [Fact]
+        public void CommandBindWithParameterObservable()
+        {
+            var vm = new CommandBindViewModel();
+            var view = new CommandBindView() { ViewModel = vm };
+
+            var received = 0;
+            var cmd = ReactiveCommand.Create<int>(i => { received = i; });
+            vm.Command1 = cmd;
+
+            var value = Observable.Return(42);
+            var disp = view.BindCommand(vm, x => x.Command1, x => x.Command1, value, nameof(CustomClickButton.CustomClick));
+
+            vm.Value = 42;
+            view.Command1.RaiseCustomClick();
+
+            Assert.Equal(42, received);
+        }
     }
 }
diff --git a/src/ReactiveUI/CommandBinding.cs b/src/ReactiveUI/CommandBinding.cs
index e97205e4d6..f393a899ec 100755
--- a/src/ReactiveUI/CommandBinding.cs
+++ b/src/ReactiveUI/CommandBinding.cs
@@ -17,37 +17,9 @@ static CommandBinder()
         {
             RxApp.EnsureInitialized();
 
-            binderImplementation = Locator.Current.GetService<ICommandBinderImplementation>() ?? 
+            binderImplementation = Locator.Current.GetService<ICommandBinderImplementation>() ??
                 new CommandBinderImplementation();
         }
-                
-        /// <summary>
-        /// Bind a command from the ViewModel to an explicitly specified control
-        /// on the View.
-        /// </summary>
-        /// <returns>A class representing the binding. Dispose it to disconnect
-        /// the binding</returns>
-        /// <param name="view">The View</param>
-        /// <param name="viewModel">The View model</param>
-        /// <param name="controlName">The name of the control on the view</param>
-        /// <param name="propertyName">The ViewModel command to Bind.</param>
-        /// <param name="withParameter">The ViewModel property to pass as the 
-        /// param of the ICommand</param>
-        /// <param name="toEvent">If specified, bind to the specific event 
-        /// instead of the default.</param>
-        public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                this TView view, 
-                TViewModel viewModel, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
-                Expression<Func<TView, TControl>> controlName,
-                Func<TParam> withParameter,
-                string toEvent = null)
-            where TViewModel : class
-            where TView : class, IViewFor<TViewModel>
-            where TProp : ICommand
-        {
-            return binderImplementation.BindCommand(viewModel, view, propertyName, controlName, withParameter, toEvent);
-        }
 
         /// <summary>
         /// Bind a command from the ViewModel to an explicitly specified control
@@ -59,14 +31,14 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
         /// <param name="viewModel">The View model</param>
         /// <param name="controlName">The name of the control on the view</param>
         /// <param name="propertyName">The ViewModel command to Bind.</param>
-        /// <param name="withParameter">The ViewModel property to pass as the 
+        /// <param name="withParameter">The ViewModel property to pass as the
         /// param of the ICommand</param>
-        /// <param name="toEvent">If specified, bind to the specific event 
+        /// <param name="toEvent">If specified, bind to the specific event
         /// instead of the default.</param>
         public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                this TView view, 
-                TViewModel viewModel, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                this TView view,
+                TViewModel viewModel,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 IObservable<TParam> withParameter,
                 string toEvent = null)
@@ -86,12 +58,12 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
         /// <param name="view">The View</param>
         /// <param name="viewModel">The View model</param>
         /// <param name="controlName">The name of the control on the view</param>
-        /// <param name="toEvent">If specified, bind to the specific event 
+        /// <param name="toEvent">If specified, bind to the specific event
         /// instead of the default.</param>
         public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl>(
-                this TView view, 
-                TViewModel viewModel, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                this TView view,
+                TViewModel viewModel,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 string toEvent = null)
             where TViewModel : class
@@ -100,7 +72,7 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
         {
             return binderImplementation.BindCommand(viewModel, view, propertyName, controlName, toEvent);
         }
-                
+
         /// <summary>
         /// Bind a command from the ViewModel to an explicitly specified control
         /// on the View.
@@ -110,14 +82,14 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
         /// <param name="view">The View</param>
         /// <param name="viewModel">The View model</param>
         /// <param name="controlName">The name of the control on the view</param>
-        /// <param name="withParameter">The ViewModel property to pass as the 
+        /// <param name="withParameter">The ViewModel property to pass as the
         /// param of the ICommand</param>
-        /// <param name="toEvent">If specified, bind to the specific event 
+        /// <param name="toEvent">If specified, bind to the specific event
         /// instead of the default.</param>
         public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                this TView view, 
-                TViewModel viewModel, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                this TView view,
+                TViewModel viewModel,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 Expression<Func<TViewModel, TParam>> withParameter,
                 string toEvent = null)
@@ -132,9 +104,9 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
     interface ICommandBinderImplementation : IEnableLogger
     {
         IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 Func<TParam> withParameter,
                 string toEvent = null)
@@ -143,9 +115,9 @@ IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp,
             where TProp : ICommand;
 
         IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 IObservable<TParam> withParameter,
                 string toEvent = null)
@@ -154,12 +126,12 @@ IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp,
             where TProp : ICommand;
     }
 
-    public class CommandBinderImplementation : ICommandBinderImplementation 
+    public class CommandBinderImplementation : ICommandBinderImplementation
     {
         public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> vmProperty, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> vmProperty,
                 Expression<Func<TView, TControl>> controlProperty,
                 Func<TParam> withParameter,
                 string toEvent = null)
@@ -171,11 +143,11 @@ public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel,
             var controlExpression = Reflection.Rewrite(controlProperty.Body);
             var source = Reflection.ViewModelWhenAnyValue(viewModel, view, vmExpression).Cast<TProp>();
 
-            IDisposable bindingDisposable = bindCommandInternal(source, view, controlExpression, Observable.Empty<object>(), toEvent, cmd => {
+            IDisposable bindingDisposable = bindCommandInternal(source, view, controlExpression, Observable.Defer(() => Observable.Return(withParameter())), toEvent, cmd => {
                 var rc = cmd as ReactiveCommand;
                 if (rc == null) {
                     return new RelayCommand(cmd.CanExecute, _ => cmd.Execute(withParameter()));
-                } 
+                }
 
                 var ret = ReactiveCommand.Create(() => ((ICommand)rc).Execute(null), rc.CanExecute);
                 return ret;
@@ -186,9 +158,9 @@ public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel,
         }
 
         public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> vmProperty, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> vmProperty,
                 Expression<Func<TView, TControl>> controlProperty,
                 IObservable<TParam> withParameter,
                 string toEvent = null)
@@ -202,14 +174,14 @@ public IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel,
 
             IDisposable bindingDisposable = bindCommandInternal(source, view, controlExpression, withParameter, toEvent);
 
-            return new ReactiveBinding<TView, TViewModel, TProp>(view, viewModel, controlExpression, vmExpression, 
+            return new ReactiveBinding<TView, TViewModel, TProp>(view, viewModel, controlExpression, vmExpression,
                 source, BindingDirection.OneWay, bindingDisposable);
         }
 
         IDisposable bindCommandInternal<TView, TProp, TParam>(
                 IObservable<TProp> This,
-                TView view, 
-                Expression controlExpression, 
+                TView view,
+                Expression controlExpression,
                 IObservable<TParam> withParameter,
                 string toEvent,
                 Func<ICommand, ICommand> commandFixuper = null)
@@ -242,7 +214,7 @@ IDisposable bindCommandInternal<TView, TProp, TParam>(
             return Disposable.Create(() => {
                 propSub.Dispose();
                 disp.Dispose();
-            });                      
+            });
         }
     }
 
@@ -250,9 +222,9 @@ static class CommandBinderImplementationMixins
     {
         public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl>(
                 this ICommandBinderImplementation This,
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 string toEvent = null)
             where TViewModel : class
@@ -264,9 +236,9 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
 
         public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TViewModel, TProp, TControl, TParam>(
                 this ICommandBinderImplementation This,
-                TViewModel viewModel, 
-                TView view, 
-                Expression<Func<TViewModel, TProp>> propertyName, 
+                TViewModel viewModel,
+                TView view,
+                Expression<Func<TViewModel, TProp>> propertyName,
                 Expression<Func<TView, TControl>> controlName,
                 Expression<Func<TViewModel, TParam>> withParameter,
                 string toEvent = null)
@@ -280,7 +252,7 @@ public static IReactiveBinding<TView, TViewModel, TProp> BindCommand<TView, TVie
 
     class CreatesCommandBinding
     {
-        static readonly MemoizingMRUCache<Type, ICreatesCommandBinding> bindCommandCache = 
+        static readonly MemoizingMRUCache<Type, ICreatesCommandBinding> bindCommandCache =
             new MemoizingMRUCache<Type, ICreatesCommandBinding>((t, _) => {
                 return Locator.Current.GetServices<ICreatesCommandBinding>()
                     .Aggregate(Tuple.Create(0, (ICreatesCommandBinding)null), (acc, x) => {
@@ -289,7 +261,7 @@ class CreatesCommandBinding
                     }).Item2;
             }, RxApp.SmallCacheLimit);
 
-        static readonly MemoizingMRUCache<Type, ICreatesCommandBinding> bindCommandEventCache = 
+        static readonly MemoizingMRUCache<Type, ICreatesCommandBinding> bindCommandEventCache =
             new MemoizingMRUCache<Type, ICreatesCommandBinding>((t, _) => {
                 return Locator.Current.GetServices<ICreatesCommandBinding>()
                     .Aggregate(Tuple.Create(0, (ICreatesCommandBinding)null), (acc, x) => {