Docs: F# style guidelines: Recommend non-vanity indentation

[cmeeren]

The following is a request to change the F# style guidelines.

In section Formatting white space, subsection Place parameters on a new line for long definitions, the style guide says:

If you have a very long function definition, place the parameters on new lines and indent them to match the indentation level of the subsequent parameter.

f# module M = let LongFunctionWithLotsOfParameters(aVeryLongParam: AVeryLongTypeThatYouNeedToUse) (aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse) (aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse) = // ... the body of the method follows

As mentioned in https://github.com/fsprojects/fantomas/issues/657, this kind of indentation (AFAIK called "vanity indentation") has several drawbacks:

• Important code moved far to the right
• There is little space left for the actual code
• Refactoring names breaks the alignment (even if using tooling like Fantomas to auto-format code, the tooling may need to be explicitly invoked on all changed files, and AFAIK renames that extend names may cause compiler warnings)
• There is an unseemly gap on the left-hand side (personal opinion)

I would format the above example like this:

```f#
module M =
let LongFunctionWithLotsOfParameters
(aVeryLongParam: AVeryLongTypeThatYouNeedToUse)
(aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse)
(aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse)
=
// ... the body of the method follows

This has none of the mentioned drawbacks.

As seen in the linked section, this also goes for the recommendations for members and constructors.

Another section which has the same problem is [Formatting copy-and-update record expressions](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-copy-and-update-record-expressions), which has this example:

> ```f#
> let newState =
>     {
>         state with
>             F = Some {
>                     F1 = 0
>                     F2 = ""
>                 }
>     }
> ```

Notice how the indentation of the `F` value is sensitive to the name of `F`:


```f#
let newState =
    {
        state with
            AGorillaHoldingTheBananaAndTheEntireJungle = Some {
                                                             F1 = 0
                                                             F2 = ""
                                                         }
    }

A better approach would be:

f# let newState = { state with AGorillaHoldingTheBananaAndTheEntireJungle = Some { F1 = 0 F2 = "" } }

---

Conversely, in the section Formatting discriminated unions (unrelated to explicit whitespace recommendations), my preferred approach is demonstrated:

f# let tree1 = BinaryNode (BinaryNode(BinaryValue 1, BinaryValue 2), BinaryNode(BinaryValue 3, BinaryValue 4))

Furthermore, in the section Formatting pipeline operators, the non-vanity indentation kind is also shown (though vanity indentation would have |> aligned three more spaces to the right):

```f#
// Preferred approach
let methods2 =
System.AppDomain.CurrentDomain.GetAssemblies()
|> List.ofArray
|> List.map (fun assm -> assm.GetTypes())
|> Array.concat
|> List.ofArray
|> List.map (fun t -> t.GetMethods())
|> Array.concat

// Not OK
let methods2 = System.AppDomain.CurrentDomain.GetAssemblies()
|> List.ofArray
|> List.map (fun assm -> assm.GetTypes())
|> Array.concat
|> List.ofArray
|> List.map (fun t -> t.GetMethods())
|> Array.concat
```

In light of these counter-examples, one could argue that changing recommending non-vanity indentation would make the style guide more consistent.

---

To fix this issue, I suggest the following:

