From c05137ac776103c2ce852b18290e4e05009f2f7a Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Wed, 16 Mar 2022 05:49:46 -0500 Subject: [PATCH] Changed ExcludeNonBrowsable in IEquivalencyAssertionOptions into separate properties IgnoreNonBrowsableOnSubject and ExcludeNonBrowsableOnExpectation. Updated implementations of the interface accordingly. Added new selection rule ExcludeNonBrowsableMembersRule.cs and updated SelfReferenceEquivalencyAssertionOptions.SelectionRules to inject an instance of it after AllPropertiesSelectionRule and AllFieldsSelectionRule but before explicit member selection rules. Updated the ToString implementation in SelfReferenceEquivalencyAssertionOptions.cs to consider ignoreNonBrowsableOnSubject and excludeNonBrowsableOnExpectation independently. Removed processing of options.ExcludeNonBroswable from StructuralEqualityEquivalencyStep's GetMembersFromExpectation, since this is now handled by selection rules. Added tests that exercise the situation where a non-browsable field or property is explicitly included after a call to ExcludingNonBrowsableMembers. The current logic actually doesn't provide a way to do a wildcard inclusion of all properties or fields if any Include call targeting a specific member is included, but the tests have been written so that they'll continue to effectively test this function if that changes in the future. Adjusted existing tests in SelectionRulesSpecs.cs to use IgnoringNonBrowsableMembersOnSubject where appropriate, and to verify that ExcludingNonBrowsableMembers does _not_ cause non-browsable members on the subject to be ignored. --- ...llectionMemberAssertionOptionsDecorator.cs | 4 +- .../IEquivalencyAssertionOptions.cs | 10 ++- .../ExcludeNonBrowsableMembersRule.cs | 16 ++++ ...elfReferenceEquivalencyAssertionOptions.cs | 48 +++++++--- .../StructuralEqualityEquivalencyStep.cs | 11 +-- .../UsersOfGetClosedGenericInterfaces.cs | 4 +- .../SelectionRulesSpecs.cs | 90 ++++++++++++++++++- 7 files changed, 151 insertions(+), 32 deletions(-) create mode 100644 Src/FluentAssertions/Equivalency/Selection/ExcludeNonBrowsableMembersRule.cs diff --git a/Src/FluentAssertions/Equivalency/Execution/CollectionMemberAssertionOptionsDecorator.cs b/Src/FluentAssertions/Equivalency/Execution/CollectionMemberAssertionOptionsDecorator.cs index 868c0ba910..86ec59e34a 100644 --- a/Src/FluentAssertions/Equivalency/Execution/CollectionMemberAssertionOptionsDecorator.cs +++ b/Src/FluentAssertions/Equivalency/Execution/CollectionMemberAssertionOptionsDecorator.cs @@ -61,7 +61,9 @@ public IEnumerable UserEquivalencySteps public MemberVisibility IncludedFields => inner.IncludedFields; - public bool ExcludeNonBrowsable => inner.ExcludeNonBrowsable; + public bool IgnoreNonBrowsableOnSubject => inner.IgnoreNonBrowsableOnSubject; + + public bool ExcludeNonBrowsableOnExpectation => inner.ExcludeNonBrowsableOnExpectation; public bool CompareRecordsByValue => inner.CompareRecordsByValue; diff --git a/Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs b/Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs index fa6c28021d..114bad0f8d 100644 --- a/Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs +++ b/Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs @@ -74,10 +74,16 @@ public interface IEquivalencyAssertionOptions MemberVisibility IncludedFields { get; } /// - /// Gets a value indicating whether members marked with [EditorBrowsable] + /// Gets a value indicating whether members on the subject marked with [EditorBrowsable] + /// and an EditorBrowsableState of Never should be treated as though they don't exist. + /// + bool IgnoreNonBrowsableOnSubject { get; } + + /// + /// Gets a value indicating whether members on the expectation marked with [EditorBrowsable] /// and an EditorBrowsableState of Never should be excluded. /// - bool ExcludeNonBrowsable { get; } + bool ExcludeNonBrowsableOnExpectation { get; } /// /// Gets a value indicating whether records should be compared by value instead of their members diff --git a/Src/FluentAssertions/Equivalency/Selection/ExcludeNonBrowsableMembersRule.cs b/Src/FluentAssertions/Equivalency/Selection/ExcludeNonBrowsableMembersRule.cs new file mode 100644 index 0000000000..7060c2a45e --- /dev/null +++ b/Src/FluentAssertions/Equivalency/Selection/ExcludeNonBrowsableMembersRule.cs @@ -0,0 +1,16 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace FluentAssertions.Equivalency.Selection +{ + internal class ExcludeNonBrowsableMembersRule : IMemberSelectionRule + { + public bool IncludesMembers => false; + + public IEnumerable SelectMembers(INode currentNode, IEnumerable selectedMembers, MemberSelectionContext context) + { + return selectedMembers.Where(member => member.IsBrowsable); + } + } +} diff --git a/Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyAssertionOptions.cs b/Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyAssertionOptions.cs index 742126d2c3..2500f8f3b0 100644 --- a/Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyAssertionOptions.cs +++ b/Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyAssertionOptions.cs @@ -56,7 +56,8 @@ public abstract class SelfReferenceEquivalencyAssertionOptions : IEquival private MemberVisibility includedProperties; private MemberVisibility includedFields; - private bool excludeNonBrowsable; + private bool ignoreNonBrowsableOnSubject; + private bool excludeNonBrowsableOnExpectation; private bool compareRecordsByValue; @@ -81,7 +82,8 @@ protected SelfReferenceEquivalencyAssertionOptions(IEquivalencyAssertionOptions useRuntimeTyping = defaults.UseRuntimeTyping; includedProperties = defaults.IncludedProperties; includedFields = defaults.IncludedFields; - excludeNonBrowsable = defaults.ExcludeNonBrowsable; + ignoreNonBrowsableOnSubject = defaults.IgnoreNonBrowsableOnSubject; + excludeNonBrowsableOnExpectation = defaults.ExcludeNonBrowsableOnExpectation; compareRecordsByValue = defaults.CompareRecordsByValue; ConversionSelector = defaults.ConversionSelector.Clone(); @@ -117,6 +119,11 @@ IEnumerable IEquivalencyAssertionOptions.SelectionRules yield return new AllFieldsSelectionRule(); } + if (excludeNonBrowsableOnExpectation) + { + yield return new ExcludeNonBrowsableMembersRule(); + } + foreach (IMemberSelectionRule rule in selectionRules) { yield return rule; @@ -164,7 +171,9 @@ IEnumerable IEquivalencyAssertionOptions.SelectionRules MemberVisibility IEquivalencyAssertionOptions.IncludedFields => includedFields; - bool IEquivalencyAssertionOptions.ExcludeNonBrowsable => excludeNonBrowsable; + bool IEquivalencyAssertionOptions.IgnoreNonBrowsableOnSubject => ignoreNonBrowsableOnSubject; + + bool IEquivalencyAssertionOptions.ExcludeNonBrowsableOnExpectation => excludeNonBrowsableOnExpectation; public bool CompareRecordsByValue => compareRecordsByValue; @@ -322,7 +331,7 @@ public TSelf ExcludingProperties() /// public TSelf IncludingNonBrowsableMembers() { - excludeNonBrowsable = false; + excludeNonBrowsableOnExpectation = false; return (TSelf)this; } @@ -334,7 +343,13 @@ public TSelf IncludingNonBrowsableMembers() /// public TSelf ExcludingNonBrowsableMembers() { - excludeNonBrowsable = true; + excludeNonBrowsableOnExpectation = true; + return (TSelf)this; + } + + public TSelf IgnoringNonBrowsableMembersOnSubject() + { + ignoreNonBrowsableOnSubject = true; return (TSelf)this; } @@ -724,6 +739,11 @@ public override string ToString() .Append(useRuntimeTyping ? "runtime" : "declared") .AppendLine(" types and members"); + if (ignoreNonBrowsableOnSubject) + { + builder.AppendLine("- Do not consider members marked non-browsable on the subject"); + } + if (isRecursive) { if (allowInfiniteRecursion) @@ -753,15 +773,6 @@ public override string ToString() builder.AppendLine("- Compare records by their members"); } - if (excludeNonBrowsable) - { - builder.AppendLine("- Exclude non-browsable members"); - } - else - { - builder.AppendLine("- Include non-browsable members"); - } - foreach (Type valueType in valueTypes) { builder.AppendLine($"- Compare {valueType} by value"); @@ -772,6 +783,15 @@ public override string ToString() builder.AppendLine($"- Compare {type} by its members"); } + if (excludeNonBrowsableOnExpectation) + { + builder.AppendLine("- Exclude non-browsable members"); + } + else + { + builder.AppendLine("- Include non-browsable members"); + } + foreach (IMemberSelectionRule rule in selectionRules) { builder.Append("- ").AppendLine(rule.ToString()); diff --git a/Src/FluentAssertions/Equivalency/Steps/StructuralEqualityEquivalencyStep.cs b/Src/FluentAssertions/Equivalency/Steps/StructuralEqualityEquivalencyStep.cs index 2e9e37528f..382b686d73 100644 --- a/Src/FluentAssertions/Equivalency/Steps/StructuralEqualityEquivalencyStep.cs +++ b/Src/FluentAssertions/Equivalency/Steps/StructuralEqualityEquivalencyStep.cs @@ -82,9 +82,7 @@ public class StructuralEqualityEquivalencyStep : IEquivalencyStep where match is not null select match; - // This excludes non-browsable members from the subject. If a member is marked non-browsable - // in the expectation, it gets excluded in GetMembersFromExpectation. - if (config.ExcludeNonBrowsable) + if (config.IgnoreNonBrowsableOnSubject) { query = query.Where(member => member.IsBrowsable); } @@ -103,13 +101,6 @@ public class StructuralEqualityEquivalencyStep : IEquivalencyStep new MemberSelectionContext(comparands.CompileTimeType, comparands.RuntimeType, options)); } - // This excludes non-browsable members from the expectation. If a member is marked non-browsable - // in the subject, it gets excluded in FindMatchFor. - if (options.ExcludeNonBrowsable) - { - members = members.Where(member => member.IsBrowsable); - } - return members; } } diff --git a/Tests/Benchmarks/UsersOfGetClosedGenericInterfaces.cs b/Tests/Benchmarks/UsersOfGetClosedGenericInterfaces.cs index c95bf80054..7a285dea61 100644 --- a/Tests/Benchmarks/UsersOfGetClosedGenericInterfaces.cs +++ b/Tests/Benchmarks/UsersOfGetClosedGenericInterfaces.cs @@ -69,7 +69,9 @@ private class Config : IEquivalencyAssertionOptions public MemberVisibility IncludedFields => throw new NotImplementedException(); - public bool ExcludeNonBrowsable => throw new NotImplementedException(); + public bool IgnoreNonBrowsableOnSubject => throw new NotImplementedException(); + + public bool ExcludeNonBrowsableOnExpectation => throw new NotImplementedException(); public bool CompareRecordsByValue => throw new NotImplementedException(); diff --git a/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs b/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs index cd07a0af9d..510265e6ae 100644 --- a/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs +++ b/Tests/FluentAssertions.Equivalency.Specs/SelectionRulesSpecs.cs @@ -1724,14 +1724,29 @@ public void When_non_browsable_property_differs_excluding_non_browsable_members_ } [Fact] - public void When_property_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_make_it_succeed() + public void When_property_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_not_make_it_succeed() + { + // Arrange + var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { PropertyThatMightBeNonBrowsable = 0 }; + var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { PropertyThatMightBeNonBrowsable = 1 }; + + // Act + Action action = + () => subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers()); + + // Assert + action.Should().Throw().WithMessage("Expected property subject.PropertyThatMightBeNonBrowsable to be 1, but found 0.*"); + } + + [Fact] + public void When_property_is_non_browsable_only_in_subject_ignoring_non_browsable_members_on_subject_should_make_it_succeed() { // Arrange var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { PropertyThatMightBeNonBrowsable = 0 }; var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { PropertyThatMightBeNonBrowsable = 1 }; // Act & Assert - subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers()); + subject.Should().BeEquivalentTo(expectation, config => config.IgnoringNonBrowsableMembersOnSubject()); } [Fact] @@ -1746,14 +1761,29 @@ public void When_property_is_non_browsable_only_in_expectation_excluding_non_bro } [Fact] - public void When_field_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_make_it_succeed() + public void When_field_is_non_browsable_only_in_subject_excluding_non_browsable_members_should_not_make_it_succeed() + { + // Arrange + var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { FieldThatMightBeNonBrowsable = 0 }; + var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { FieldThatMightBeNonBrowsable = 1 }; + + // Act + Action action = + () => subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers()); + + // Assert + action.Should().Throw().WithMessage("Expected field subject.FieldThatMightBeNonBrowsable to be 1, but found 0.*"); + } + + [Fact] + public void When_field_is_non_browsable_only_in_subject_ignoring_non_browsable_members_on_subject_should_make_it_succeed() { // Arrange var subject = new ClassWhereMemberThatCouldBeNonBrowsableIsNonBrowsable() { FieldThatMightBeNonBrowsable = 0 }; var expectation = new ClassWhereMemberThatCouldBeNonBrowsableIsBrowsable() { FieldThatMightBeNonBrowsable = 1 }; // Act & Assert - subject.Should().BeEquivalentTo(expectation, config => config.ExcludingNonBrowsableMembers()); + subject.Should().BeEquivalentTo(expectation, config => config.IgnoringNonBrowsableMembersOnSubject()); } [Fact] @@ -1904,6 +1934,58 @@ public void When_field_is_missing_from_expectation_excluding_non_browsable_membe // Act & Assert subject.Should().BeEquivalentTo(expected, opt => opt.ExcludingNonBrowsableMembers()); } + + [Fact] + public void When_non_browsable_members_are_excluded_it_should_still_be_possible_to_explicitly_include_non_browsable_field() + { + // Arrange + var subject = + new ClassWithNonBrowsableMembers() + { + NonBrowsableField = 1, + }; + + var expectation = + new ClassWithNonBrowsableMembers() + { + NonBrowsableField = 2, + }; + + // Act + Action action = + () => subject.Should().BeEquivalentTo( + expectation, + opt => opt.IncludingFields().ExcludingNonBrowsableMembers().Including(e => e.NonBrowsableField)); + + // Assert + action.Should().Throw().WithMessage("Expected field subject.NonBrowsableField to be 2, but found 1.*"); + } + + [Fact] + public void When_non_browsable_members_are_excluded_it_should_still_be_possible_to_explicitly_include_non_browsable_property() + { + // Arrange + var subject = + new ClassWithNonBrowsableMembers() + { + NonBrowsableProperty = 1, + }; + + var expectation = + new ClassWithNonBrowsableMembers() + { + NonBrowsableProperty = 2, + }; + + // Act + Action action = + () => subject.Should().BeEquivalentTo( + expectation, + opt => opt.IncludingProperties().ExcludingNonBrowsableMembers().Including(e => e.NonBrowsableProperty)); + + // Assert + action.Should().Throw().WithMessage("Expected property subject.NonBrowsableProperty to be 2, but found 1.*"); + } } private class ClassWithNonBrowsableMembers