Implementing basic detection of type mismatches for Returns().

Source/NSubstitute.Acceptance.Specs/FieldReports/CallingIntoNewSubWithinReturns.cs

@@ -1,4 +1,5 @@
-using NSubstitute.Exceptions;
+using System;
+using NSubstitute.Exceptions;
 using NUnit.Framework;
 
 namespace NSubstitute.Acceptance.Specs.FieldReports
@@ -7,19 +8,22 @@ namespace NSubstitute.Acceptance.Specs.FieldReports
     {
         public interface IFoo { string SomeString { get; set; } }
         public interface IBar { IFoo GetFoo(); }
+        public interface IZap { int Num { get; set; } }
 
         [Test]
-        [Pending, Explicit]
         public void ShouldDetectTypeMismatchInReturns()
         {
             var sub = Substitute.For<IBar>();
 
-            Assert.Throws<CouldNotSetReturnException>(() => 
-                // GetFoo() called, then IPityTheFoo(), then Returns(..) is called.
-                // This means Returns(..) tries to update the last called sub, 
-                // which is IFoo.SomeString, not IBar.GetFoo(). 
-                sub.GetFoo().Returns(IPityTheFoo())
-            );
+            var ex = 
+                Assert.Throws<CouldNotSetReturnDueToTypeMismatchException>(() =>
+                    // GetFoo() called, then IPityTheFoo(), then Returns(..) is called.
+                    // This means Returns(..) tries to update the last called sub, 
+                    // which is IFoo.SomeString, not IBar.GetFoo(). 
+                    sub.GetFoo().Returns(IPityTheFoo())
+                );
+
+            Assert.That(ex.Message, Is.StringStarting("Can not return value of type "));
         }
 
         private IFoo IPityTheFoo()
@@ -28,5 +32,42 @@ namespace NSubstitute.Acceptance.Specs.FieldReports
             foo.SomeString = "call a property so static LastCalled is updated";
             return foo;
         }
+
+        [Test]
+        public void ShouldDetectedWhenNestedReturnsClearsLastCallRouter()
+        {
+            var sub = Substitute.For<IBar>();
+
+            Assert.Throws<CouldNotSetReturnDueToNoLastCallException>(() => 
+                sub.GetFoo().Returns(CreateFooAndCallReturns())
+            );
+        }
+
+        private IFoo CreateFooAndCallReturns()
+        {
+            var foo = Substitute.For<IFoo>();
+            foo.SomeString.Returns("nested set");
+            return foo;
+        }
+
+        [Test]
+        public void ShouldDetectTypeMismatchWhenNullIsInvolved()
+        {
+            var sub = Substitute.For<IBar>();
+
+            var ex = 
+                Assert.Throws<CouldNotSetReturnDueToTypeMismatchException>(() => 
+                    sub.GetFoo().Returns(DoEvilAndReturnNullRef())
+                );
+
+            Assert.That(ex.Message, Is.StringStarting("Can not return null for"));
+        }
+
+        private IFoo DoEvilAndReturnNullRef()
+        {
+            var zap = Substitute.For<IZap>();
+            zap.Num = 2;
+            return null;
+        }
     }
 }

Source/NSubstitute.Acceptance.Specs/FieldReports/StaticStateBleeding.cs

@@ -16,7 +16,7 @@ namespace NSubstitute.Acceptance.Specs.FieldReports
         [Test]
         public void Test_1_affected_by_test_0()
         {
-            Assert.Throws<CouldNotSetReturnException>(() => 2.Returns(2));
+            Assert.Throws<CouldNotSetReturnDueToNoLastCallException>(() => 2.Returns(2));
         }
 
         public interface IFoo { }

Source/NSubstitute.Acceptance.Specs/ReturningResults.cs

@@ -116,20 +116,16 @@ namespace NSubstitute.Acceptance.Specs
         [Test]
         public void Throw_when_blatantly_misusing_returns()
         {
-            const string expectedMessage = 
-                "Could not find a call to return from.\n"+
-                "Make sure you called Returns() after calling your substitute (for example: mySub.SomeMethod().Returns(value)).\n" +
-                "If you substituted for a class rather than an interface, check that the call to your substitute was on a virtual/abstract member.\n" +
-                "Return values cannot be configured for non-virtual/non-abstract members.";
+            const string expectedMessagePrefix = "Could not find a call to return from.";
 
-            var exception = Assert.Throws<CouldNotSetReturnException>(() =>
+            var exception = Assert.Throws<CouldNotSetReturnDueToNoLastCallException>(() =>
               {
                   //Start with legitimate call to Returns (so the static context will not have any residual calls stored).
                   _something.Echo(1).Returns("one");
                   //Now we'll misuse Returns.
                   "".Returns("I shouldn't be calling returns like this!");
               });
-            Assert.That(exception.Message, Contains.Substring(expectedMessage));
+            Assert.That(exception.Message, Is.StringContaining(expectedMessagePrefix));
         }
 
         [SetUp]

Source/NSubstitute.Specs/ConfigureCallSpecs.cs

@@ -1,5 +1,6 @@
 using System;
 using NSubstitute.Core;
+using NSubstitute.Exceptions;
 using NSubstitute.Specs.Infrastructure;
 using NUnit.Framework;
 
@@ -12,7 +13,7 @@ namespace NSubstitute.Specs
             protected ICallActions _callActions;
             protected ICallResults _configuredResults;
             protected IGetCallSpec _getCallSpec;
-            protected IReturn _returnValue;
+            protected IReturn _compatibleReturnValue;
 
             public override void Context()
             {
@@ -20,7 +21,8 @@ namespace NSubstitute.Specs
                 _callActions = mock<ICallActions>();
                 _getCallSpec = mock<IGetCallSpec>();
 
-                _returnValue = mock<IReturn>();
+                _compatibleReturnValue = mock<IReturn>();
+                _compatibleReturnValue.stub(x => x.CanBeAssignedTo(It.IsAny<Type>())).Return(true);
             }
 
             public override ConfigureCall CreateSubjectUnderTest()
@@ -37,9 +39,9 @@ namespace NSubstitute.Specs
                 var lastCallSpec = mock<ICallSpecification>();
                 _getCallSpec.stub(x => x.FromLastCall(MatchArgs.AsSpecifiedInCall)).Return(lastCallSpec);
 
-                sut.SetResultForLastCall(_returnValue, MatchArgs.AsSpecifiedInCall);
+                sut.SetResultForLastCall(_compatibleReturnValue, MatchArgs.AsSpecifiedInCall);
 
-                _configuredResults.received(x => x.SetResult(lastCallSpec, _returnValue));
+                _configuredResults.received(x => x.SetResult(lastCallSpec, _compatibleReturnValue));
             }
         }
 
@@ -52,9 +54,9 @@ namespace NSubstitute.Specs
                 var callSpec = mock<ICallSpecification>();
                 _getCallSpec.stub(x => x.FromCall(call, MatchArgs.AsSpecifiedInCall)).Return(callSpec);
 
-                sut.SetResultForCall(call, _returnValue, MatchArgs.AsSpecifiedInCall);
+                sut.SetResultForCall(call, _compatibleReturnValue, MatchArgs.AsSpecifiedInCall);
 
-                _configuredResults.received(x => x.SetResult(callSpec, _returnValue));
+                _configuredResults.received(x => x.SetResult(callSpec, _compatibleReturnValue));
             }
         }
 
@@ -68,7 +70,7 @@ namespace NSubstitute.Specs
                 var lastCallSpec = mock<ICallSpecification>();
                 _getCallSpec.stub(x => x.FromLastCall(MatchArgs.AsSpecifiedInCall)).Return(lastCallSpec);
 
-                var config = sut.SetResultForLastCall(_returnValue, MatchArgs.AsSpecifiedInCall);
+                var config = sut.SetResultForLastCall(_compatibleReturnValue, MatchArgs.AsSpecifiedInCall);
                 config
                     .AndDoes(firstAction)
                     .AndDoes(secondAction);
@@ -77,5 +79,27 @@ namespace NSubstitute.Specs
                 _callActions.received(x => x.Add(lastCallSpec, secondAction));
             }
         }
+
+        public class When_setting_incompatible_result_for_call : Concern
+        {
+            [Test]
+            public void Configure_result_for_last_specified_call()
+            {
+                var lastCallSpec = mock<ICallSpecification>();
+                _getCallSpec.stub(x => x.FromLastCall(MatchArgs.AsSpecifiedInCall)).Return(lastCallSpec);
+                lastCallSpec.stub(x => x.GetMethodInfo()).Return(ReflectionHelper.GetMethod(() => SomeType.SampleMethod()));
+
+                var incompatibleReturn = mock<IReturn>();
+                incompatibleReturn.stub(x => x.CanBeAssignedTo(It.IsAny<Type>())).Return(false);
+                incompatibleReturn.stub(x => x.TypeOrNull()).Return(typeof (SomeType));
+
+                Assert.Throws<CouldNotSetReturnDueToTypeMismatchException>(
+                        () => sut.SetResultForLastCall(incompatibleReturn, MatchArgs.AsSpecifiedInCall)
+                    );
+                _configuredResults.did_not_receive(x => x.SetResult(lastCallSpec, incompatibleReturn));
+            }
+        }
+
+        private class SomeType { public static SomeType SampleMethod() { return null; } }
     }
 }

Source/NSubstitute.Specs/SubstitutionContextSpecs.cs

@@ -38,7 +38,7 @@ namespace NSubstitute.Specs
             [Test]
             public void Should_throw_if_trying_to_set_another_return_value_before_another_call_is_made_on_a_substitute()
             {
-                Assert.Throws<CouldNotSetReturnException>(() => sut.LastCallShouldReturn(mock<IReturn>(), MatchArgs.AsSpecifiedInCall));
+                Assert.Throws<CouldNotSetReturnDueToNoLastCallException>(() => sut.LastCallShouldReturn(mock<IReturn>(), MatchArgs.AsSpecifiedInCall));
             }
 
             public override void Because()
@@ -60,7 +60,7 @@ namespace NSubstitute.Specs
             [Test]
             public void Should_throw()
             {
-                Assert.Throws<CouldNotSetReturnException>(() => sut.LastCallShouldReturn(mock<IReturn>(), MatchArgs.AsSpecifiedInCall));
+                Assert.Throws<CouldNotSetReturnDueToNoLastCallException>(() => sut.LastCallShouldReturn(mock<IReturn>(), MatchArgs.AsSpecifiedInCall));
             }
         }
 

Source/NSubstitute/Core/CallSpecification.cs

@@ -20,10 +20,9 @@ namespace NSubstitute.Core
             _argumentSpecifications = argumentSpecifications.ToArray();
         }
 
-        public MethodInfo GetMethodInfo()
-        {
-            return _methodInfo;
-        }
+        public MethodInfo GetMethodInfo() { return _methodInfo; }
+
+        public Type ReturnType() { return _methodInfo.ReturnType; }
 
         public bool IsSatisfiedBy(ICall call)
         {

Source/NSubstitute/Core/ConfigureCall.cs

@@ -1,4 +1,4 @@
-using System;
+using NSubstitute.Exceptions;
 
 namespace NSubstitute.Core
 {
@@ -18,14 +18,25 @@ namespace NSubstitute.Core
         public ConfiguredCall SetResultForLastCall(IReturn valueToReturn, MatchArgs matchArgs)
         {
             var spec = _getCallSpec.FromLastCall(matchArgs);
+            CheckResultIsCompatibleWithCall(valueToReturn, spec);
             _configuredResults.SetResult(spec, valueToReturn);
             return new ConfiguredCall(action => _callActions.Add(spec, action));
         }
 
         public void SetResultForCall(ICall call, IReturn valueToReturn, MatchArgs matchArgs)
         {
-            var callSpecification = _getCallSpec.FromCall(call, matchArgs);
-            _configuredResults.SetResult(callSpecification, valueToReturn);
+            var spec = _getCallSpec.FromCall(call, matchArgs);
+            CheckResultIsCompatibleWithCall(valueToReturn, spec);
+            _configuredResults.SetResult(spec, valueToReturn);
+        }
+
+        private static void CheckResultIsCompatibleWithCall(IReturn valueToReturn, ICallSpecification spec)
+        {
+            var requiredReturnType = spec.ReturnType();
+            if (!valueToReturn.CanBeAssignedTo(requiredReturnType))
+            {
+                throw new CouldNotSetReturnDueToTypeMismatchException(valueToReturn.TypeOrNull(), spec.GetMethodInfo());
+            }
         }
     }
 }

Source/NSubstitute/Core/ICallSpecification.cs

@@ -13,5 +13,6 @@ namespace NSubstitute.Core
         void InvokePerArgumentActions(CallInfo callInfo);
         IEnumerable<ArgumentMatchInfo> NonMatchingArguments(ICall call);
         MethodInfo GetMethodInfo();
+        Type ReturnType();
     }
 }

Source/NSubstitute/Core/IReturn.cs

@@ -7,6 +7,8 @@ namespace NSubstitute.Core
     public interface IReturn
     {
         object ReturnFor(CallInfo info);
+        Type TypeOrNull();
+        bool CanBeAssignedTo(Type t);
     }
 
     public class ReturnValue : IReturn
@@ -14,6 +16,8 @@ namespace NSubstitute.Core
         private readonly object _value;
         public ReturnValue(object value) { _value = value; }
         public object ReturnFor(CallInfo info) { return _value; }
+        public Type TypeOrNull() { return _value == null ? null : _value.GetType(); }
+        public bool CanBeAssignedTo(Type t) { return _value.IsCompatibleWith(t); }
     }
 
     public class ReturnValueFromFunc<T> : IReturn
@@ -21,6 +25,9 @@ namespace NSubstitute.Core
         private readonly Func<CallInfo, T> _funcToReturnValue;
         public ReturnValueFromFunc(Func<CallInfo, T> funcToReturnValue) { _funcToReturnValue = funcToReturnValue ?? ReturnNull(); }
         public object ReturnFor(CallInfo info) { return _funcToReturnValue(info); }
+        public Type TypeOrNull() { return typeof (T); }
+        public bool CanBeAssignedTo(Type t) { return typeof (T).IsAssignableFrom(t); }
+
         private static Func<CallInfo, T> ReturnNull()
         {
             if (typeof(T).IsValueType) throw new CannotReturnNullForValueType(typeof(T));
@@ -36,6 +43,8 @@ namespace NSubstitute.Core
             _valuesToReturn = ValuesToReturn(values).GetEnumerator();
         }
         public object ReturnFor(CallInfo info) { return GetNext(); }
+        public Type TypeOrNull() { return typeof (T); }
+        public bool CanBeAssignedTo(Type t) { return typeof (T).IsAssignableFrom(t); }
 
         private T GetNext()
         {
@@ -63,6 +72,8 @@ namespace NSubstitute.Core
             _valuesToReturn = ValuesToReturn(funcs).GetEnumerator();
         }
         public object ReturnFor(CallInfo info) { return GetNext(info); }
+        public Type TypeOrNull() { return typeof (T); }
+        public bool CanBeAssignedTo(Type t) { return typeof (T).IsAssignableFrom(t); }
 
         private T GetNext(CallInfo info)
         {

Source/NSubstitute/Core/SubstitutionContext.cs

@@ -46,7 +46,7 @@ namespace NSubstitute.Core
 
         public ConfiguredCall LastCallShouldReturn(IReturn value, MatchArgs matchArgs)
         {
-            if (_lastCallRouter.Value == null) throw new CouldNotSetReturnException();
+            if (_lastCallRouter.Value == null) throw new CouldNotSetReturnDueToNoLastCallException();
             var configuredCall = _lastCallRouter.Value.LastCallShouldReturn(value, matchArgs);
             ClearLastCallRouter();
             return configuredCall;

Source/NSubstitute/Exceptions/CouldNotSetReturnException.cs

@@ -1,16 +1,49 @@
-using System.Runtime.Serialization;
+using System;
+using System.Reflection;
+using System.Runtime.Serialization;
 
 namespace NSubstitute.Exceptions
 {
-    public class CouldNotSetReturnException : SubstituteException
+    public abstract class CouldNotSetReturnException : SubstituteException
     {
-        const string WhatProbablyWentWrong = 
-                "Could not find a call to return from.\n"+
-                "Make sure you called Returns() after calling your substitute (for example: mySub.SomeMethod().Returns(value)).\n" +
+        protected const string WhatProbablyWentWrong = 
+                "Make sure you called Returns() after calling your substitute (for example: mySub.SomeMethod().Returns(value)),\n" +
+                "and that you are not configuring other substitutes within Returns() (for example, avoid this: mySub.SomeMethod().Returns(ConfigOtherSub())).\n" +
+                "\n" +
                 "If you substituted for a class rather than an interface, check that the call to your substitute was on a virtual/abstract member.\n" +
-                "Return values cannot be configured for non-virtual/non-abstract members.";
+                "Return values cannot be configured for non-virtual/non-abstract members.\n" +
+                "\n" +
+                "Correct use:\n" +
+                "\tmySub.SomeMethod().Returns(returnValue);\n" +
+                "\n" +
+                "Potentially problematic use:\n" +
+                "\tmySub.SomeMethod().Returns(ConfigOtherSub());\n" +
+                "Instead try:\n" +
+                "\tvar returnValue = ConfigOtherSub();\n" +
+                "\tmySub.SomeMethod().Returns(returnValue);\n" +
+                "";
 
-        public CouldNotSetReturnException() : base(WhatProbablyWentWrong) { }
+        protected CouldNotSetReturnException(string s) : base(s + "\n\n" + WhatProbablyWentWrong) { }
         protected CouldNotSetReturnException(SerializationInfo info, StreamingContext context) : base(info, context) { }
     }
+
+    public class CouldNotSetReturnDueToNoLastCallException : CouldNotSetReturnException
+    {
+        public CouldNotSetReturnDueToNoLastCallException() : base("Could not find a call to return from.") { }
+        protected CouldNotSetReturnDueToNoLastCallException(SerializationInfo info, StreamingContext context) : base(info, context) { }
+    }
+
+    public class CouldNotSetReturnDueToTypeMismatchException : CouldNotSetReturnException
+    {
+        public CouldNotSetReturnDueToTypeMismatchException(Type returnType, MethodInfo member) : base(DescribeProblem(returnType, member)) { }
+
+        private static string DescribeProblem(Type typeOfReturnValueOrNull, MethodInfo member)
+        {
+            return typeOfReturnValueOrNull == null 
+                ? String.Format("Can not return null for {0}.{1} (expected type {2}).", member.DeclaringType.Name, member.Name, member.ReturnType.Name) 
+                : String.Format("Can not return value of type {0} for {1}.{2} (expected type {3}).", typeOfReturnValueOrNull.Name, member.DeclaringType.Name, member.Name, member.ReturnType.Name);
+        }
+
+        protected CouldNotSetReturnDueToTypeMismatchException(SerializationInfo info, StreamingContext context) : base(info, context) { }
+    }
 }