Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions src/NSubstitute/Core/CallSpecification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ private static Type[] ParameterTypes(MethodInfo info)
return info.GetParameters().Select(p => p.ParameterType).ToArray();
}

internal static bool TypesAreAllEquivalent(Type[] aArgs, Type[] bArgs)
internal static bool TypesAreAllEquivalent(Type[] aArgs, Type[] bArgs, AreAssignableInclusionType assignableInclusionType = AreAssignableInclusionType.Include)
{
if (aArgs.Length != bArgs.Length) return false;
for (var i = 0; i < aArgs.Length; i++)
Expand All @@ -79,21 +79,31 @@ internal static bool TypesAreAllEquivalent(Type[] aArgs, Type[] bArgs)
if (first.IsGenericType && second.IsGenericType
&& first.GetGenericTypeDefinition() == second.GetGenericTypeDefinition())
{
var genericArgumentsInclusionType = assignableInclusionType switch
{
AreAssignableInclusionType.Exclude => AreAssignableInclusionType.Exclude,
AreAssignableInclusionType.ExcludeGenericArguments => AreAssignableInclusionType.Exclude,
_ => AreAssignableInclusionType.Include
};

// both are the same generic type. If their GenericTypeArguments match then they are equivalent
if (!TypesAreAllEquivalent(first.GenericTypeArguments, second.GenericTypeArguments))
if (!TypesAreAllEquivalent(first.GenericTypeArguments, second.GenericTypeArguments, genericArgumentsInclusionType))
{
return false;
}
continue;
}

var includeAreAssignable = assignableInclusionType == AreAssignableInclusionType.Include
|| assignableInclusionType == AreAssignableInclusionType.ExcludeGenericArguments;

var areAssignable = first.IsAssignableFrom(second) || second.IsAssignableFrom(first);
var areAnyTypeAssignable = typeof(Arg.AnyType).IsAssignableFrom(first) ||
typeof(Arg.AnyType).IsAssignableFrom(second);
var areByRefAnyTypeAssignable = first.IsByRef && second.IsByRef &&
(typeof(Arg.AnyType).IsAssignableFrom(first.GetElementType()) ||
typeof(Arg.AnyType).IsAssignableFrom(second.GetElementType()));
var areEquivalent = areAssignable || areAnyTypeAssignable || areByRefAnyTypeAssignable;
var areEquivalent = (includeAreAssignable && areAssignable) || areAnyTypeAssignable || areByRefAnyTypeAssignable;
if (!areEquivalent) return false;
}
return true;
Expand All @@ -102,7 +112,12 @@ internal static bool TypesAreAllEquivalent(Type[] aArgs, Type[] bArgs)
private static bool AreEquivalentDefinitions(MethodInfo a, MethodInfo b)
{
return a.IsGenericMethod == b.IsGenericMethod
&& TypesAreAllEquivalent([a.ReturnType], [b.ReturnType])

// Exclude the assignable check for generic arguments, generic types generally are not equivalent when a generic argument is not
// exactly the same, even though the types themselves may be assignable.
// See https://github.com/nsubstitute/NSubstitute/issues/974.
&& TypesAreAllEquivalent([a.ReturnType], [b.ReturnType], AreAssignableInclusionType.ExcludeGenericArguments)
Comment on lines +116 to +119

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: do you know if there needs to be any additional handling for generics with co/contravariance?


&& a.Name.Equals(b.Name, StringComparison.Ordinal);
}

Expand Down Expand Up @@ -175,4 +190,25 @@ private bool HasDifferentNumberOfArguments(ICall call)
{
return _argumentSpecifications.Length != call.GetOriginalArguments().Length;
}

/// <summary>
/// Specifies whether an assignability check between types should be included for type equivalence checks.
/// </summary>
internal enum AreAssignableInclusionType
{
/// <summary>
/// Include the check
/// </summary>
Include,

/// <summary>
/// Exclude the check
/// </summary>
Exclude,

/// <summary>
/// Exclude the check for generic arguments, but include it for the current type
/// </summary>
ExcludeGenericArguments,
}
}
22 changes: 20 additions & 2 deletions tests/NSubstitute.Acceptance.Specs/GenericArguments.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using NUnit.Framework;
using System.Collections;
using System.Globalization;
using System.Reflection;
using NUnit.Framework;

namespace NSubstitute.Acceptance.Specs;

Expand Down Expand Up @@ -180,6 +180,24 @@ public void Returns_works_with_AnyType_for_ref_parameter_with_AnyType_generic_ar
Assert.That(result, Is.True);
}

/// <summary>
/// See https://github.com/nsubstitute/NSubstitute/issues/974.
/// </summary>
[Test]
public void Returns_works_with_mismatching_generic_return_types()
{
ISomethingWithGenerics something = Substitute.For<ISomethingWithGenerics>();

something
.SomeFunction(Arg.Any<ICloneable>())
.Returns(Substitute.For<ICollection<ICloneable>>());

something
.SomeFunction(Arg.Any<string>())
.Returns(Substitute.For<ICollection<string>>());
}


Comment on lines +186 to +200

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: might be worth adding the specific case from #974 as well:

    [Test]
    public void Issue974()
    {
        var sub = Substitute.For<ISender>();

        sub.Send(Arg.Any<NonGenericRequest>()).Returns(Substitute.For<IResult>());
        sub.Send(Arg.Any<GenericRequest>()).Returns(Substitute.For<IResult<int>>());

        sub.Send(new NonGenericRequest());
        sub.Send(new GenericRequest());
    }

    public interface IResult { }
    public interface IResult<T> : IResult { }
    public interface IRequest<TResponse> { }
    public class NonGenericRequest : IRequest<IResult> { }
    public class GenericRequest : IRequest<IResult<int>> { }
    public interface ISender {
        Task<TResponse> Send<TResponse>(IRequest<TResponse> request);
    }

[Test]
public void Callback_allows_access_to_method_call()
{
Expand Down Expand Up @@ -210,4 +228,4 @@ static ICollection<T> CreateSubstitute<T>(int count)

Assert.That(result.Count, Is.EqualTo(7));
}
}
}
Loading