Merge pull request #891 from stakx/eventhandlercollection

Make mock event subscription equally strict as regular event subscription

CHANGELOG.md

@@ -15,6 +15,8 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 * Consistent `Callback` delegate validation regardless of whether or not `Callback` is preceded by a `Returns`: Validation for post-`Returns` callback delegates used to be very relaxed, but is now equally strict as in the pre-`Returns` case.) (@stakx, #876)
 
+* Subscription to mocked events used to be handled less strictly than subscription to regular CLI events. As with the latter, subscribing to mocked events now also requires all handlers to have the same delegate type. (@stakx, #891)
+
 #### Added
 
 * Added support for setup and verification of the event handlers through `Setup[Add|Remove]` and `Verify[Add|Remove|All]` (@lepijohnny, #825) 

src/Moq/EventHandlerCollection.cs

@@ -3,33 +3,23 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
 
 namespace Moq
 {
 	internal sealed class EventHandlerCollection
 	{
-		private Dictionary<string, List<Delegate>> eventHandlers;
+		private readonly Dictionary<string, Delegate> eventHandlers;
 
 		public EventHandlerCollection()
 		{
-			this.eventHandlers = new Dictionary<string, List<Delegate>>();
+			this.eventHandlers = new Dictionary<string, Delegate>();
 		}
 
 		public void Add(string eventName, Delegate eventHandler)
 		{
 			lock (this.eventHandlers)
 			{
-				List<Delegate> handlers;
-				if (!this.eventHandlers.TryGetValue(eventName, out handlers))
-				{
-					handlers = new List<Delegate>();
-					this.eventHandlers.Add(eventName, handlers);
-				}
-
-				handlers.Add(eventHandler);
+				this.eventHandlers[eventName] = Delegate.Combine(this.TryGet(eventName), eventHandler);
 			}
 		}
 
@@ -45,26 +35,21 @@ public void Remove(string eventName, Delegate eventHandler)
 		{
 			lock (this.eventHandlers)
 			{
-				List<Delegate> handlers;
-				if (this.eventHandlers.TryGetValue(eventName, out handlers))
-				{
-					handlers.Remove(eventHandler);
-				}
+				this.eventHandlers[eventName] = Delegate.Remove(this.TryGet(eventName), eventHandler);
 			}
 		}
 
-		public Delegate[] ToArray(string eventName)
+		public bool TryGet(string eventName, out Delegate handlers)
 		{
 			lock (this.eventHandlers)
 			{
-				List<Delegate> handlers;
-				if (!this.eventHandlers.TryGetValue(eventName, out handlers))
-				{
-					return new Delegate[0];
-				}
-
-				return handlers.ToArray();
+				return this.eventHandlers.TryGetValue(eventName, out handlers) && handlers != null;
 			}
 		}
+
+		private Delegate TryGet(string eventName)
+		{
+			return this.eventHandlers.TryGetValue(eventName, out var handlers) ? handlers : null;
+		}
 	}
 }

src/Moq/Mock.cs

@@ -635,11 +635,10 @@ internal static void RaiseEvent(Mock mock, LambdaExpression expression, Stack<In
 							expression));
 				}
 
-				foreach (var eventHandler in mock.EventHandlers.ToArray(eventName))
+				if (mock.EventHandlers.TryGet(eventName, out var handlers))
 				{
-					eventHandler.InvokePreserveStack(arguments);
+					handlers.InvokePreserveStack(arguments);
 				}
-
 			}
 			else if (mock.Setups.GetInnerMockSetups().TryFind(part, out var innerMockSetup) && innerMockSetup.ReturnsInnerMock(out var innerMock))
 			{

tests/Moq.Tests/EventHandlerTypesMustMatchFixture.cs

@@ -0,0 +1,71 @@
+// Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD.
+// All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
+
+using System;
+
+using Xunit;
+
+namespace Moq.Tests
+{
+	public class EventHandlerTypesMustMatchFixture
+	{
+		[Fact]
+		public void CLI_requires_event_handlers_to_have_the_exact_same_type()
+		{
+			var mouse = new Mouse();
+			var result = 2;
+
+			mouse.LeftButtonClicked += new Action<LeftButton>(_ => result += 3);
+			mouse.LeftButtonClicked += new Action<LeftButton>(_ => result *= 4);
+			mouse.RaiseLeftButtonClicked(new LeftButton());
+
+			Assert.Equal(20, result);
+		}
+
+		[Fact]
+		public void CLI_throws_if_event_handlers_do_not_have_the_exact_same_type()
+		{
+			var mouse = new Mouse();
+			mouse.LeftButtonClicked += new Action<Button>(delegate { });
+			Assert.Throws<ArgumentException>(() => mouse.LeftButtonClicked += new Action<LeftButton>(delegate { }));
+		}
+
+		[Fact]
+		public void Moq_requires_event_handlers_to_have_the_exact_same_type()
+		{
+			var mouseMock = new Mock<Mouse>();
+			var mouse = mouseMock.Object;
+			var result = 2;
+
+			mouse.LeftButtonClicked += new Action<Button>(_ => result += 3);
+			mouse.LeftButtonClicked += new Action<Button>(_ => result *= 4);
+			mouseMock.Raise(m => m.LeftButtonClicked += null, new LeftButton());
+
+			Assert.Equal(20, result);
+		}
+
+		[Fact]
+		public void Moq_throws_if_event_handlers_do_not_have_the_exact_same_type()
+		{
+			var mouseMock = new Mock<Mouse>();
+			var mouse = mouseMock.Object;
+
+			mouse.LeftButtonClicked += new Action<Button>(delegate { });
+			Assert.Throws<ArgumentException>(() => mouse.LeftButtonClicked += new Action<LeftButton>(delegate { }));
+		}
+
+		public class Mouse
+		{
+			public virtual event Action<LeftButton> LeftButtonClicked;
+
+			public void RaiseLeftButtonClicked(LeftButton button)
+			{
+				this.LeftButtonClicked?.Invoke(button);
+			}
+		}
+
+		public abstract class Button { }
+
+		public class LeftButton : Button { }
+	}
+}