From 1590bc489aa43459104e92c1350cc9703f64c766 Mon Sep 17 00:00:00 2001 From: Thomas Levesque Date: Thu, 26 Sep 2019 12:20:47 +0200 Subject: [PATCH 1/2] Fix equality for wrapping fakes --- src/FakeItEasy/Core/WrappedObjectRule.cs | 40 ++++++++- tests/FakeItEasy.Specs/WrappingFakeSpecs.cs | 91 ++++++++++++++++++++- 2 files changed, 127 insertions(+), 4 deletions(-) diff --git a/src/FakeItEasy/Core/WrappedObjectRule.cs b/src/FakeItEasy/Core/WrappedObjectRule.cs index 291346cfa..a1c9dd41e 100644 --- a/src/FakeItEasy/Core/WrappedObjectRule.cs +++ b/src/FakeItEasy/Core/WrappedObjectRule.cs @@ -9,6 +9,8 @@ namespace FakeItEasy.Core internal class WrappedObjectRule : IFakeObjectCallRule { + private static readonly MethodInfo EqualsMethod = typeof(object).GetMethod(nameof(object.Equals), new[] { typeof(object) }); + private readonly object wrappedObject; /// @@ -51,10 +53,35 @@ public void Apply(IInterceptedFakeObjectCall fakeObjectCall) Guard.AgainstNull(fakeObjectCall, nameof(fakeObjectCall)); var parameters = fakeObjectCall.Arguments.GetUnderlyingArgumentsArray(); - object valueFromWrappedInstance; + object returnValue; try { - valueFromWrappedInstance = fakeObjectCall.Method.Invoke(this.wrappedObject, parameters); + if (IsCallsToEquals(fakeObjectCall)) + { + var arg = parameters[0]; + if (ReferenceEquals(arg, fakeObjectCall.FakedObject)) + { + // fake.Equals(fake) returns true + returnValue = true; + } + else if (ReferenceEquals(arg, this.wrappedObject)) + { + // fake.Equals(wrappedObject) returns wrappedObject.Equals(fake) + // This will be false if Equals isn't overriden (reference equality) + // and true if Equals is overriden to implement value semantics. + // This approach has the benefit of keeping Equals symmetrical. + returnValue = this.wrappedObject.Equals(fakeObjectCall.FakedObject); + } + else + { + // fake.Equals(somethingElse) is delegated to the wrapped object (no special case) + returnValue = this.wrappedObject.Equals(arg); + } + } + else + { + returnValue = fakeObjectCall.Method.Invoke(this.wrappedObject, parameters); + } } catch (TargetInvocationException ex) { @@ -62,7 +89,14 @@ public void Apply(IInterceptedFakeObjectCall fakeObjectCall) throw; } - fakeObjectCall.SetReturnValue(valueFromWrappedInstance); + fakeObjectCall.SetReturnValue(returnValue); + } + + private static bool IsCallsToEquals(IFakeObjectCall fakeObjectCall) + { + return fakeObjectCall.Method.DeclaringType == EqualsMethod.DeclaringType && + fakeObjectCall.Method.MetadataToken == EqualsMethod.MetadataToken && + fakeObjectCall.Method.Module == EqualsMethod.Module; } } } diff --git a/tests/FakeItEasy.Specs/WrappingFakeSpecs.cs b/tests/FakeItEasy.Specs/WrappingFakeSpecs.cs index 3de56b1f2..314026c99 100644 --- a/tests/FakeItEasy.Specs/WrappingFakeSpecs.cs +++ b/tests/FakeItEasy.Specs/WrappingFakeSpecs.cs @@ -1,4 +1,4 @@ -namespace FakeItEasy.Specs +namespace FakeItEasy.Specs { using System; using FakeItEasy.Tests.TestHelpers; @@ -17,6 +17,11 @@ public interface IFoo void OutAndRefMethod(ref int @ref, out int @out); } + public interface IBar + { + int Id { get; } + } + [Scenario] public static void NonVoidSuccess( Foo realObject, @@ -136,6 +141,70 @@ public interface IFoo .x(() => @out.Should().Be(42)); } + [Scenario] + public static void FakeEqualsFake(Foo realObject, IFoo wrapper, bool equals) + { + "Given a real object" + .x(() => realObject = new Foo()); + + "And a fake wrapping this object" + .x(() => wrapper = A.Fake(o => o.Wrapping(realObject))); + + "When Equals is called on the fake with itself as the argument" + .x(() => equals = wrapper.Equals(wrapper)); + + "Then it should return true" + .x(() => equals.Should().BeTrue()); + } + + [Scenario] + public static void FakeEqualsWrappedObject(Foo realObject, IFoo wrapper, bool equals) + { + "Given a real object" + .x(() => realObject = new Foo()); + + "And a fake wrapping this object" + .x(() => wrapper = A.Fake(o => o.Wrapping(realObject))); + + "When Equals is called on the fake with the real object as the argument" + .x(() => equals = wrapper.Equals(realObject)); + + "Then it should return false" + .x(() => equals.Should().BeFalse()); + } + + [Scenario] + public static void FakeEqualsFakeWithValueSemantics(Bar realObject, IBar wrapper, bool equals) + { + "Given a real object that overrides Equals with value semantics" + .x(() => realObject = new Bar(42)); + + "And a fake wrapping this object" + .x(() => wrapper = A.Fake(o => o.Wrapping(realObject))); + + "When Equals is called on the fake with itself as the argument" + .x(() => equals = wrapper.Equals(wrapper)); + + "Then it should return true" + .x(() => equals.Should().BeTrue()); + } + + [Scenario] + public static void FakeEqualsWrappedObjectWithValueSemantics(Bar realObject, IBar wrapper, bool equals) + { + "Given a real object that overrides Equals with value semantics" + .x(() => realObject = new Bar(42)); + + "And a fake wrapping this object" + .x(() => wrapper = A.Fake(o => o.Wrapping(realObject))); + + "When Equals is called on the fake with the real object as the argument" + .x(() => equals = wrapper.Equals(realObject)); + + "Then it should return true" + .x(() => equals.Should().BeTrue()); + } + public class Foo : IFoo { public bool NonVoidMethodCalled { get; private set; } @@ -171,5 +240,25 @@ public void OutAndRefMethod(ref int @ref, out int @out) @out = 42; } } + + public class Bar : IBar + { + public Bar(int id) + { + this.Id = id; + } + + public int Id { get; } + + public override bool Equals(object obj) + { + return obj is IBar other && other.Id == this.Id; + } + + public override int GetHashCode() + { + return this.Id.GetHashCode(); + } + } } } From 74a3ef07c8f4f51657b654c0fde921d0dbb3ab1a Mon Sep 17 00:00:00 2001 From: Thomas Levesque Date: Wed, 2 Oct 2019 23:10:10 +0200 Subject: [PATCH 2/2] Refactor IsSameMethod to avoid duplication Also refactor Object's MethodInfos into a new class --- .../Core/FakeManager.ObjectMemberRule.cs | 28 +++++-------------- src/FakeItEasy/Core/MethodInfoManager.cs | 10 +------ src/FakeItEasy/Core/WrappedObjectRule.cs | 12 ++------ src/FakeItEasy/MethodBaseExtensions.cs | 8 ++++++ src/FakeItEasy/ObjectMembers.cs | 12 ++++++++ 5 files changed, 30 insertions(+), 40 deletions(-) create mode 100644 src/FakeItEasy/ObjectMembers.cs diff --git a/src/FakeItEasy/Core/FakeManager.ObjectMemberRule.cs b/src/FakeItEasy/Core/FakeManager.ObjectMemberRule.cs index 75e96ff93..9f65c215c 100644 --- a/src/FakeItEasy/Core/FakeManager.ObjectMemberRule.cs +++ b/src/FakeItEasy/Core/FakeManager.ObjectMemberRule.cs @@ -1,8 +1,6 @@ namespace FakeItEasy.Core { - using System; - using System.Linq; - using System.Reflection; + using static ObjectMembers; /// Object member rule. public partial class FakeManager @@ -10,14 +8,10 @@ public partial class FakeManager private class ObjectMemberRule : SharedFakeObjectCallRule { - private static readonly MethodInfo EqualsMethod = typeof(object).GetMethod(nameof(object.Equals), new[] { typeof(object) }); - private static readonly MethodInfo ToStringMethod = typeof(object).GetMethod(nameof(object.ToString), Type.EmptyTypes); - private static readonly MethodInfo GetHashCodeMethod = typeof(object).GetMethod(nameof(object.GetHashCode), Type.EmptyTypes); - public override bool IsApplicableTo(IFakeObjectCall fakeObjectCall) => - IsSameMethod(fakeObjectCall.Method, EqualsMethod) || - IsSameMethod(fakeObjectCall.Method, ToStringMethod) || - IsSameMethod(fakeObjectCall.Method, GetHashCodeMethod); + fakeObjectCall.Method.IsSameMethodAs(EqualsMethod) || + fakeObjectCall.Method.IsSameMethodAs(ToStringMethod) || + fakeObjectCall.Method.IsSameMethodAs(GetHashCodeMethod); public override void Apply(IInterceptedFakeObjectCall fakeObjectCall) { @@ -38,17 +32,9 @@ public override void Apply(IInterceptedFakeObjectCall fakeObjectCall) } } - private static bool IsSameMethod(MethodInfo first, MethodInfo second) - { - return first.DeclaringType == second.DeclaringType - && first.MetadataToken == second.MetadataToken - && first.Module == second.Module - && first.GetGenericArguments().SequenceEqual(second.GetGenericArguments()); - } - private static bool TryHandleGetHashCode(IInterceptedFakeObjectCall fakeObjectCall, FakeManager fakeManager) { - if (!IsSameMethod(fakeObjectCall.Method, GetHashCodeMethod)) + if (!fakeObjectCall.Method.IsSameMethodAs(GetHashCodeMethod)) { return false; } @@ -60,7 +46,7 @@ private static bool TryHandleGetHashCode(IInterceptedFakeObjectCall fakeObjectCa private static bool TryHandleToString(IInterceptedFakeObjectCall fakeObjectCall, FakeManager fakeManager) { - if (!IsSameMethod(fakeObjectCall.Method, ToStringMethod)) + if (!fakeObjectCall.Method.IsSameMethodAs(ToStringMethod)) { return false; } @@ -72,7 +58,7 @@ private static bool TryHandleToString(IInterceptedFakeObjectCall fakeObjectCall, private static bool TryHandleEquals(IInterceptedFakeObjectCall fakeObjectCall, FakeManager fakeManager) { - if (!IsSameMethod(fakeObjectCall.Method, EqualsMethod)) + if (!fakeObjectCall.Method.IsSameMethodAs(EqualsMethod)) { return false; } diff --git a/src/FakeItEasy/Core/MethodInfoManager.cs b/src/FakeItEasy/Core/MethodInfoManager.cs index f340e25e8..84b5a3a00 100644 --- a/src/FakeItEasy/Core/MethodInfoManager.cs +++ b/src/FakeItEasy/Core/MethodInfoManager.cs @@ -46,7 +46,7 @@ private static bool HasSameBaseMethod(MethodInfo first, MethodInfo second) var baseOfFirst = GetBaseDefinition(first); var baseOfSecond = GetBaseDefinition(second); - return IsSameMethod(baseOfFirst, baseOfSecond); + return baseOfFirst.IsSameMethodAs(baseOfSecond); } private static MethodInfo GetBaseDefinition(MethodInfo method) @@ -59,14 +59,6 @@ private static MethodInfo GetBaseDefinition(MethodInfo method) return method.GetBaseDefinition(); } - private static bool IsSameMethod(MethodInfo first, MethodInfo second) - { - return first.DeclaringType == second.DeclaringType - && first.MetadataToken == second.MetadataToken - && first.Module == second.Module - && first.GetGenericArguments().SequenceEqual(second.GetGenericArguments()); - } - private static MethodInfo FindMethodOnTypeThatWillBeInvokedByMethodInfo(Type type, FakeItEasy.Compatibility.MethodInfoWrapper methodWrapper) { var result = diff --git a/src/FakeItEasy/Core/WrappedObjectRule.cs b/src/FakeItEasy/Core/WrappedObjectRule.cs index a1c9dd41e..6fb002837 100644 --- a/src/FakeItEasy/Core/WrappedObjectRule.cs +++ b/src/FakeItEasy/Core/WrappedObjectRule.cs @@ -1,6 +1,7 @@ namespace FakeItEasy.Core { using System.Reflection; + using static ObjectMembers; /// /// A call rule that applies to any call and just delegates the @@ -9,8 +10,6 @@ namespace FakeItEasy.Core internal class WrappedObjectRule : IFakeObjectCallRule { - private static readonly MethodInfo EqualsMethod = typeof(object).GetMethod(nameof(object.Equals), new[] { typeof(object) }); - private readonly object wrappedObject; /// @@ -56,7 +55,7 @@ public void Apply(IInterceptedFakeObjectCall fakeObjectCall) object returnValue; try { - if (IsCallsToEquals(fakeObjectCall)) + if (fakeObjectCall.Method.IsSameMethodAs(EqualsMethod)) { var arg = parameters[0]; if (ReferenceEquals(arg, fakeObjectCall.FakedObject)) @@ -91,12 +90,5 @@ public void Apply(IInterceptedFakeObjectCall fakeObjectCall) fakeObjectCall.SetReturnValue(returnValue); } - - private static bool IsCallsToEquals(IFakeObjectCall fakeObjectCall) - { - return fakeObjectCall.Method.DeclaringType == EqualsMethod.DeclaringType && - fakeObjectCall.Method.MetadataToken == EqualsMethod.MetadataToken && - fakeObjectCall.Method.Module == EqualsMethod.Module; - } } } diff --git a/src/FakeItEasy/MethodBaseExtensions.cs b/src/FakeItEasy/MethodBaseExtensions.cs index c9338e95c..b87f51d0c 100644 --- a/src/FakeItEasy/MethodBaseExtensions.cs +++ b/src/FakeItEasy/MethodBaseExtensions.cs @@ -56,6 +56,14 @@ public static string GetDescription(this MethodBase method) return builder.ToString(); } + public static bool IsSameMethodAs(this MethodBase method, MethodBase otherMethod) + { + return method.DeclaringType == otherMethod.DeclaringType + && method.MetadataToken == otherMethod.MetadataToken + && method.Module == otherMethod.Module + && method.GetGenericArguments().SequenceEqual(otherMethod.GetGenericArguments()); + } + private static void AppendMethodName(StringBuilder builder, MethodBase method) { if (IsPropertyGetterOrSetter(method)) diff --git a/src/FakeItEasy/ObjectMembers.cs b/src/FakeItEasy/ObjectMembers.cs new file mode 100644 index 000000000..ad58ed3b1 --- /dev/null +++ b/src/FakeItEasy/ObjectMembers.cs @@ -0,0 +1,12 @@ +namespace FakeItEasy +{ + using System; + using System.Reflection; + + internal static class ObjectMembers + { + public static readonly MethodInfo EqualsMethod = typeof(object).GetMethod(nameof(object.Equals), new[] { typeof(object) }); + public static readonly MethodInfo ToStringMethod = typeof(object).GetMethod(nameof(object.ToString), Type.EmptyTypes); + public static readonly MethodInfo GetHashCodeMethod = typeof(object).GetMethod(nameof(object.GetHashCode), Type.EmptyTypes); + } +}