Issue with setting up mock Objects, i.e. calling Mock<T>.Setup, when arguments have the same hashcodes

[pawelelert]

Hi,

I believe there is inconsistency in which Moq handles setting up matching call signatures.
Let consider following snippet of code:

Argument arg1 = new Argument("aaa");
Argument arg2 = new Argument("bbb");

var mocked = new Mock<IToBeMocked>(MockBehavior.Strict);
mocked.Setup(x => x.BlaBla(arg1));
mocked.Setup(x => x.BlaBla(arg2));
// This invocation of Setup override's the first invocation

mocked.Object.BlaBla(arg1);
// This calls fails with statement: invocation failed with mock behavior Strict.
// All invocations on the mock must have a corresponding setup

Expected behavior: mocked.Object.BlaBla(arg1) call does not throw an exception.

My clue: call to mocked.Setup(x => x.BlaBla(arg2))

• is invoking: Interceptor.AddCall method
• which in turn invokes Interceptor.calls.ContainsKey(key)
• this invokes ExpressionKey.Equal and ExpressionKey.getHashCode
  Then, ExpressionKey.getHashCode seems to be working fine, whether ExpressionKey.Equals seems to be extremely tolerant and generally always returns equality.

Finally, this introduce very awkward behavior where ability to match objects depends mainly at hashcodes and their potential collisions.

I would like to suggest fixing it, by changing ExpressionKey.Equal implementation to be as restrictive as gethashcode, i.e. for each different hashcode equals should return false. If you agree with such approach I could introduce such change by myself.

[pawelelert]

I am attaching test unit code that demonstrate this issue.
Furthermore, I think that this issue can be solved and Moq can be simplified by converting set of calls from dictionary to lists object and abandon comparing ExpressionKeys and deleting them just by adding the calls to the list and then choosing the last matching element, this will emulate current use experience that the last method will be picked first and can overwrite the first ones.
Moq issue.txt

[stakx]

Well spotted... thank you for the test case and your analysis.

ExpressionKey.Equals seems to be extremely tolerant and generally always returns equality

That's the cause for what you observed. The error is in these lines of code.

Issue #135 and PR #134 already cover the same ground. We'll probably see to those first, but then I think it might be good to follow up on your last suggestion:

Furthermore, I think that this issue can be solved and Moq can be simplified by converting set of calls from dictionary to lists object and abandon comparing ExpressionKeys and deleting them just by adding the calls to the list and then choosing the last matching element, this will emulate current use experience that the last method will be picked first and can overwrite the first ones.

Thank you once again for taking the time to report this! 👍