ArgumentOutOfRangeException when formatting objects using UseLineBreaks

[jnyrup]

Description

When using ReferenceTypeAssertions.BeSameAs and failing I got a ArgumentOutOfRangeException instead of an XunitException.

From the stack trace the problem lies somewhere in the formatting of the subject combined with using UseLineBreaks.

Only a few assertions specify UseLineBreaks per default, which is why we might not have seen this before.

Reproduction Steps

[Fact]
public void Array_of_arrays_of_toStringable_objects()
{
    var sigmets = new Point[][]
    {
        [new Point()],
        [new Point()],
    };

    Formatter.ToString(sigmets, new() { UseLineBreaks = true });
}

[Fact]
public void Array_of_objects_with_array_of_toStringable_objects()
{
    var sigmets = new[]
    {
        new
        {
            Points = new Point[] { new() }
        },
        new
        {
            Points = new Point[] { new() }
        }
    };

    Formatter.ToString(sigmets, new() { UseLineBreaks = true });
}

private class Point
{
    public override string ToString() => "P";
}

Expected behavior

Both tests should be able to complete without throwing an exception

Actual behavior

Array_of_arrays_of_toStringable_objects:

Message: 
Test method TestProject30.UnitTest1.Array_of_arrays_of_toStringable_objects threw exception: 
System.ArgumentOutOfRangeException: startIndex ('3') must be less than or equal to '0'. (Parameter 'startIndex')
Actual value was 3.

  Stack Trace: 
ArgumentOutOfRangeException.ThrowGreater[T](T value, T other, String paramName)
String.Insert(Int32 startIndex, String value)
PossibleMultilineFragment.InsertLineOrFragment(String fragment)
EnumerableValueFormatter.Format(Object value, FormattedObjectGraph formattedGraph, FormattingContext context, FormatChild formatChild)
Formatter.Format(Object value, FormattedObjectGraph output, FormattingContext context, FormatChild formatChild)
Formatter.ToString(Object value, FormattingOptions options)
UnitTest1.Array_of_arrays_of_toStringable_objects() line 17
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Array_of_objects_with_array_of_toStringable_objects:

Message: 
Test method TestProject30.UnitTest1.Array_of_objects_with_array_of_toStringable_objects threw exception: 
System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')

  Stack Trace: 
