From e8be030e73d19e82daaa9bfd472c3401bd62439e Mon Sep 17 00:00:00 2001 From: stakx Date: Sun, 10 Mar 2019 15:11:50 +0100 Subject: [PATCH] Make `stringBuilder.AppendValueOf` safer ...by only showing the first 10 items of enumerables for certain safe collection types (arrays and `List`). It cannot be generally assum- ed that `IEnumerable` actually work. I cannot easily add the test case provided in #741 to the regression test suite as it relies on EF Core, and the test project already uses EF, so we'd likely have type conflicts if both of them are referenced. --- CHANGELOG.md | 1 + src/Moq/StringBuilderExtensions.cs | 32 ++++++++++++++++-------------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e36ffd3e4..a80a4f5d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ This release contains several **minor breaking changes**. Please review your cod * Use of `SetupSet` 'forgets' method setup (@TimothyHayes, #432) * Recursive mocks don't work with argument matching (@thalesmello, #142) * Recursive property setup overrides previous setups (@jamesfoster, #110) +* Formatting of enumerable object for error message broke EF Core test case (@MichaelSagalovich, #741) ## 4.10.1 (2018-12-03) diff --git a/src/Moq/StringBuilderExtensions.cs b/src/Moq/StringBuilderExtensions.cs index fa7480d11..1e2b94cc9 100644 --- a/src/Moq/StringBuilderExtensions.cs +++ b/src/Moq/StringBuilderExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Diagnostics; using System.Reflection; using System.Text; @@ -82,14 +83,15 @@ public static StringBuilder AppendValueOf(this StringBuilder stringBuilder, obje { stringBuilder.Append(d.ToString("G17")); } - else if (obj is IEnumerable enumerable && !(obj is IMocked)) - { // ^^^^^^^^^^^^^^^^^ - // This second check ensures that we have a usable implementation of IEnumerable. - // If value is a mocked object, its IEnumerable implementation might very well - // not work correctly. + else if (obj.GetType().IsEnum) + { + stringBuilder.AppendNameOf(obj.GetType()).Append('.').Append(obj); + } + else if (obj.GetType().IsArray || (obj.GetType().IsConstructedGenericType && obj.GetType().GetGenericTypeDefinition() == typeof(List<>))) + { stringBuilder.Append('['); const int maxCount = 10; - var enumerator = enumerable.GetEnumerator(); + var enumerator = ((IEnumerable)obj).GetEnumerator(); for (int i = 0; enumerator.MoveNext() && i < maxCount + 1; ++i) { if (i > 0) @@ -107,17 +109,17 @@ public static StringBuilder AppendValueOf(this StringBuilder stringBuilder, obje } stringBuilder.Append(']'); } - else if (obj.ToString() == obj.GetType().ToString()) - { - stringBuilder.AppendNameOf(obj.GetType()); - } - else if (obj.GetType().IsEnum) - { - stringBuilder.AppendNameOf(obj.GetType()).Append('.').Append(obj); - } else { - stringBuilder.Append(obj.ToString()); + var formatted = obj.ToString(); + if (formatted == null || formatted == obj.GetType().ToString()) + { + stringBuilder.AppendNameOf(obj.GetType()); + } + else + { + stringBuilder.Append(formatted); + } } return stringBuilder;