fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
517 stars 144 forks source link

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

Open josh-degraw opened 10 months ago

josh-degraw commented 10 months ago

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).

nojaf commented 10 months ago

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.

dsyme commented 10 months ago

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

dsyme commented 10 months ago

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.