MultiLineLambdaClosingNewline unnecessarily breaks parameter list

[cmeeren]

Issue created from fantomas-online

Code

Target.create "Build" (fun ctx ->
    // code
    // here
    ())

Result

Target.create
    "Build"
    (fun ctx ->
        // code
        // here
        ()
    )

Problem description

The example above is straight out of the F# style guide, section Formatting lambdas.

I would have expected setting MultiLineLambdaClosingNewline to true would (as its name implies) only move the final parenthesis, like this:

Target.create "Build" (fun ctx ->
    // code
    // here
    ()
)

This compiles, and it would be in keeping with the style guide:

When a lambda expression is used as an argument in a multi-line expression, and is followed by other arguments, place the body of a lambda expression on a new line, indented by one level:

However, as seen above, Fantomas additionally breaks the parameter list.

Extra information

• [ ] The formatted result breaks my code.
• [ ] The formatted result gives compiler warnings.
• [ ] I or my company would be willing to help fix this.
  • Maybe, with some guidance (I don't know the Fantomas code base), though not for several weeks due to travels

Options

Fantomas main branch at 1/1/1990

    { config with
                MultiLineLambdaClosingNewline = true }

_{Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?}

[Smaug123]

Extra context: the G-Research F# formatting conventions are silent on this, and I'm happy with Microsoft's recommendation, so we would align GR with Microsoft here.

[nojaf]

I disagree this was implemented according to the example of the G-Research formatting conventions.

The sample above has always been considered the equivalent of:

// OK if lambda body is long enough to require splitting lines
let printListWithOffset a list1 =
    List.iter
        (fun elem ->
            printfn "%d" (a + elem)
        )
        list1

[cmeeren]

@nojaf Sorry, I'm confused. What did you disagree with - the remark by @Smaug123, or my issue description?

For clarity, the example I posted is taken vertbatim from the F# style guide, section Formatting lambdas. So if you do not want Fantomas to align with the below, then the F# guidelines should probably be adjusted:

Target.create "Build" (fun ctx ->
    // code
    // here
    ())

[josh-degraw]

Okay, slightly more detailed answer here:

As mentioned, Fantomas has historically followed/supported 2 style guides: the Microsoft style guide (because F# came from Microsoft), and the G-Research style guide (because G-Research was a key sponsor of fantomas).

In this case there are 2 rules at play:

1. The MultiLineLambdaClosingNewline rule
2. How we format function parameters (what stays on one line vs when do we apply all args on a new line)

MultiLineLambdaClosingNewLine, in isolation, does fully adhere to your expectation above when the lambda is the only parameter, as shown here in both style guides:

Microsoft

// ✔️ OK
Target.create "Build" (fun ctx ->
    // code
    // here
    ())

G-Research:

// OK
let printListWithOffset a list1 =
    list1
    |> List.iter (fun elem ->
        printfn "%d" (a + elem)
    )

Note that Microsoft's example has the closing paren on the same line, while G-Research has it on its own line.

However, when multiple parameters are passed to a function we sometimes need to handle things differently.

In the G-Research style guide we have this:

// If any argument will not fit on a line, split all the arguments onto different lines
let printListWithOffset a list1 =
    List.iter
        (fun elem -> printfn "%d" (a + elem))
        list1

// OK if lambda body is long enough to require splitting lines
let printListWithOffset a list1 =
    List.iter
        (fun elem ->
            printfn "%d" (a + elem)
        )
        list1

// OK
let printListWithOffset a list1 =
    list1
    |> List.iter (fun elem ->
        printfn "%d" (a + elem)
    )

// OK if lambda body is long enough to require splitting...
let printListWithOffset a list1 =
    list1
    |> List.iter (
        ((+) veryVeryVeryVeryLongThing)
        >> printfn "%d"
    )

// ... but if lambda body will fit on one line, don't split
let printListWithOffset' a list1 =
    list1
    |> List.iter (((+) a) >> printfn "%d")

// If any argument will not fit on a line, split all the arguments onto different lines
let mySuperFunction v =
    someOtherFunction
        (fun a ->
            let meh = "foo"
            a
        )
        somethingElse
        (fun b -> 42)
        v

Note // If any argument will not fit on a line, split all the arguments onto different lines

Then, in Microsoft's style guide, we have:

// If the lambda argument is the last argument in a function application, place all arguments until the arrow on the same line.

// ✔️ OK
Target.create "Build" (fun ctx ->
    // code
    // here
    ())

// ✔️ OK
let printListWithOffsetPiped a list1 =
    list1
    |> List.map (fun x -> x + 1)
    |> List.iter (fun elem ->
        printfn $"A very long line to format the value: %d{a + elem}")

// When there are many leading or multi-line arguments before the lambda indent all arguments with one level.

// ✔️ OK
functionName
    arg1
    arg2
    arg3
    (fun arg4 ->
        bodyExpr)

// ✔️ OK
functionName
    arg1
    arg2
    arg3
    (function
     | Choice1of2 x -> 1
     | Choice2of2 y -> 2)

Here also note // When there are many leading or multi-line arguments before the lambda indent all arguments with one level.

Which operates according to the same rules in both style guides, the only differences being: whether or not a multi-line lambda has the closing paren on its own line, and Microsoft's style guide is silent on when exactly to split up multiple parameters when a lambda is involved, it only states how to format the lambda "When there are many leading or multi-line arguments".

So in short: this setting does already technically conform to both style guides, but, since the G-Research style guide specifically mentioned that "If any argument will not fit on a line, split all the arguments onto different lines," and that was the style guide that inspired the addition of MultiLineLambdaClosingNewLine, the heuristic that was chosen to be applied when that setting is on was that if the lambda spans multiple lines, all arguments should go on their own lines. Since Microsoft's style guide didn't specify when to give arguments their own line, we do that less aggressively when the setting is off.

From my perspective I'd say 1. this isn't technically a bug, but 2. I do see how this can be confusing (especially because I feel like this is handled more deterministically for stroustrup records, for example) and I don't think it's out of the realm of possibility to have a setting to more deterministically specify when arguments should be split up into multiple lines or not when a lambda is involved. If that's something that's strongly desired, I'd suggest to make a new feature request and we can discuss the possibility there in more depth, and also probably to make a suggestion to https://github.com/fsharp/fslang-design to see if there can be a more specific guideline about when to split up arguments onto their own lines when lambdas are involved.

[Smaug123]

Sorry for the confusion; I specifically meant that the special case of "everything fits on one line except for the final argument which is a lambda" isn't covered explicitly in the G-Research style guide. We do indeed say "split everything onto new lines if the whole thing doesn't fit", but I was interpreting the quoted rule (which is about a trailing lambda) as being specifically high in priority versus other rules according to the MS guide, and I can see why one would want that rule to take precedence. Personally I think either option of "obey that rule in this special case of having a trailing lambda in an otherwise short function call" or "always split over multiple lines" would be appropriate, so I'm in favour of whatever is the least work/is most intuitive to everyone/is best aligned with Microsoft. (I have an intuition that aligning with Microsoft would involve less work in the long run, although no actual concrete reason to believe that other than general principles.)