Consider - keep Field.Method() on the same line when breaking long method chain

[will-molloy]

I came across this when working with fluent APIs, like Serilog.LoggerConfiguration and FluentAssertions.

I understand you need to insert the line breaks somewhere, but if you have Field.Method(), or Field.Field.Method() etc. it makes sense to keep them on the same line? (And only insert the line break after a method call?) Thoughts?

I guess this breaks if you have Field.Field.Field.Field... such that it overflows the line width, but I feel that would be as rare as a variable name taking up the entire width (don't have data to back that up sorry).

Few examples:

---

configuring Serilog logger

Expected:

        var loggerConfiguration = new LoggerConfiguration()
            .Enrich.FromLogContext()
            .Enrich.WithProperty("key", "value")
            .Enrich.WithProperty("key", "value")
            .Enrich.WithProperty("key", "value")
            .Enrich.WithProperty("key", "value")
            .WriteTo.Console(outputTemplate: "template");

Actual:

        var loggerConfiguration = new LoggerConfiguration()
            .Enrich
            .FromLogContext()
            .Enrich
            .WithProperty("key", "value")
            .Enrich
            .WithProperty("key", "value")
            .Enrich
            .WithProperty("key", "value")
            .Enrich
            .WithProperty("key", "value")
            .WriteTo
            .Console(outputTemplate: "template");

---

using FluentAssertions on a List<string>

Expected:

        list.Should()
            .NotBeNull()
            .And.NotBeEmpty()
            .And.HaveCount(3)
            .And.ContainInOrder("A", "B", "C");

Actual:

        list.Should()
            .NotBeNull()
            .And
            .NotBeEmpty()
            .And
            .HaveCount(3)
            .And
            .ContainInOrder("A", "B", "C");

---

using IHttpContextAccessor in .NET

Expected:

        if (
            _httpContextAccessor.HttpContext != null
            && _httpContextAccessor.HttpContext.Request.Headers.TryGetValue(
                "my-header",
                out var value
            )
        )
        {
            return value;
        }

Actual:

        if (
            _httpContextAccessor.HttpContext != null
            && _httpContextAccessor
                .HttpContext
                .Request
                .Headers
                .TryGetValue("my-header", out var value)
        )
        {
            return value;
        }

[belav]

This formatting only changed recently. Previously there was logic that kept properties and methods on the same line at times, which is how prettier formats.

We had some discussion about it on discord, and it does feel a bit odd to treat properties differently than methods.

var loggerConfiguration = new LoggerConfiguration()
    .Enrich.FromLogContext()
    .Enrich.WithProperty("key", "value")
    .Enrich.WithProperty("key", "value")
    .Enrich.WithProperty("key", "value")
    .Enrich.WithProperty("key", "value")
    .WriteTo.Console(outputTemplate: "template");

// vs

var loggerConfiguration = new LoggerConfiguration()
    .Enrich()
    .FromLogContext()
    .Enrich()
    .WithProperty("key", "value")
    .Enrich()
    .WithProperty("key", "value")
    .Enrich()
    .WithProperty("key", "value")
    .Enrich()
    .WithProperty("key", "value")
    .WriteTo()
    .Console(outputTemplate: "template");

But I do think your first few examples look better without the breaks. The Property.Method() could be thought of almost like a longer method call, for example

var loggerConfiguration = new LoggerConfiguration()
    .EnrichFromLogContext()
    .EnrichWithProperty("key", "value")
    .EnrichWithProperty("key", "value")
    .EnrichWithProperty("key", "value")
    .EnrichWithProperty("key", "value")
    .WriteToConsole(outputTemplate: "template");

I think it is worth considering - for a single property call followed by a method call, keep them on the same line.

The last example I do prefer the actual version, but maybe something like this would work. There is an issue somewhere around breaking the chain before breaking the method call.

        if (
            _httpContextAccessor.HttpContext != null
            && _httpContextAccessor.HttpContext.Request.Headers
                .TryGetValue("my-header", out var value)
        )
        {
            return value;
        }

[will-molloy]

The last example I do prefer the actual version

Same here. I guess it was more of a 'counter' example.

[belav]

Another example from #1043. Maybe it is best to go back to the way this was, I don't think there were any complaints about it previously.

Simple chained attribute access is split into multiple lines
Reproduction:

class X
{
    private void Method()
    {
        Application.Current.Dispatcher.Invoke(() =>
        {
            Console.WriteLine("hi!");
        });
    }
}

On 0.25.0, the above code is already formatted properly.

On 0.26.3, it is transformed into this:

class X
{
    private void Method()
    {
        Application
            .Current
            .Dispatcher
            .Invoke(() =>
            {
                Console.WriteLine("hi!");
            });
    }
}

I believe this was introduced by #967 and while I agree that long chains should be split, this seems a bit too eager.

[loraderon]

Maybe it is best to go back to the way this was, I don't think there were any complaints about it previously.

Will you consider reverting this in a 0.26.x release?

[belav]

Maybe it is best to go back to the way this was, I don't think there were any complaints about it previously.

Will you consider reverting this in a 0.26.x release?

That was.... way easier that I was thinking it would be. It was mixed in with other changes and was worried it would take more work to undo it. I'll have it out in a 0.26.7 soonish.