Merge pull request #783 from stakx/issue-110

Manually set up inner mocks should be discoverable by Moq

CHANGELOG.md

@@ -36,6 +36,8 @@ This release contains several **minor breaking changes**. Please review your cod
 * `VerifySet` fails on non-trivial property setup (@TimothyHayes, #430)
 * Use of `SetupSet` 'forgets' method setup (@TimothyHayes, #432)
 * Recursive mocks don't work with argument matching (@thalesmello, #142)
+* Recursive property setup overrides previous setups (@jamesfoster, #110)
+
 
 ## 4.10.1 (2018-12-03)
 

src/Moq/AutoImplementedPropertyGetterSetup.cs

@@ -10,7 +10,7 @@ namespace Moq
 	/// <summary>
 	///   Setup used by <see cref="Mock.SetupAllProperties(Mock)"/> for property getters.
 	/// </summary>
-	internal sealed class AutoImplementedPropertyGetterSetup : Setup, IDeterministicReturnValueSetup
+	internal sealed class AutoImplementedPropertyGetterSetup : Setup
 	{
 		private static Expression[] noArguments = new Expression[0];
 
@@ -22,13 +22,17 @@ public AutoImplementedPropertyGetterSetup(LambdaExpression originalExpression, M
 			this.getter = getter;
 		}
 
-		public object ReturnValue => this.getter.Invoke();
-
 		public override void Execute(Invocation invocation)
 		{
 			invocation.Return(this.getter.Invoke());
 		}
 
+		public override bool TryGetReturnValue(out object returnValue)
+		{
+			returnValue = this.getter.Invoke();
+			return true;
+		}
+
 		public override MockException TryVerify()
 		{
 			return this.TryVerifyInnerMock(innerMock => innerMock.TryVerify());

src/Moq/Extensions.cs

@@ -273,13 +273,15 @@ private static void AddPropertiesInDepthFirstOrderTo(this Type type, List<Proper
 			}
 		}
 
-		public static bool TryFind(this IEnumerable<IDeterministicReturnValueSetup> setups, InvocationShape expectation, out IDeterministicReturnValueSetup setup)
+		public static bool TryFind(this IEnumerable<Setup> innerMockSetups, InvocationShape expectation, out Setup setup)
 		{
-			foreach (Setup s in setups)
+			Debug.Assert(innerMockSetups.All(s => s.ReturnsInnerMock(out _)));
+
+			foreach (Setup innerMockSetup in innerMockSetups)
 			{
-				if (s.Expectation.Equals(expectation))
+				if (innerMockSetup.Expectation.Equals(expectation))
 				{
-					setup = (IDeterministicReturnValueSetup)s;
+					setup = innerMockSetup;
 					return true;
 				}
 			}
@@ -288,13 +290,13 @@ public static bool TryFind(this IEnumerable<IDeterministicReturnValueSetup> setu
 			return false;
 		}
 
-		public static bool TryFind(this IEnumerable<IDeterministicReturnValueSetup> setups, Invocation invocation, out IDeterministicReturnValueSetup setup)
+		public static bool TryFind(this IEnumerable<Setup> innerMockSetups, Invocation invocation, out Setup setup)
 		{
-			foreach (Setup s in setups)
+			foreach (Setup innerMockSetup in innerMockSetups)
 			{
-				if (s.Matches(invocation))
+				if (innerMockSetup.Matches(invocation))
 				{
-					setup = (IDeterministicReturnValueSetup)s;
+					setup = innerMockSetup;
 					return true;
 				}
 			}

src/Moq/FluentMockVisitor.cs

@@ -172,10 +172,12 @@ private static Mock<TResult> FluentMock<T, TResult>(Mock<T> mock, Expression<Fun
 
 			Mock fluentMock;
 			object result;
-			if (mock.GetInnerMockSetups().TryFind(new InvocationShape(setup, info, arguments), out var inner))
+			if (mock.Setups.GetInnerMockSetups().TryFind(new InvocationShape(setup, info, arguments), out var inner))
 			{
+				Debug.Assert(inner.TryGetReturnValue(out _));  // guaranteed by .GetInnerMockSetups()
+
 				fluentMock = inner.GetInnerMock();
-				result = inner.ReturnValue;
+				_ = inner.TryGetReturnValue(out result);
 			}
 			else
 			{

src/Moq/InnerMockSetup.cs

@@ -3,7 +3,7 @@
 
 namespace Moq
 {
-	internal sealed class InnerMockSetup : SetupWithOutParameterSupport, IDeterministicReturnValueSetup
+	internal sealed class InnerMockSetup : SetupWithOutParameterSupport
 	{
 		private readonly object returnValue;
 
@@ -13,13 +13,17 @@ public InnerMockSetup(InvocationShape expectation, object returnValue)
 			this.returnValue = returnValue;
 		}
 
-		public object ReturnValue => this.returnValue;
-
 		public override void Execute(Invocation invocation)
 		{
 			invocation.Return(this.returnValue);
 		}
 
+		public override bool TryGetReturnValue(out object returnValue)
+		{
+			returnValue = this.returnValue;
+			return true;
+		}
+
 		public override MockException TryVerify()
 		{
 			return this.TryVerifyInnerMock(innerMock => innerMock.TryVerify());

src/Moq/Linq/Mock.cs

@@ -49,7 +49,7 @@ public static T Of<T>(Expression<Func<T, bool>> predicate) where T : class
 			// TODO: Make LINQ to Mocks set up mocks without causing invocations of its own, then remove this hack.
 			var mock = Mock.Get(mocked);
 			mock.Invocations.Clear();
-			foreach (var inner in mock.GetInnerMockSetups())
+			foreach (var inner in mock.Setups.GetInnerMockSetups())
 			{
 				inner.GetInnerMock().Invocations.Clear();
 			}

src/Moq/MethodCall.cs

@@ -123,6 +123,20 @@ public override void Execute(Invocation invocation)
 			}
 		}
 
+		public override bool TryGetReturnValue(out object returnValue)
+		{
+			if (this.returnOrThrowResponse is ReturnEagerValueResponse revs)
+			{
+				returnValue = revs.Value;
+				return true;
+			}
+			else
+			{
+				returnValue = default;
+				return false;
+			}
+		}
+
 		public void SetCallBaseResponse()
 		{
 			if (this.Mock.TargetType.IsDelegate())
@@ -416,16 +430,16 @@ public override void RespondTo(Invocation invocation)
 
 		private sealed class ReturnEagerValueResponse : Response
 		{
-			private readonly object value;
+			public readonly object Value;
 
 			public ReturnEagerValueResponse(object value)
 			{
-				this.value = value;
+				this.Value = value;
 			}
 
 			public override void RespondTo(Invocation invocation)
 			{
-				invocation.Return(this.value);
+				invocation.Return(this.Value);
 			}
 		}
 

src/Moq/Mock.cs

@@ -300,7 +300,7 @@ internal static void VerifyNoOtherCalls(Mock mock)
 		{
 			var unverifiedInvocations = mock.MutableInvocations.ToArray(invocation => !invocation.Verified);
 
-			var innerMockSetups = mock.GetInnerMockSetups();
+			var innerMockSetups = mock.Setups.GetInnerMockSetups();
 
 			if (unverifiedInvocations.Any())
 			{
@@ -496,7 +496,7 @@ private static TSetup SetupRecursivePexProtected<TSetup>(Mock mock, LambdaExpres
 			else
 			{
 				Mock innerMock;
-				if (!(mock.GetInnerMockSetups().TryFind(part, out var setup) && setup.ReturnsInnerMock(out innerMock)))
+				if (!(mock.Setups.GetInnerMockSetups().TryFind(part, out var setup) && setup.ReturnsInnerMock(out innerMock)))
 				{
 					var returnValue = mock.GetDefaultValue(method, out innerMock, useAlternateProvider: DefaultValueProvider.Mock);
 					if (innerMock == null)
@@ -647,7 +647,7 @@ internal static void RaiseEvent(Mock mock, LambdaExpression expression, Stack<In
 				}
 
 			}
-			else if (mock.GetInnerMockSetups().TryFind(part, out var innerMockSetup) && innerMockSetup.ReturnsInnerMock(out var innerMock))
+			else if (mock.Setups.GetInnerMockSetups().TryFind(part, out var innerMockSetup) && innerMockSetup.ReturnsInnerMock(out var innerMock))
 			{
 				Mock.RaiseEvent(innerMock, expression, parts, arguments);
 			}
@@ -777,12 +777,6 @@ internal void AddInnerMockSetup(MethodInfo method, IReadOnlyList<Expression> arg
 			this.Setups.Add(new InnerMockSetup(new InvocationShape(expression, method, arguments), returnValue));
 		}
 
-		internal IEnumerable<IDeterministicReturnValueSetup> GetInnerMockSetups()
-		{
-			return this.Setups.ToArrayLive(s => s is IDeterministicReturnValueSetup drvs && drvs.ReturnsInnerMock(out _))
-			                  .Cast<IDeterministicReturnValueSetup>();
-		}
-
 		#endregion
 	}
 }

src/Moq/MockException.cs

@@ -165,7 +165,7 @@ internal static MockException UnmatchedSetup(Setup setup)
 					setup));
 		}
 
-		internal static MockException FromInnerMockOf(IDeterministicReturnValueSetup setup, MockException error)
+		internal static MockException FromInnerMockOf(Setup setup, MockException error)
 		{
 			var message = new StringBuilder();
 

src/Moq/Setup.cs

@@ -1,6 +1,7 @@
 // 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.Expressions;
 using System.Reflection;
@@ -31,11 +32,40 @@ protected Setup(InvocationShape expectation)
 
 		public abstract void Execute(Invocation invocation);
 
+		public Mock GetInnerMock()
+		{
+			return this.ReturnsInnerMock(out var innerMock) ? innerMock : throw new InvalidOperationException();
+		}
+
+		/// <summary>
+		///   Attempts to get this setup's return value without invoking user code
+		///   (which could have side effects beyond Moq's understanding and control).
+		/// </summary>
+		public virtual bool TryGetReturnValue(out object returnValue)
+		{
+			returnValue = default;
+			return false;
+		}
+
 		public bool Matches(Invocation invocation)
 		{
 			return this.expectation.IsMatch(invocation) && (this.Condition == null || this.Condition.IsTrue);
 		}
 
+		public bool ReturnsInnerMock(out Mock mock)
+		{
+			if (this.TryGetReturnValue(out var returnValue) && Unwrap.ResultIfCompletedTask(returnValue) is IMocked mocked)
+			{
+				mock = mocked.Mock;
+				return true;
+			}
+			else
+			{
+				mock = null;
+				return false;
+			}
+		}
+
 		public virtual void SetOutParameters(Invocation invocation)
 		{
 		}
@@ -59,5 +89,19 @@ public virtual MockException TryVerify()
 		}
 
 		public abstract MockException TryVerifyAll();
+
+		public MockException TryVerifyInnerMock(Func<Mock, MockException> verify)
+		{
+			if (this.ReturnsInnerMock(out var innerMock))
+			{
+				var error = verify(innerMock);
+				if (error?.IsVerificationError == true)
+				{
+					return MockException.FromInnerMockOf(this, error);
+				}
+			}
+
+			return null;
+		}
 	}
 }

src/Moq/SetupCollection.cs

@@ -79,6 +79,11 @@ public Setup FindMatchFor(Invocation invocation)
 			return matchingSetup;
 		}
 
+		public IEnumerable<Setup> GetInnerMockSetups()
+		{
+			return this.ToArrayLive(setup => setup.ReturnsInnerMock(out _));
+		}
+
 		public Setup[] ToArrayLive(Func<Setup, bool> predicate)
 		{
 			var matchingSetups = new Stack<Setup>();

tests/Moq.Tests/RecursiveMocksFixture.cs

@@ -374,6 +374,46 @@ public void When_manually_set_up_3()
 			}
 		}
 
+		public class Inner_mock_reachability
+		{
+			[Fact]
+			public void Reachable_if_set_up_using_eager_Returns()
+			{
+				var bar = new Mock<IBar>();
+				bar.Setup(b => b.Value).Returns(42);
+
+				var foo = new Mock<IFoo>();
+				foo.Setup(f => f.Bar).Returns(bar.Object);
+				foo.Setup(f => f.Bar.Baz);
+
+				Assert.Equal(42, foo.Object.Bar.Value);
+				bar.VerifyGet(b => b.Value, Times.Once);
+			}
+
+			[Fact]
+			public void Not_reachable_if_set_up_using_lazy_Returns()
+			{
+				var bar = new Mock<IBar>();
+				bar.Setup(b => b.Value).Returns(42);
+
+				var foo = new Mock<IFoo>();
+				foo.Setup(f => f.Bar).Returns(() => bar.Object);
+				//                            ^^^^^^
+				// Main difference to the above test. What we want to test for here is
+				// that Moq won't execute user-provided callbacks to figure out a setup's
+				// return value (as this could have side effects without Moq's control).
+
+				foo.Setup(f => f.Bar.Baz);
+				//              ^^^^^
+				// ... and because Moq can't query the above setup to figure out there's
+				// already an inner mock attached, it will create a fresh setup instead,
+				// effectively "cutting off" the above `IBar` mock.
+
+				Assert.NotEqual(42, foo.Object.Bar.Value);
+				bar.VerifyGet(b => b.Value, Times.Never);
+			}
+		}
+
 		public class Foo : IFoo
 		{