• In the examples above, use the non-vanity kind of indentation
• Add a separate sub-section under "Formatting white space" explicitly stating that the non-vanity kind of indentation is preferred over vanity indentation for the aforementioned reasons. This section could, among other examples, show mutable setters (https://github.com/fsprojects/fantomas/issues/659) and the reverse pipeline operator (#21459).

If this is acceptable, I can help create a PR if desired, but I have no personal need to put my name on this change and really just want it changed regardless of who does the work.

/cc @nojaf @knocte @lydell @pragmatrix @cartermp

[knocte]

I would format the above example like this:

module M =
let LongFunctionWithLotsOfParameters
(aVeryLongParam: AVeryLongTypeThatYouNeedToUse)
(aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse)
(aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse)
=

Thing is, if you advocate for this approach above, it would only be logical to apply the same approach to members, and to non-currified parameters, and to even constructors! (otherwise it's not consistent), however, that would generate warnings, as we found in the past: see issue and fix.

[cmeeren]

Thing is, if you advocate for this approach above, it would only be logical to apply the same approach to members, and to non-currified parameters, and to even constructors!

Yes, as I said. :)

however, that would generate warnings, as we found in the past: see issue and fix.

Simply place the opening parenthesis on the line with the first parameter.

So this example from the docs:

```f#
type TM() =
member _.LongMethodWithLotsOfParameters(aVeryLongParam: AVeryLongTypeThatYouNeedToUse,
aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse,
aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse) =
// ... the body of the method

type TC(aVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse,
aSecondVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse,
aThirdVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse) =
// ... the body of the class follows
```

Would become:

```f#
type TM() =
member _.LongMethodWithLotsOfParameters
(aVeryLongParam: AVeryLongTypeThatYouNeedToUse,
aSecondVeryLongParam: AVeryLongTypeThatYouNeedToUse,
aThirdVeryLongParam: AVeryLongTypeThatYouNeedToUse) =
// ... the body of the method

type TC
(aVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse,
aSecondVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse,
aThirdVeryLongCtorParam: AVeryLongTypeThatYouNeedToUse) =
// ... the body of the class follows
```

---

Protip: You can select text from a comment here on GH and click the three dots -> "Quote reply" to preserve the markdown formatting.

[cartermp]

For constructors we tend to use this in the F# codebase:

type C
    (
        abc: ...,
        def:...
    ) =

    ...

If this works consistently for other members that might actually not be a bad approach. I am hesitant to introduce more churn in fantomas though.

[lydell]

@cartermp That style looks very easy to edit, I like it.

[nojaf]

This looks similar to what GR is doing for constructors so I wouldn't mind ;)

[knocte]

Simply place the opening parenthesis on the line with the first parameter.

Wow I didn't realise that slight difference, thanks for pointing out!

I am hesitant to introduce more churn in fantomas though.

If hesitant about this, a setting for fantomas could be developed, but its maintainer has just spoken so maybe this can be the new default. I'm happy to create a PR for this repo to change the style guidelines if this is the case.

[cartermp]

The main downside is that symmetry at the call site looks kinda funky. Though in practice I've found that passing values is a lot less verbose than the definition:

type C
    (
        x,
        y,
        z
    ) =
    member _.M
        (
            x,
            y,
            z
        ) = ()

    static member SM
        (
            x,
            y,
            z
        ) = ()

let c =
    C
        (
            1,
            2,
            3
        )

Happy to accept a PR here @knocte :)

[knocte]

symmetry at the call site looks kinda funky

Kinda agree with that actually. Although the name C being extra short contributes to the funkyness, this looks better:

let foo =
    Foo
        (
            1,
            2,
            3
        )

But the reason we had to put the parenthesis in the next line for defining constructors was to avoid a warning, and I don't think there would be a warning when calling them, so it could be this way in this case:

let foo =
    Foo(
        1,
        2,
        3
    )

Better?

[cmeeren]

Better?

Yes, I prefer the latter, but my main gripe is with vanity indentation, so I can live with either.

Actually, the parenthesis could be on the line with the last argument, which I think may be more consistent with the rest of the style guide (can't check right now):

f# let foobar = Foobar( 1, 2, 3)

But again, for me the particular solution is of less importance than avoiding vanity indentation in the first place.

[knocte]

Happy to accept a PR here @knocte :)

I've proposed https://github.com/dotnet/docs/pull/21690, please have a look.

[knocte]

I am hesitant to introduce more churn in fantomas though.

hey @nojaf another idea to avoid churn could be if the old behaviour was kept as opt-in (so default behaviour doesn't use vanity, and old behaviour is enabled through config flag fsharp_enable_vanity_alignment for example).

[nojaf]

Settings are churn for Fantomas, so I rather have no settings at all.
I'm ok with implementing whatever comes out of the style guide, as long as there is one way to do it.

Side note, the vanity alignment stuff is harder to implement in Fantomas so less of that would be great ;)

[cmeeren]

@nojaf PR #21690 was just merged, so now you can safely remove vanity indentation from Fantomas. Check out the PR diff for details on exactly what is recommended.