fslang-design icon indicating copy to clipboard operation
fslang-design copied to clipboard

[style-guide]: Multiline generic type parameters bracket alignment

Open josh-degraw opened this issue 2 years ago • 3 comments

As discussed in https://github.com/fsprojects/fantomas/issues/2706 and https://github.com/fsprojects/fantomas/pull/2852, with the recent changes regarding multiline bracket formatting, multiline generic type parameters don't necessarily always follow the same guidelines, so we should formalize how they can/should be formatted.

For example, if using Stroustrup bracket style, you might expect

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo<{|
        kind: string
        properties: {| statisticsEnabled: bool |}
    |}>

to be formatted like this:

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo<
           {|
               kind: string
               properties: {| statisticsEnabled: bool |}
           |} 
       >

However, the placement of the angle brackets can be problematic.

For example, if you have an application expression:

let bv = unbox<Foo<'innerContextLongLongLong, 'bb -> 'b>> bf

And try to directly apply Stroustrup style to it:

let bv =
    unbox<
        Foo<
            'innerContextLongLongLong,
            'bb -> 'b
        >
    >
        bf

This code is invalid. The compiler sees these as two separate statements. So short of updating the compiler, code in this style will need to have at least one additional space on the newline before the closing angle bracket.

let bv =
    unbox<
        Foo<
            'innerContextLongLongLong,
            'bb -> 'b
-        >
+         >
-    >
+     >
        bf

Bikeshedding question: should this be the minimal change necessary (1 space)? Or should it be a multiplier of your indent_size (if you use 2 spaces, use 2 spaces)?

Also, it probably should be noted that truly "Aligned" angle bracket style doesn't work:

let private asJson (arm: IArmResource) =
    arm.JsonModel
    |> convertTo
        <
           {|
               kind: string
               properties: {| statisticsEnabled: bool |}
           |} 
        >

Some users may expect that to work but it's invalid code. We should maybe update the style to mention this (if it's not there already).

josh-degraw avatar Nov 13 '23 18:11 josh-degraw

Thanks for bringing this up!

Bikeshedding question: should this be the minimal change necessary (1 space)? Or should it be a multiplier of your indent_size (if you use 2 spaces, use 2 spaces)?

Using the indent_size will make it seem more deliberate I think. A single space will indeed strike more as a bug.

nojaf avatar Nov 13 '23 19:11 nojaf

From a style guide perspective, the above proposal seems fine.

dsyme avatar Nov 14 '23 17:11 dsyme

This code is invalid. The compiler sees these as two separate statements. So short of updating the compiler, code in this style will need to have at least one additional space on the newline before the closing angle bracket.

I agree that ideally this should be valid for the compiler.

dsyme avatar Nov 14 '23 17:11 dsyme