Fix multiplied ConversionSelector's descriptions appearing in asserti…

…on message (#941)

* Fix ConversionSelector's cloning of the description

* Make ConvertionSelector immutable and injectable into IEquivalencyAssertionOptions.GetUserEquivalencySteps

* Use chaining in newly added test

* Add test for case od displaying "Without automatic conversion"

* Add test checking whole message with description of all the equivalency steps

Src/FluentAssertions/Equivalency/CollectionMemberAssertionOptionsDecorator.cs

@@ -39,9 +39,9 @@ public OrderingRuleCollection OrderingRules
 
         public ConversionSelector ConversionSelector => inner.ConversionSelector;
 
-        public IEnumerable<IEquivalencyStep> UserEquivalencySteps
+        public IEnumerable<IEquivalencyStep> GetUserEquivalencySteps(ConversionSelector conversionSelector)
         {
-            get { return inner.UserEquivalencySteps.Select(step => new CollectionMemberAssertionRuleDecorator(step)).ToArray(); }
+            return inner.GetUserEquivalencySteps(conversionSelector).Select(step => new CollectionMemberAssertionRuleDecorator(step)).ToArray();
         }
 
         public bool IsRecursive => inner.IsRecursive;

Src/FluentAssertions/Equivalency/ConversionSelector.cs

@@ -3,7 +3,6 @@
 using System.Linq;
 using System.Linq.Expressions;
 using System.Text;
-using FluentAssertions.Execution;
 
 namespace FluentAssertions.Equivalency
 {
@@ -12,46 +11,72 @@ namespace FluentAssertions.Equivalency
     /// </summary>
     public class ConversionSelector
     {
-        private List<Func<IMemberInfo, bool>> inclusions = new List<Func<IMemberInfo, bool>>();
-        private List<Func<IMemberInfo, bool>> exclusions = new List<Func<IMemberInfo, bool>>();
-        private StringBuilder description = new StringBuilder();
+        private class ConversionSelectorRule
+        {
+            public Func<IMemberInfo, bool> Predicate { get; }
+            public string Description { get; }
+
+            public ConversionSelectorRule(Func<IMemberInfo, bool> predicate, string description)
+            {
+                Predicate = predicate;
+                Description = description;
+            }
+        }
+        private List<ConversionSelectorRule> inclusions = new List<ConversionSelectorRule>();
+        private List<ConversionSelectorRule> exclusions = new List<ConversionSelectorRule>();
 
         public void IncludeAll()
         {
-            inclusions.Add(_ => true);
-            description.Append("Try conversion of all members. ");
+            inclusions.Add(new ConversionSelectorRule(_ => true, "Try conversion of all members. "));
         }
 
         public void Include(Expression<Func<IMemberInfo, bool>> predicate)
         {
-            inclusions.Add(predicate.Compile());
-            description.Append("Try conversion of member ").Append(predicate.Body).Append(". ");
+            inclusions.Add(new ConversionSelectorRule(
+                predicate.Compile(),
+                $"Try conversion of member {predicate.Body}. "));
         }
 
         public void Exclude(Expression<Func<IMemberInfo, bool>> predicate)
         {
-            exclusions.Add(predicate.Compile());
-            description.Append("Do not convert member ").Append(predicate.Body).Append(". ");
+            exclusions.Add(new ConversionSelectorRule(
+                predicate.Compile(),
+                $"Do not convert member {predicate.Body}."));
         }
 
         public bool RequiresConversion(IMemberInfo info)
         {
-            return inclusions.Any(p => p(info)) && !exclusions.Any(p => p(info));
+            return inclusions.Any(p => p.Predicate(info)) && !exclusions.Any(p => p.Predicate(info));
         }
 
         public override string ToString()
         {
-            string result = description.ToString();
-            return (result.Length > 0) ? result : "Without automatic conversion.";
+            if(inclusions.Count == 0 && exclusions.Count == 0)
+            {
+                return "Without automatic conversion.";
+            }
+
+            StringBuilder descriptionBuilder = new StringBuilder();
+
+            foreach (var inclusion in inclusions)
+            {
+                descriptionBuilder.Append(inclusion.Description);
+            }
+
+            foreach (var exclusion in exclusions)
+            {
+                descriptionBuilder.Append(exclusion.Description);
+            }
+
+            return descriptionBuilder.ToString();
         }
 
         public ConversionSelector Clone()
         {
             return new ConversionSelector
             {
-                inclusions = new List<Func<IMemberInfo, bool>>(inclusions),
-                exclusions = new List<Func<IMemberInfo, bool>>(exclusions),
-                description = description
+                inclusions = new List<ConversionSelectorRule>(inclusions),
+                exclusions = new List<ConversionSelectorRule>(exclusions),
             };
         }
     }

Src/FluentAssertions/Equivalency/IEquivalencyAssertionOptions.cs

@@ -53,7 +53,7 @@ public interface IEquivalencyAssertionOptions
         /// <summary>
         /// Gets an ordered collection of Equivalency steps how a subject is compared with the expectation.
         /// </summary>
-        IEnumerable<IEquivalencyStep> UserEquivalencySteps { get; }
+        IEnumerable<IEquivalencyStep> GetUserEquivalencySteps(ConversionSelector conversionSelector);
 
         /// <summary>
         /// Gets a value indicating whether the runtime type of the expectation should be used rather than the declared type.

Src/FluentAssertions/Equivalency/RunAllUserStepsEquivalencyStep.cs

@@ -10,13 +10,13 @@ public class RunAllUserStepsEquivalencyStep : IEquivalencyStep
     {
         public bool CanHandle(IEquivalencyValidationContext context, IEquivalencyAssertionOptions config)
         {
-            return config.UserEquivalencySteps.Any(s => s.CanHandle(context, config));
+            return config.GetUserEquivalencySteps(config.ConversionSelector).Any(s => s.CanHandle(context, config));
         }
 
         public bool Handle(IEquivalencyValidationContext context, IEquivalencyValidator parent,
             IEquivalencyAssertionOptions config)
         {
-            return config.UserEquivalencySteps
+            return config.GetUserEquivalencySteps(config.ConversionSelector)
                 .Any(step => step.CanHandle(context, config) && step.Handle(context, parent, config));
         }
     }

Src/FluentAssertions/Equivalency/SelfReferenceEquivalencyAssertionOptions.cs

@@ -77,11 +77,12 @@ protected SelfReferenceEquivalencyAssertionOptions(IEquivalencyAssertionOptions
             includeProperties = defaults.IncludeProperties;
             includeFields = defaults.IncludeFields;
 
+            ConversionSelector = defaults.ConversionSelector.Clone();
+
             selectionRules.AddRange(defaults.SelectionRules);
-            userEquivalencySteps.AddRange(defaults.UserEquivalencySteps);
+            userEquivalencySteps.AddRange(defaults.GetUserEquivalencySteps(ConversionSelector));
             matchingRules.AddRange(defaults.MatchingRules);
             orderingRules = new OrderingRuleCollection(defaults.OrderingRules);
-            ConversionSelector = defaults.ConversionSelector.Clone();
 
             getDefaultEqualityStrategy = defaults.GetEqualityStrategy;
             TraceWriter = defaults.TraceWriter;
@@ -125,8 +126,8 @@ IEnumerable<IMemberSelectionRule> IEquivalencyAssertionOptions.SelectionRules
         /// <summary>
         ///     Gets an ordered collection of Equivalency steps how a subject is compared with the expectation.
         /// </summary>
-        IEnumerable<IEquivalencyStep> IEquivalencyAssertionOptions.UserEquivalencySteps =>
-            userEquivalencySteps.Concat(new[] { new TryConversionStep(ConversionSelector) });
+        IEnumerable<IEquivalencyStep> IEquivalencyAssertionOptions.GetUserEquivalencySteps(ConversionSelector convertionSelector) =>
+            userEquivalencySteps.Concat(new[] {new TryConversionStep(convertionSelector) });
 
         public ConversionSelector ConversionSelector { get; } = new ConversionSelector();
 

Src/FluentAssertions/Primitives/ObjectAssertions.cs

@@ -12,7 +12,6 @@ namespace FluentAssertions.Primitives
     /// <summary>
     /// Contains a number of methods to assert that an <see cref="object"/> is in the expected state.
     /// </summary>
-    [DebuggerNonUserCode]
     public class ObjectAssertions : ReferenceTypeAssertions<object, ObjectAssertions>
     {
         public ObjectAssertions(object value)

Tests/Shared.Specs/BasicEquivalencySpecs.cs

@@ -2581,7 +2581,10 @@ public void When_a_specific_mismatching_property_is_excluded_from_conversion_it_
             //-----------------------------------------------------------------------------------------------------------
             // Assert
             //-----------------------------------------------------------------------------------------------------------
-            act.Should().Throw<XunitException>().WithMessage("Expected*<1973-09-20>*\"1973-09-20\"*");
+            act.Should().Throw<XunitException>().Which.Message
+                .Should().Match("Expected*<1973-09-20>*\"1973-09-20\"*", "{0} field is of mismatched type", nameof(expectation.Birthdate))
+                .And.Subject.Should().Match("*Try conversion of all members*", "conversion description should be present")
+                .And.Subject.Should().NotMatch("*Try conversion of all members*Try conversion of all members*", "conversion description should not be duplicated");
         }
 
         [Fact]
@@ -2686,7 +2689,7 @@ public void When_the_expectation_contains_a_nested_null_it_should_properly_repor
             // Assert
             //-----------------------------------------------------------------------------------------------------------
             act.Should().Throw<XunitException>()
-                .WithMessage("*Expected*Level.Level to be <null>, but found*Level2*");
+                .WithMessage("*Expected*Level.Level to be <null>, but found*Level2*Without automatic conversion*");
         }
 
         [Fact]
@@ -2787,9 +2790,12 @@ public void When_not_all_the_properties_of_the_nested_objects_are_equal_it_shoul
             // Assert
             //-----------------------------------------------------------------------------------------------------------
             act
-                .Should().Throw<XunitException>()
-                .WithMessage(
-                    "Expected member Level.Text to be \"Level2\", but \"Level1\" differs near \"1\" (index 5)*");
+                .Should().Throw<XunitException>().Which.Message
+                // Checking exception message exactly is against general guidelines
+                // but in that case it was done on purpose, so that we have at least single
+                // test confirming that whole mechanism of gathering description from
+                // equivalency steps works.
+                .Should().Be("Expected member Level.Text to be \"Level2\", but \"Level1\" differs near \"1\" (index 5).\r\n\nWith configuration:\n- Use declared types and members\r\n- Compare enums by value\r\n- Match member by name (or throw)\r\n- Without automatic conversion.\r\n- Be strict about the order of items in byte arrays\r\n");
         }
 
         [Fact]