Skip to content

Commit

Permalink
Make stringBuilder.AppendValueOf safer
Browse files Browse the repository at this point in the history
...by only showing the first 10 items of enumerables for certain safe
collection types (arrays and `List<T>`). 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.
  • Loading branch information
stakx committed Mar 10, 2019
1 parent 2604031 commit bb80771
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
32 changes: 17 additions & 15 deletions src/Moq/StringBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
using System.Text;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down

0 comments on commit bb80771

Please sign in to comment.