diff --git a/CHANGELOG.md b/CHANGELOG.md index f94f34ebc..b6ab533ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 * AmbiguousMatchException when setting up the property, that hides another one (@ishatalkin, #939) * `ArgumentException` ("Interface not found") when setting up `object.ToString` on an interface mock (@vslynko, #942) * Cannot "return" to original mocked type after downcasting with `Mock.Get` and then upcasting with `mock.As<>` (@pjquirk, #943) +* `params` arrays in recursive setup expressions are matched by reference equality instead of by structural equality (@danielcweber, #946) ## 4.13.0 (2019-08-31) diff --git a/src/Moq/InvocationShape.cs b/src/Moq/InvocationShape.cs index 14ecd775b..738160e9d 100644 --- a/src/Moq/InvocationShape.cs +++ b/src/Moq/InvocationShape.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -160,8 +161,34 @@ public bool Equals(InvocationShape other) other.partiallyEvaluatedArguments = PartiallyEvaluateArguments(other.Arguments); } - for (int i = 0, n = this.partiallyEvaluatedArguments.Length; i < n; ++i) + var lastParameter = this.Method.GetParameters().LastOrDefault(); + var lastParameterIsParamArray = lastParameter != null && lastParameter.ParameterType.IsArray && lastParameter.IsDefined(typeof(ParamArrayAttribute)); + + for (int i = 0, li = this.partiallyEvaluatedArguments.Length - 1; i <= li; ++i) { + // Special case for final `params` parameters, which need to be compared by structural equality, + // not array reference equality: + if (i == li && lastParameterIsParamArray) + { + // In the following, if we retrieved the `params` arrays via `partiallyEvaluatedArguments`, + // we might see them either as `NewArrayExpression`s or reduced to `ConstantExpression`s. + // By retrieving them via `Arguments` we always see them as non-reduced `NewArrayExpression`s, + // so we don't have to distinguish between two cases. (However, the expressions inside those + // have already been partially evaluated by `MatcherFactory` earlier on!) + if (this.Arguments[li] is NewArrayExpression e1 && other.Arguments[li] is NewArrayExpression e2 && e1.Expressions.Count == e2.Expressions.Count) + { + for (int j = 0, nj = e1.Expressions.Count; j < nj; ++j) + { + if (!ExpressionComparer.Default.Equals(e1.Expressions[j], e2.Expressions[j])) + { + return false; + } + } + + continue; + } + } + if (!ExpressionComparer.Default.Equals(this.partiallyEvaluatedArguments[i], other.partiallyEvaluatedArguments[i])) { return false; diff --git a/tests/Moq.Tests/InvocationShapeFixture.cs b/tests/Moq.Tests/InvocationShapeFixture.cs new file mode 100644 index 000000000..f90e4ce62 --- /dev/null +++ b/tests/Moq.Tests/InvocationShapeFixture.cs @@ -0,0 +1,75 @@ +// 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 System.Diagnostics; +using System.Linq; +using System.Linq.Expressions; + +using Xunit; + +namespace Moq.Tests +{ + public class InvocationShapeFixture + { + // This test is unspectacular but sets the stage for the one following it. See comment below. + [Fact] + public void Regular_parameters_are_compared_using_equality() + { + var fst = ToInvocationShape(a => a.Method(1, 2, 3)); + var snd = ToInvocationShape(a => a.Method(1, 2, 3)); + + Assert.NotSame(fst, snd); + Assert.Equal(fst, snd); + } + + // If you look at just this test code, and not at the definition for `B.Method`, + // you'd rightly expect the same outcome as for the above test, which looks almost the same. + // What we're testing here is that when the compiler silently transforms literal values to + // `params` arrays, we care about their structural equality, not about their differing identities. + [Fact] + public void Param_array_args_are_compared_using_structural_equality_not_reference_equality() + { + var fst = ToInvocationShape(b => b.Method(1, 2, 3)); + var snd = ToInvocationShape(b => b.Method(1, 2, 3)); + + Assert.NotSame(fst, snd); + Assert.Equal(fst, snd); + } + + [Fact] + public void Param_array_args_are_compared_partially_evaluated() + { + int x = 1; + + var fst = ToInvocationShape(b => b.Method(1, 2, 3)); + var snd = ToInvocationShape(b => b.Method(x, 2, 3)); + // ^ + // `x` will be captured and represented in the expression tree as a display class field access: + var xExpr = ((snd.Expression.Body as MethodCallExpression).Arguments.Last() as NewArrayExpression).Expressions.First(); + + Assert.False(xExpr is ConstantExpression); + Assert.NotSame(fst, snd); + Assert.Equal(fst, snd); + } + + private static InvocationShape ToInvocationShape(Expression> expression) + { + Debug.Assert(expression != null); + Debug.Assert(expression.Body is MethodCallExpression); + + var methodCall = (MethodCallExpression)expression.Body; + return new InvocationShape(expression, methodCall.Method, methodCall.Arguments); + } + + public interface A + { + void Method(int arg1, int arg2, int arg3); + } + + public interface B + { + void Method(params int[] args); + } + } +} diff --git a/tests/Moq.Tests/RecursiveMocksFixture.cs b/tests/Moq.Tests/RecursiveMocksFixture.cs index 383ee53fc..0e743c574 100644 --- a/tests/Moq.Tests/RecursiveMocksFixture.cs +++ b/tests/Moq.Tests/RecursiveMocksFixture.cs @@ -315,6 +315,16 @@ public void FullMethodInvocationInsideFluentCanUseMatchers() Assert.Equal(5, fooMock.Object.Bar.GetBaz("foo").Value); } + [Fact] + public void Param_array_args_in_setup_expression_parts_are_compared_by_structural_equality_not_reference_equality() + { + var mock = new Mock(); + mock.Setup(m => m.GetBar(1).Value).Returns(1); + mock.Setup(m => m.GetBar(1).OtherValue).Returns(2); + Assert.Equal(1, mock.Object.GetBar(1).Value); + Assert.Equal(2, mock.Object.GetBar(1).OtherValue); + } + public class Verify_can_tell_apart_different_arguments_in_intermediate_part_of_fluent_expressions { [Fact] @@ -418,6 +428,7 @@ public class Foo : IFoo public IBar BarField; public IBar Bar { get; set; } public IBar GetBar() { return null; } + public IBar GetBar(params int[] indices) { return null; } public IBar this[int index] { get { return null; } set { } } public string Do(string command) @@ -432,11 +443,13 @@ public interface IFoo IBar this[int index] { get; set; } string Do(string command); IBar GetBar(); + IBar GetBar(params int[] indices); } public interface IBar { int Value { get; set; } + int OtherValue { get; set; } string Do(string command); IBaz Baz { get; set; } IBaz GetBaz(string value);