List`1.get_Item(Int32 index)
PossibleMultilineFragment.InsertAtStartOfLine(Int32 lineIndex, String insertion)
PossibleMultilineFragment.AddStartingLineOrFragment(String fragment)
EnumerableValueFormatter.Format(Object value, FormattedObjectGraph formattedGraph, FormattingContext context, FormatChild formatChild)
Formatter.Format(Object value, FormattedObjectGraph output, FormattingContext context, FormatChild formatChild)
Formatter.FormatChild(String path, Object value, FormattedObjectGraph output, FormattingContext context, FormattingOptions options, ObjectGraph graph)
<>c__DisplayClass6_0.<FormatChild>b__0(String childPath, Object childValue, FormattedObjectGraph nestedOutput)
DefaultValueFormatter.WriteMemberValueTextFor(Object value, MemberInfo member, FormattedObjectGraph formattedGraph, FormatChild formatChild)
DefaultValueFormatter.WriteMemberValues(Object obj, MemberInfo[] members, FormattedObjectGraph formattedGraph, FormatChild formatChild)
DefaultValueFormatter.WriteTypeValue(Object obj, FormattedObjectGraph formattedGraph, FormatChild formatChild, Type type)
DefaultValueFormatter.WriteTypeAndMemberValues(Object obj, FormattedObjectGraph formattedGraph, FormatChild formatChild)
DefaultValueFormatter.Format(Object value, FormattedObjectGraph formattedGraph, FormattingContext context, FormatChild formatChild)
Formatter.Format(Object value, FormattedObjectGraph output, FormattingContext context, FormatChild formatChild)
Formatter.FormatChild(String path, Object value, FormattedObjectGraph output, FormattingContext context, FormattingOptions options, ObjectGraph graph)
<>c__DisplayClass5_0.<ToString>b__0(String path, Object childValue, FormattedObjectGraph output)
EnumerableValueFormatter.Format(Object value, FormattedObjectGraph formattedGraph, FormattingContext context, FormatChild formatChild)
Formatter.Format(Object value, FormattedObjectGraph output, FormattingContext context, FormatChild formatChild)
Formatter.ToString(Object value, FormattingOptions options)
UnitTest1.Array_of_objects_with_array_of_toStringable_objects() line 35
RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Regression?

Yes

In 6.11.0 Formatter.ToString doesn't throw.

Known Workarounds

No response

Configuration

Tried with both Version 6.12 and develop

Other information

By looking in the release notes for 6.12 I suspect #2144
CC: @benagain

Are you willing to help with a pull-request?

Yes, if anyone can figure out what the problem is

[ITaluone]

The second one is actually a pretty easy fix.

But the first one is pretty tricky.. because if you handle the ArgumentOutOfRangeException the output is still messed up

See:

{
    {
    {P}
            P
    }
}

[jnyrup]

What where your findings?
So far I've been focusing on FormattedObjectGraph.AddFragmentOnNewLine.

Preferably we should not handle ArgumentOutOfRangeException but avoid it from ever happening

[ITaluone]

[Fact]
public void Array_of_objects_with_array_of_toStringable_objects()
{
    var sigmets = new[]
    {
        new
        {
            Points = new Point[] { new() }
        },
        new
        {
            Points = new Point[] { new() }
        }
    };

    var result = Formatter.ToString(sigmets, new() { UseLineBreaks = true });

    result.Should().Be(
        """

        {
            {
                Points =
                {
                    P
                }
            },
            {
                Points =
                {
                    P
                }
            }
        }
        """);
}

    private void InsertAtStartOfLine(int lineIndex, string insertion)
    {
+      lineIndex = parentGraph.lines.Count <= lineIndex ? parentGraph.lines.Count - 1 : lineIndex;

        if (!parentGraph.lines[lineIndex].StartsWith(insertion, StringComparison.Ordinal))
        {
            parentGraph.lines[lineIndex] = parentGraph.lines[lineIndex].Insert(0, insertion);
        }
     }

All tests green (except the first one in this ticket)

Preferably we should not handle ArgumentOutOfRangeException but avoid it from ever happening

That's what I meant 😉

[benagain]

In Array_of_arrays_of_toStringable_objects it is very confused - It doesn't indent the inner arrays; puts the comma between the two arrays on the wrong line, and doesn't wrap the second array in braces:

{
    , 
    {
    {P}
            P
    }
}

[benagain]

It's something to do with the first sub-array...

        var sigmets = new Point[][]
        {
            [new Point(), new Point(), new Point()],
            [new Point(), new Point()],
            [new Point(), new Point()],
        };

results in

{
    {
    P, 
        P, 
            P
    }, 
    {
        P, 
        P
    }, 
    {
        P, 
        P
    }
}

[ITaluone]

The ArgumentOutOfRangeException does only occur, if the first inner array has only one element.
This is producing the malformed formatting, but no exception

var sigmets = new Point[][]
{
    [new Point(), new Point()],
    [new Point()],
    [new Point()],
};

[jnyrup]

Interesting findings from both of you. Thanks for helping looking into this.

[IT-VBFK]

As it seems, the formatting does not recognize, that the first item in the first sub-array is one intendation layer deeper

var sigmets = new Point[][]
        {
            [new Point(), new Point(), new Point()],
            [new Point(), new Point()],
            [new Point(), new Point()],
        };

{
    {
    P,  <--- here the intendation is 1 as well
        P, 
            P
    }, 
    {
        P, 
        P
    }, 
    {
        P, 
        P
    }
}

[jnyrup]

I suggest we remove the usage of UsingLineBreaks from BeSameAs and NotBeSameAs.
They are the only two assertions in ReferenceTypeAssertions that use UsingLineBreaks.

While it doesn't solve the problem if an end user chooses to use UsingLineBreaks, it should solve the case for the default usage.
@dennisdoomen What do you think?

[dennisdoomen]

Yeah, makes sense. We should also investigate what we intend to accomplish with UsingLineBreaks.

[benagain]

I think the problem boils down to this:

    [Fact]
    public void Indentation_before_first_fragment()
    {
        var formatter = new FormattedObjectGraph(100);
        formatter.WithIndentation();          // from EnumerableValueFormatter (indirectly)
        formatter.AddFragmentOnNewLine("P");  // from DefaultValueFormatter
        formatter.AddFragmentOnNewLine("P");  // from DefaultValueFormatter

        var s = formatter.ToString();
        s.Should().Be(
            """
                P
                P
            """);
    }

The formatted string is actually

P
    P

and this is because FormattedObjectGraph.lineBuilderWhitespace is not set until after the first fragment is added.

I traced through what's going on when we format those arrays, and these are the calls made on FormattedObjectGraph:

EnumerableValueFormatter calls formatChild on the first element

fluentassertions/Src/FluentAssertions/Formatting/EnumerableValueFormatter.cs

Line 43 in 08178fc

formatChild(iterator.Index.ToString(CultureInfo.InvariantCulture), iterator.Current, formattedGraph);

which calls WithIndentation on the formatter

fluentassertions/Src/FluentAssertions/Formatting/Formatter.cs

Lines 146 to 150 in 08178fc

	using (output.WithIndentation())
	{
	Format(value, output, context, (childPath, childValue, nestedOutput) =>
	FormatChild(childPath, childValue, nestedOutput, context, options, graph));
	}

which uses DefaultValueFormatter that calls AddFragmentToline on the formatter

fluentassertions/Src/FluentAssertions/Formatting/DefaultValueFormatter.cs

Lines 35 to 38 in 08178fc

	else if (context.UseLineBreaks)
	{
	formattedGraph.AddFragmentOnNewLine(value.ToString());
	}

The problem is that FormattedObjectGraph knows nothing about UseLineBreaks and so shouldn't automatically indent the first line, it's the interplay between EnumerableFormatter and DefaultValueFormatter. It's tricky to work out where to add the whitespace that won't break all the other use cases.