Weird formatting of linq chain

[Rudomitori]

Input:

var something_________________________________________ = x.Answers
    .Select(y => (DateTime?)y.CreatedAt)
    .Min();

Output:

var something_________________________________________ = x.Answers.Select(y =>
    (DateTime?)y.CreatedAt
)
    .Min();

Expected behavior:

var something_________________________________________ = x.Answers
    .Select(y => (DateTime?)y.CreatedAt)
    .Min();

// or
var something_________________________________________ = x
    .Answers.Select(y => (DateTime?)y.CreatedAt)
    .Min();

// or
var something_________________________________________ = x
    .Answers
    .Select(y => (DateTime?)y.CreatedAt)
    .Min();

[loraderon]

I think this is the same issue as #1079

[belav]

This seems related to #1079, but this particular case did work quite a ways back. I was having a bit of trouble tracking down how exactly the regression was introduced so went back quite a few versions.

This appeared in 0.26.7 and I was able I track down the source of the problem. The changes in https://github.com/belav/csharpier/pull/967 were somewhat dependant on this single line I reverted in https://github.com/belav/csharpier/pull/1074. I still need to find a solution that makes all of the updated test cases happy.

Side note - running multiple versions of the playground would be nice to track down regressions like this. Or maybe a utility that uses docker to show the changes over versions for some code. But I don't know that this comes up often enough to be worth the effort.

[shocklateboy92]

Side note - running multiple versions of the playground would be nice to track down regressions like this. Or maybe a utility that uses docker to show the changes over versions for some code. But I don't know that this comes up often enough to be worth the effort.

I think git bisect is what you're after: https://git-scm.com/docs/git-bisect

Especially since you can write a trivial test and a script to repro it, you can even have it run/find the commit automatically: https://git-scm.com/docs/git-bisect#_bisect_run

[jods4]

A change was recently published that keeps method calls on same line as property access, for stuff like this (look at .ReadFrom., and .Enrich.):

// Formatted with 0.27
builder.UseSerilog(
    (context, logConfig) =>
        logConfig
            .ReadFrom.Configuration(context.Configuration)
            .Enrich.FromLogContext()
            .Enrich.WithExceptionDetails(
                new DestructuringOptionsBuilder().WithDefaultDestructurers()
            )
);

(Unrelated aside: @belav I think the code above would look better if the lambda parameters were kept on the UseSerilog(.. line. It's one more example of the recurring "single parameter lambda" theme.)

But I also noticed that in some cases CSharpier keeps the first .Property.Method(.. of a chain on the inital line, like OP. I'd agree it's not best for readability and for long chains I'd rather always break and indent at the first property.

Here's another test case taken from my project. The part entityType / .Keys.Select(k =>..) / .ToList() is ok in my opinion. The part k.KeyAttributes.Select(f => ..) / .ToList() I don't like and would like a break after k so that all subsequent calls are on the same indentation, like entityType. The worst aspect of this formatting is that closing )) from .Select( are less indented than following .ToList() and it just looks weird / not very readable.

// Formatted with 0.27
var AlternateKeys =
    entityType.Keys.Count > 0
        ? entityType
            .Keys.Select(k => new KeyTemplate
            {
                Name = k.DisplayName.GetDisplayName(),
                ClassName = AddSuffix(k.DisplayName.GetFriendlyName("<missing key name>"), "Key"),
                Fields = k.KeyAttributes.Select(f => new KeyValuePair<string, PropertyTemplate>(
                    f,
                    Properties.SingleOrDefault(x => x.LogicalName == f)
                ))
                    .ToList(),
            })
            .ToList()
        : null;

[belav]

@shocklateboy92 thanks for the link, that would have helped make what I did a bit easier!

@jods4 I happened to make changes that I believe take care of your second example. When the initial identifier is <= 4 characters, there was logic to keep the first group of the chain on the same line. Its main goal was to prevent formatting like this from occurring.

x
    .CallMethod____________________________________________________________()
    .CallMethod____________________________________________________________();

That logic is tweaked a bit, and results in your example like so.

// Formatted with 0.27.1
var AlternateKeys =
    entityType.Keys.Count > 0
        ? entityType
            .Keys.Select(k => new KeyTemplate
            {
                Name = k.DisplayName.GetDisplayName(),
                ClassName = AddSuffix(k.DisplayName.GetFriendlyName("<missing key name>"), "Key"),
                Fields = k
                    .KeyAttributes.Select(f => new KeyValuePair<string, PropertyTemplate>(
                        f,
                        Properties.SingleOrDefault(x => x.LogicalName == f)
                    ))
                    .ToList(),
            })
            .ToList()
        : null;

[jods4]

Looks good to me!

@belav Aside: entityType.Keys.Count > 0 is not kept on the same line as var AlternateKeys = because of the Rectangle Rule? I feel like Conditionals are one more instance where it would just look better if we broke it. IMHO it creates 2 almost empty lines, increases indentation and doesn't really improve readability.