Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Expression is not an event add" when using .Raises() #1175

Closed
howcheng opened this issue Jun 22, 2021 · 6 comments · Fixed by #1276
Closed

"Expression is not an event add" when using .Raises() #1175

howcheng opened this issue Jun 22, 2021 · 6 comments · Fixed by #1276

Comments

@howcheng
Copy link

When trying to have a mocked method raise an event, I'm getting the error message "Expression is not an event add". The interface I'm using is a mess of inheritance and new versions of events, so maybe that's related, but here's a fiddle with a stripped-down version where you can see the error happening.

https://dotnetfiddle.net/kqhOln

Did I do something wrong, or is this a bug?

Thanks.

@tonyhallett
Copy link
Contributor

When DoSomething is called the corresponding
MethodCall ExecuteCore will Execute the RaiseEvent Behaviour
which calls Mock.RaiseEvent

https://github.com/moq/moq4/blob/a6fde8b6d79a7437bf642d115785b97f40779b6a/src/Moq/Mock.cs#L688


		internal static void RaiseEvent(Mock mock, LambdaExpression expression, Stack<InvocationShape> parts, object[] arguments)
		{
			const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly;

			var part = parts.Pop();
			var method = part.Method;

			if (parts.Count == 0)
			{
				EventInfo @event;
				if (method.IsEventAddAccessor())
				{
					var implementingMethod = method.GetImplementingMethod(mock.Object.GetType());
					@event = implementingMethod.DeclaringType.GetEvents(bindingFlags).SingleOrDefault(e => e.GetAddMethod(true) == implementingMethod);
					if (@event == null) //************************************
					{
						throw new ArgumentException(
							string.Format(
								CultureInfo.CurrentCulture,
								Resources.SetupNotEventAdd,
								part.Expression));
					}
				}

There is similar code hit from SetupAdd which should add the handler to be called in Mock.RaiseEvent

https://github.com/moq/moq4/blob/a6fde8b6d79a7437bf642d115785b97f40779b6a/src/Moq/Interception/InterceptionAspects.cs#L133

	internal static class HandleEventSubscription
	{
		public static bool Handle(Invocation invocation, Mock mock)
		{
			const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly;

			var methodName = invocation.Method.Name;

			// Special case for event accessors. The following, seemingly random character checks are guards against
			// more expensive checks (for the common case where the invoked method is *not* an event accessor).
			if (methodName.Length > 4)
			{
				if (methodName[0] == 'a' && methodName[3] == '_' && invocation.Method.IsEventAddAccessor())
				{
					var implementingMethod = invocation.Method.GetImplementingMethod(invocation.ProxyType);
					var @event = implementingMethod.DeclaringType.GetEvents(bindingFlags).SingleOrDefault(e => e.GetAddMethod(true) == implementingMethod);
// ********************************************************** event is null 
					if (@event != null)
					{
						if (mock.CallBase && !invocation.Method.IsAbstract)
						{
							invocation.ReturnValue = invocation.CallBase();
							return true;
						}
						else if (invocation.Arguments.Length > 0 && invocation.Arguments[0] is Delegate delegateInstance)
						{
//********* this is what expect to occur
							mock.EventHandlers.Add(@event, delegateInstance);
							return true;
						}
					}
				}

if we look at the events on the proxy ( note that I have pasted your code into Moq tests )
implementingMethod.DeclaringType.GetEvents(bindingFlags)

MessageReceived
System.EventHandler`1[[Moq.Tests.EventHandlersFixture+ItemEventArgs`1[[System.Object, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], Moq.Tests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=69f491c39445e920]]

IReadableQueue`1[Object].MessageReceived
System.EventHandler`1[[Moq.Tests.EventHandlersFixture+ItemEventArgs`1[[System.Object, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], Moq.Tests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=69f491c39445e920]]

Which disagrees with the single event declared on the ISqsQueueClient<object> type

MessageReceived
System.EventHandler`1[[Moq.Tests.EventHandlersFixture+SqsMessageEventArgs`1[[System.Object, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]], Moq.Tests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=69f491c39445e920]]

I agree that it is a mess of inheritance and new versions of events but it still looks like a bug to me ( in Castle.Core.)

@stakx Your thoughts

@tonyhallett
Copy link
Contributor

All 3 events have accessors on the proxy

image

@stakx
Copy link
Contributor

stakx commented Jun 28, 2021

@howcheng, thanks for reporting this issue. And thanks @tonyhallett for doing some preliminary investigating.

First, to simplify things a little, let's use a more minimal repro (derived from the original one above mostly through a process of elimination):

using System;
using Moq;

Mock<IDerived> mock = new Mock<IDerived>();
mock.Setup(x => x.RaiseEvent()).Raises(x => x.Event += null, false);
mock.Object.Event += _ => { Console.WriteLine("Hello world!!!"); };
mock.Object.RaiseEvent();

public interface IBase
{
    event Action Event;
    void RaiseEvent();
}

public interface IDerived : IBase
{
    new event Action<bool> Event;
}

I've played around with this a bit, and the problem appears to be triggered by having two events of the same name but different signature, spread across two types which are related via subtyping.

I've taken a closer look at what's going on under the hood. Among other things, I've used a modified version of Moq that stores DynamicProxy's dynamic assembly to disk, then I inspected that assembly using ildasm / ilspy. It turns out that DynamicProxy generates a proxy class having two events with an identical name:

.event [mscorlib]System.Action Event
{
    .addon instance void Castle.Proxies.IDerivedProxy::add_Event(class [mscorlib]System.Action)
    .removeon instance void Castle.Proxies.IDerivedProxy::remove_Event(class [mscorlib]System.Action)
}

.event class [mscorlib]System.Action`1<bool> Event
{
    .addon instance void Castle.Proxies.IDerivedProxy::add_Event(class [mscorlib]System.Action`1<bool>)
    .removeon instance void Castle.Proxies.IDerivedProxy::remove_Event(class [mscorlib]System.Action`1<bool>)
}

This kind of overloading is illegal, as peverify will confirm:

[MD]: Error: Event has a duplicate (token=0x14000002). [token:0x14000001]
[MD]: Error: Event has a duplicate (token=0x14000001). [token:0x14000002]
2 Error(s) Verifying CastleDynProxy2.dll

This event duplication then causes Reflection to report incorrect results, making the following .SingleOrDefault query in Moq fail:

https://github.com/moq/moq4/blob/a6fde8b6d79a7437bf642d115785b97f40779b6a/src/Moq/Mock.cs#L687

... which in turn causes the error message initially reported.

We'll likely need to get this fixed in DynamicProxy first before we can do anything in Moq (if any change will be necessary after patching DynamicProxy).

@toppiovi
Copy link

Here is another instance of Raise() failing when the base class event is hidden by a new generic child class event.
Interestingly, this issue was not present in Moq 4.5.3, but started occurring when updating to Moq 4.16.1
I hope providing this small example helps!

    public interface IEvents
    {
        event EventHandler Created;
    }

    public interface IGenericHidingEvents<T> : IEvents
    {
        new event EventHandler<T> Created;
    }


    public class GenericHidingEventConsumer
    {
        private IGenericHidingEvents<bool> myHidingEvents;

        public GenericHidingEventConsumer(IGenericHidingEvents<bool> theHidingEvents)
        {
            this.myHidingEvents = theHidingEvents;
            this.myHidingEvents.Created += this.HidingEventsOnCreated;
        }

        private void HidingEventsOnCreated(object theSender, bool theE)
        {
            EventHandled = true;
        }

        public bool EventHandled { get; set; }
    }

    [TestClass]
    public class GenericHidingEventsConsumerTests
    {
        [TestMethod]
        public void created_event_is_handled()
        {
            var aEventsMock = new Mock<IGenericHidingEvents<bool>>();
            var aConsumer = new GenericHidingEventConsumer(aEventsMock.Object);

            aEventsMock.Raise(theO => theO.Created += null, true);

//     Test method playground.GenericHidingEventsConsumerTests.created_event_is_handled threw exception: 
//     System.ArgumentException: Expression is not an event add: theO => theO.add_Created(null)_

            Assert.IsTrue(aConsumer.EventHandled); // Fails
        }
    }

@stakx
Copy link
Contributor

stakx commented Aug 2, 2022

@toppiovi, FYI, your test case has a slight problem that would prevent it from passing even if there were no issue with Moq:

-aEventsMock.Raise(theO => theO.Created += null, true);
+aEventsMock.Raise(theO => theO.Created += null, /* sender */, true);

I'm going to add a corrected version of your test case to the regression suite.

@stakx
Copy link
Contributor

stakx commented Aug 2, 2022

I've just pushed Moq version 4.18.2 to NuGet, that version includes a bugfix for this issue. Thanks again for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants