diff --git a/Source/NSubstitute.Acceptance.Specs/FieldReports/CallingIntoNewSubWithinReturns.cs b/Source/NSubstitute.Acceptance.Specs/FieldReports/CallingIntoNewSubWithinReturns.cs index 04d49d8..b16d61a 100644 --- a/Source/NSubstitute.Acceptance.Specs/FieldReports/CallingIntoNewSubWithinReturns.cs +++ b/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(); - Assert.Throws(() => - // 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(() => + // 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(); + + Assert.Throws(() => + sub.GetFoo().Returns(CreateFooAndCallReturns()) + ); + } + + private IFoo CreateFooAndCallReturns() + { + var foo = Substitute.For(); + foo.SomeString.Returns("nested set"); + return foo; + } + + [Test] + public void ShouldDetectTypeMismatchWhenNullIsInvolved() + { + var sub = Substitute.For(); + + var ex = + Assert.Throws(() => + sub.GetFoo().Returns(DoEvilAndReturnNullRef()) + ); + + Assert.That(ex.Message, Is.StringStarting("Can not return null for")); + } + + private IFoo DoEvilAndReturnNullRef() + { + var zap = Substitute.For(); + zap.Num = 2; + return null; + } } } \ No newline at end of file diff --git a/Source/NSubstitute.Acceptance.Specs/FieldReports/StaticStateBleeding.cs b/Source/NSubstitute.Acceptance.Specs/FieldReports/StaticStateBleeding.cs index e512704..905447a 100644 --- a/Source/NSubstitute.Acceptance.Specs/FieldReports/StaticStateBleeding.cs +++ b/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(() => 2.Returns(2)); + Assert.Throws(() => 2.Returns(2)); } public interface IFoo { } diff --git a/Source/NSubstitute.Acceptance.Specs/ReturningResults.cs b/Source/NSubstitute.Acceptance.Specs/ReturningResults.cs index df02485..5ccf370 100644 --- a/Source/NSubstitute.Acceptance.Specs/ReturningResults.cs +++ b/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(() => + var exception = Assert.Throws(() => { //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] diff --git a/Source/NSubstitute.Specs/ConfigureCallSpecs.cs b/Source/NSubstitute.Specs/ConfigureCallSpecs.cs index 9492498..d9cc9ca 100644 --- a/Source/NSubstitute.Specs/ConfigureCallSpecs.cs +++ b/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(); _getCallSpec = mock(); - _returnValue = mock(); + _compatibleReturnValue = mock(); + _compatibleReturnValue.stub(x => x.CanBeAssignedTo(It.IsAny())).Return(true); } public override ConfigureCall CreateSubjectUnderTest() @@ -37,9 +39,9 @@ namespace NSubstitute.Specs var lastCallSpec = mock(); _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(); _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(); _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(); + _getCallSpec.stub(x => x.FromLastCall(MatchArgs.AsSpecifiedInCall)).Return(lastCallSpec); + lastCallSpec.stub(x => x.GetMethodInfo()).Return(ReflectionHelper.GetMethod(() => SomeType.SampleMethod())); + + var incompatibleReturn = mock(); + incompatibleReturn.stub(x => x.CanBeAssignedTo(It.IsAny())).Return(false); + incompatibleReturn.stub(x => x.TypeOrNull()).Return(typeof (SomeType)); + + Assert.Throws( + () => sut.SetResultForLastCall(incompatibleReturn, MatchArgs.AsSpecifiedInCall) + ); + _configuredResults.did_not_receive(x => x.SetResult(lastCallSpec, incompatibleReturn)); + } + } + + private class SomeType { public static SomeType SampleMethod() { return null; } } } } diff --git a/Source/NSubstitute.Specs/SubstitutionContextSpecs.cs b/Source/NSubstitute.Specs/SubstitutionContextSpecs.cs index 5521838..76f076b 100644 --- a/Source/NSubstitute.Specs/SubstitutionContextSpecs.cs +++ b/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(() => sut.LastCallShouldReturn(mock(), MatchArgs.AsSpecifiedInCall)); + Assert.Throws(() => sut.LastCallShouldReturn(mock(), MatchArgs.AsSpecifiedInCall)); } public override void Because() @@ -60,7 +60,7 @@ namespace NSubstitute.Specs [Test] public void Should_throw() { - Assert.Throws(() => sut.LastCallShouldReturn(mock(), MatchArgs.AsSpecifiedInCall)); + Assert.Throws(() => sut.LastCallShouldReturn(mock(), MatchArgs.AsSpecifiedInCall)); } } diff --git a/Source/NSubstitute/Core/CallSpecification.cs b/Source/NSubstitute/Core/CallSpecification.cs index aa7efc4..2e7ec97 100644 --- a/Source/NSubstitute/Core/CallSpecification.cs +++ b/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) { diff --git a/Source/NSubstitute/Core/ConfigureCall.cs b/Source/NSubstitute/Core/ConfigureCall.cs index 7ead04f..3c8293b 100644 --- a/Source/NSubstitute/Core/ConfigureCall.cs +++ b/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()); + } } } } \ No newline at end of file diff --git a/Source/NSubstitute/Core/ICallSpecification.cs b/Source/NSubstitute/Core/ICallSpecification.cs index 2be57e2..7acf1d1 100644 --- a/Source/NSubstitute/Core/ICallSpecification.cs +++ b/Source/NSubstitute/Core/ICallSpecification.cs @@ -13,5 +13,6 @@ namespace NSubstitute.Core void InvokePerArgumentActions(CallInfo callInfo); IEnumerable NonMatchingArguments(ICall call); MethodInfo GetMethodInfo(); + Type ReturnType(); } } \ No newline at end of file diff --git a/Source/NSubstitute/Core/IReturn.cs b/Source/NSubstitute/Core/IReturn.cs index 8f235e3..a0543a3 100644 --- a/Source/NSubstitute/Core/IReturn.cs +++ b/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 : IReturn @@ -21,6 +25,9 @@ namespace NSubstitute.Core private readonly Func _funcToReturnValue; public ReturnValueFromFunc(Func 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 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) { diff --git a/Source/NSubstitute/Core/SubstitutionContext.cs b/Source/NSubstitute/Core/SubstitutionContext.cs index 7b85da6..c2ef214 100644 --- a/Source/NSubstitute/Core/SubstitutionContext.cs +++ b/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; diff --git a/Source/NSubstitute/Exceptions/CouldNotSetReturnException.cs b/Source/NSubstitute/Exceptions/CouldNotSetReturnException.cs index 20e6e78..7cb2ec7 100644 --- a/Source/NSubstitute/Exceptions/CouldNotSetReturnException.cs +++ b/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) { } + } } \ No newline at end of file