Specify that rules for explicit generic type arguments also apply to function application

[cmeeren]

It makes sense that the same rules should apply to explicit generic parameters in function application, too.

Currently Fantomas formats this in a way that breaks the "avoid name-sensitive alignment" clause:

Json.serialize<{| child: {| displayName: string; kind: string |}
                  newParent: {| id: string; displayName: string |}
                  requiresApproval: bool |}>

Here is a better formatting that is consistent with the rules for function/member/type definitions:

Json.serialize<
    {| child: {| displayName: string; kind: string |}
       newParent: {| id: string; displayName: string |}
       requiresApproval: bool |}>

I am updating the style guide to clarify this so there is no doubt, and so that Fantomas can hopefully be updated accordingly.

/cc @nojaf @dsyme

[BillWagner]

ping @dsyme for review

[dsyme]

@nojaf what do you think? I see the inconsistency but type applications are perhaps a bit different

[cmeeren]

type applications are perhaps a bit different

I don't want to start a big discussion, just wondering, how is it different? With the current behavior of Fantomas, you still get potentially significant code chunks (as in the demo) moved off to the right and potentially breaking your code on rename (due to offsets changing). This is in violation of the "avoid name sensitive alignments" clause which is already part of the guide.

[cmeeren]

For example, with Fantomas' current behavior, if you rename serialize in the example to serialize2, the code breaks (won't compile) and Fantomas is no longer able to format it, meaning you'd have to manually fix the code every place it's used (hundreds of places for my part!). So I'd say it's fairly important that the current behavior is changed.

This PR is basically just to make it absolutely clear; I don't see it as a prerequisite for the Fantomas change due to the aforementioned reasons (breakage on rename + avoid name-sensitive alignments should be sufficient reasons).

[dsyme]

With the current behavior of Fantomas, you still get potentially significant code chunks (as in the demo) moved off to the right and potentially breaking your code on rename (due to offsets changing). This is in violation of the "avoid name sensitive alignments" clause which is already part of the guide.

I see, yes, that's a good argument from principle. I've not often done multi-line generic argument instantiations so my intuitions aren't so good here. The only different I thought of is that the serialize and < need to be adjacent though that doesn't affect what you've proposed.

But what's the actual rule? Is it that a multi-line anon record type should always start {| on a new line? So

Json.serialize<
    {| child:
        {| displayName: string
           longLongLongLongLongKind: string |}
       newParent:
           {| id: string
              longLongLongLongDisplayName: string |}
       requiresApproval: bool |}>

etc.? That's needed to make it robust to renaming? It's not really about generic instantiations is it? This would apply to any multi-line anon record type formatting?

[cmeeren]

I don't get it. Sure, the code breaks due to offsets coming from the anonymous record. But the proposed change in this PR is still sound from a principle standpoint, as argued.

The formatting guidelines can of course introduce something about anonymous records specifically, but that is not itself relevant to this PR. The code breakage occur only because of the way Fantomas formats the generic parameters, which I guess it would do whether or not there is an anonymous record there.

[dsyme]

@cmeeren I think your suggestion is fine, I just want to discuss with @nojaf what the actual rule is.

The rules immediately preceeding your addition are describing what to do with

thing<lots, of, little, things>

Your example is

thing<one-really-big-and-long-thing
      that-covers-multiplelines-via-inner-linebreaks>

and saying it should be

thing<
    one-really-big-and-long-thing
    that-covers-multiplelines-via-inner-linebreaks>

That's ok - it's just that AFAICS it doesn't really following from the other rules described, and we should consider all the cases where this arises and check we are doing things consistently.

[cmeeren]

I see. I'll let you two hash this out. 👍

[cmeeren]

@nojaf, just a reminder that @dsyme wants your input here (in case you missed it amidst all the other talk here).

[IEvangelist]

This looks good, thank you. We'll when you make the suggested changes.

[IEvangelist]

This looks good, thank you. We'll when you make the suggested changes.

Hi friends, hey @dsyme - are you good with this, or should it be changed?

[dsyme]

My concerns here weren't really fully resolved, but the change is still basically ok

[nojaf]

Hello, I have missed all of this I'm afraid.

As for SynExpr.Typed (Foo<Bar>), I don't think there are a lot of rules in place.
Nothing really comes to mind, except for fsprojects/fantomas#1611.

SomeDatabase.CreateCommand<"
                    select name
                    from things
                    where id = :id
   "            >

The position of > matters in the case of a string.

Fantomas currently does not have anything in place for doing something different when the type is multiline.

On a side note, I'm also wondering about putting the closing > on the next line:

Json.serialize<
    {| child: {| displayName: string; kind: string |}
       newParent: {| id: string; displayName: string |}
       requiresApproval: bool |}
>

The same for anonymous type definitions, nothing really is in place.
Regardless of length, we put everything on the right-hand side (example)