From aa14724cc0bc64164bd17176904496391834b3c8 Mon Sep 17 00:00:00 2001 From: Dennis Doomen Date: Sat, 14 Jan 2023 16:35:41 +0100 Subject: [PATCH] Improves the detection of base-class hiding properties and fields --- Src/FluentAssertions/Common/TypeExtensions.cs | 38 ++-- .../Matching/MappedMemberMatchingRule.cs | 2 +- .../Matching/MappedPathMatchingRule.cs | 2 +- .../Matching/MustMatchByNameRule.cs | 4 +- .../Matching/TryMatchByNameRule.cs | 4 +- .../Equivalency/MemberFactory.cs | 6 +- .../SelectionRulesSpecs.cs | 181 +++++++++++++++++- docs/_pages/releases.md | 2 + 8 files changed, 208 insertions(+), 31 deletions(-) diff --git a/Src/FluentAssertions/Common/TypeExtensions.cs b/Src/FluentAssertions/Common/TypeExtensions.cs index 0559e7dc7d..dd3d5fa29e 100644 --- a/Src/FluentAssertions/Common/TypeExtensions.cs +++ b/Src/FluentAssertions/Common/TypeExtensions.cs @@ -178,16 +178,19 @@ public static bool OverridesEquals(this Type type) /// /// Returns if no such property exists. /// - public static PropertyInfo FindProperty(this Type type, string propertyName, Type preferredType) + public static PropertyInfo FindProperty(this Type type, string propertyName) { - List properties = - type.GetProperties(AllInstanceMembersFlag) - .Where(pi => pi.Name == propertyName) - .ToList(); + while (type != typeof(object)) + { + if (type.GetProperty(propertyName, AllInstanceMembersFlag | BindingFlags.DeclaredOnly) is { } property) + { + return property; + } + + type = type.BaseType; + } - return properties.Count > 1 - ? properties.SingleOrDefault(p => p.PropertyType == preferredType) - : properties.SingleOrDefault(); + return null; } /// @@ -196,16 +199,19 @@ public static PropertyInfo FindProperty(this Type type, string propertyName, Typ /// /// Returns if no such property exists. /// - public static FieldInfo FindField(this Type type, string fieldName, Type preferredType) + public static FieldInfo FindField(this Type type, string fieldName) { - List properties = - type.GetFields(AllInstanceMembersFlag) - .Where(pi => pi.Name == fieldName) - .ToList(); + while (type != typeof(object)) + { + if (type.GetField(fieldName, AllInstanceMembersFlag | BindingFlags.DeclaredOnly) is { } field) + { + return field; + } + + type = type.BaseType; + } - return properties.Count > 1 - ? properties.SingleOrDefault(p => p.FieldType == preferredType) - : properties.SingleOrDefault(); + return null; } public static IEnumerable GetNonPrivateMembers(this Type typeToReflect, MemberVisibility visibility) diff --git a/Src/FluentAssertions/Equivalency/Matching/MappedMemberMatchingRule.cs b/Src/FluentAssertions/Equivalency/Matching/MappedMemberMatchingRule.cs index f30353b4e4..a0eef4fda0 100644 --- a/Src/FluentAssertions/Equivalency/Matching/MappedMemberMatchingRule.cs +++ b/Src/FluentAssertions/Equivalency/Matching/MappedMemberMatchingRule.cs @@ -35,7 +35,7 @@ public IMember Match(IMember expectedMember, object subject, INode parent, IEqui { if (expectedMember.Name == expectationMemberName) { - var member = MemberFactory.Find(subject, subjectMemberName, expectedMember.Type, parent); + var member = MemberFactory.Find(subject, subjectMemberName, parent); if (member is null) { throw new ArgumentException( diff --git a/Src/FluentAssertions/Equivalency/Matching/MappedPathMatchingRule.cs b/Src/FluentAssertions/Equivalency/Matching/MappedPathMatchingRule.cs index 25a69e1f20..08dd6cedaa 100644 --- a/Src/FluentAssertions/Equivalency/Matching/MappedPathMatchingRule.cs +++ b/Src/FluentAssertions/Equivalency/Matching/MappedPathMatchingRule.cs @@ -50,7 +50,7 @@ public IMember Match(IMember expectedMember, object subject, INode parent, IEqui if (path.IsEquivalentTo(expectedMember.PathAndName)) { - var member = MemberFactory.Find(subject, subjectPath.MemberName, expectedMember.Type, parent); + var member = MemberFactory.Find(subject, subjectPath.MemberName, parent); if (member is null) { throw new ArgumentException( diff --git a/Src/FluentAssertions/Equivalency/Matching/MustMatchByNameRule.cs b/Src/FluentAssertions/Equivalency/Matching/MustMatchByNameRule.cs index 86f950738a..272545cc40 100644 --- a/Src/FluentAssertions/Equivalency/Matching/MustMatchByNameRule.cs +++ b/Src/FluentAssertions/Equivalency/Matching/MustMatchByNameRule.cs @@ -15,13 +15,13 @@ public IMember Match(IMember expectedMember, object subject, INode parent, IEqui if (config.IncludedProperties != MemberVisibility.None) { - PropertyInfo propertyInfo = subject.GetType().FindProperty(expectedMember.Name, expectedMember.Type); + PropertyInfo propertyInfo = subject.GetType().FindProperty(expectedMember.Name); subjectMember = (propertyInfo is not null) && !propertyInfo.IsIndexer() ? new Property(propertyInfo, parent) : null; } if ((subjectMember is null) && config.IncludedFields != MemberVisibility.None) { - FieldInfo fieldInfo = subject.GetType().FindField(expectedMember.Name, expectedMember.Type); + FieldInfo fieldInfo = subject.GetType().FindField(expectedMember.Name); subjectMember = (fieldInfo is not null) ? new Field(fieldInfo, parent) : null; } diff --git a/Src/FluentAssertions/Equivalency/Matching/TryMatchByNameRule.cs b/Src/FluentAssertions/Equivalency/Matching/TryMatchByNameRule.cs index b2030e85df..ff3c71e392 100644 --- a/Src/FluentAssertions/Equivalency/Matching/TryMatchByNameRule.cs +++ b/Src/FluentAssertions/Equivalency/Matching/TryMatchByNameRule.cs @@ -10,13 +10,13 @@ internal class TryMatchByNameRule : IMemberMatchingRule { public IMember Match(IMember expectedMember, object subject, INode parent, IEquivalencyAssertionOptions config) { - PropertyInfo property = subject.GetType().FindProperty(expectedMember.Name, expectedMember.Type); + PropertyInfo property = subject.GetType().FindProperty(expectedMember.Name); if ((property is not null) && !property.IsIndexer()) { return new Property(property, parent); } - FieldInfo field = subject.GetType().FindField(expectedMember.Name, expectedMember.Type); + FieldInfo field = subject.GetType().FindField(expectedMember.Name); return (field is not null) ? new Field(field, parent) : null; } diff --git a/Src/FluentAssertions/Equivalency/MemberFactory.cs b/Src/FluentAssertions/Equivalency/MemberFactory.cs index 8ba09fc650..28c7ff1608 100644 --- a/Src/FluentAssertions/Equivalency/MemberFactory.cs +++ b/Src/FluentAssertions/Equivalency/MemberFactory.cs @@ -21,15 +21,15 @@ public static IMember Create(MemberInfo memberInfo, INode parent) throw new NotSupportedException($"Don't know how to deal with a {memberInfo.MemberType}"); } - internal static IMember Find(object target, string memberName, Type preferredMemberType, INode parent) + internal static IMember Find(object target, string memberName, INode parent) { - PropertyInfo property = target.GetType().FindProperty(memberName, preferredMemberType); + PropertyInfo property = target.GetType().FindProperty(memberName); if ((property is not null) && !property.IsIndexer()) { return new Property(property, parent); } - FieldInfo field = target.GetType().FindField(memberName, preferredMemberType); + FieldInfo field = target.GetType().FindField(memberName); return (field is not null) ? new Field(field, parent) : null; } } diff --git a/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs b/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs index 65fac9013f..d17b109b17 100644 --- a/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs +++ b/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs @@ -530,16 +530,41 @@ public void Ignores_properties_hidden_by_the_derived_class() subject.Should().BeEquivalentTo(expectation); } + [Fact] + public void Ignores_properties_of_the_same_runtime_types_hidden_by_the_derived_class() + { + // Arrange + var subject = new SubclassHidingStringProperty + { + Property = "DerivedValue" + }; + + ((BaseWithStringProperty)subject).Property = "ActualBaseValue"; + + var expectation = new SubclassHidingStringProperty + { + Property = "DerivedValue" + }; + + ((BaseWithStringProperty)expectation).Property = "ExpectedBaseValue"; + + // Act / Assert + subject.Should().BeEquivalentTo(expectation); + } + [Fact] public void Includes_hidden_property_of_the_base_when_using_a_reference_to_the_base() { // Arrange - var subject = new SubclassAHidingProperty + BaseWithProperty subject = new SubclassAHidingProperty { Property = "ActualDerivedValue" }; - ((BaseWithProperty)subject).Property = "BaseValue"; + // FA doesn't know the compile-time type of the subject, so even though we pass a reference to the base-class, + // at run-time, it'll start finding the property on the subject starting from the run-time type, and thus ignore the + // hidden base-class field + ((SubclassAHidingProperty)subject).Property = "BaseValue"; AnotherBaseWithProperty expectation = new SubclassBHidingProperty { @@ -617,28 +642,172 @@ public void Excluding_the_property_hiding_the_base_class_one_does_not_reveal_the act.Should().Throw().WithMessage("*No members were found *"); } - public class BaseWithProperty + private class BaseWithProperty { public object Property { get; set; } } - public class SubclassAHidingProperty : BaseWithProperty + private class SubclassAHidingProperty : BaseWithProperty { public new T Property { get; set; } } - public class AnotherBaseWithProperty + private class BaseWithStringProperty + { + public string Property { get; set; } + } + + private class SubclassHidingStringProperty : BaseWithStringProperty + { + public new string Property { get; set; } + } + + private class AnotherBaseWithProperty { public object Property { get; set; } } - public class SubclassBHidingProperty : AnotherBaseWithProperty + private class SubclassBHidingProperty : AnotherBaseWithProperty { public new T Property { get; set; } } + + [Fact] + public void Ignores_fields_hidden_by_the_derived_class() + { + // Arrange + var subject = new SubclassAHidingField + { + Field = "DerivedValue" + }; + + ((BaseWithField)subject).Field = "ActualBaseValue"; + + var expectation = new SubclassBHidingField + { + Field = "DerivedValue" + }; + + ((AnotherBaseWithField)expectation).Field = "ExpectedBaseValue"; + + // Act / Assert + subject.Should().BeEquivalentTo(expectation, options => options.IncludingFields()); + } + + [Fact] + public void Includes_hidden_field_of_the_base_when_using_a_reference_to_the_base() + { + // Arrange + BaseWithField subject = new SubclassAHidingField + { + Field = "BaseValueFromSubject" + }; + + // FA doesn't know the compile-time type of the subject, so even though we pass a reference to the base-class, + // at run-time, it'll start finding the field on the subject starting from the run-time type, and thus ignore the + // hidden base-class field + ((SubclassAHidingField)subject).Field = "BaseValueFromExpectation"; + + AnotherBaseWithField expectation = new SubclassBHidingField + { + Field = "ExpectedDerivedValue" + }; + + expectation.Field = "BaseValueFromExpectation"; + + // Act / Assert + subject.Should().BeEquivalentTo(expectation, options => options.IncludingFields()); + } + + [Fact] + public void Run_type_typing_ignores_hidden_fields_even_when_using_a_reference_to_the_base_class() + { + // Arrange + var subject = new SubclassAHidingField + { + Field = "DerivedValue" + }; + + ((BaseWithField)subject).Field = "ActualBaseValue"; + + AnotherBaseWithField expectation = new SubclassBHidingField + { + Field = "DerivedValue" + }; + + expectation.Field = "ExpectedBaseValue"; + + // Act / Assert + subject.Should().BeEquivalentTo(expectation, options => options.IncludingFields().RespectingRuntimeTypes()); + } + + [Fact] + public void Including_the_derived_field_excludes_the_hidden_field() + { + // Arrange + var subject = new SubclassAHidingField + { + Field = "DerivedValue" + }; + + ((BaseWithField)subject).Field = "ActualBaseValue"; + + var expectation = new SubclassBHidingField + { + Field = "DerivedValue" + }; + + ((AnotherBaseWithField)expectation).Field = "ExpectedBaseValue"; + + // Act / Assert + subject.Should().BeEquivalentTo(expectation, options => options + .IncludingFields() + .Including(_ => _.Field)); + } + + [Fact] + public void Excluding_the_field_hiding_the_base_class_one_does_not_reveal_the_latter() + { + // Arrange + var subject = new SubclassAHidingField(); + + ((BaseWithField)subject).Field = "ActualBaseValue"; + + var expectation = new SubclassBHidingField(); + + ((AnotherBaseWithField)expectation).Field = "ExpectedBaseValue"; + + // Act + Action act = () => subject.Should().BeEquivalentTo(expectation, options => options + .IncludingFields() + .Excluding(b => b.Field)); + + // Assert + act.Should().Throw().WithMessage("*No members were found *"); + } + + private class BaseWithField + { + public string Field; + } + + private class SubclassAHidingField : BaseWithField + { + public new string Field; + } + + private class AnotherBaseWithField + { + public string Field; + } + + private class SubclassBHidingField : AnotherBaseWithField + { + public new string Field; + } } [Fact] diff --git a/docs/_pages/releases.md b/docs/_pages/releases.md index 1571d3542b..d3cefc6b07 100644 --- a/docs/_pages/releases.md +++ b/docs/_pages/releases.md @@ -13,6 +13,8 @@ sidebar: ### Fixes +* `BeEquivalentTo` no longer crashes on fields hiding base-class fields - [#1990](https://github.com/fluentassertions/fluentassertions/pull/1990) + ## 6.9.0 ### What's new