fsharp / fslang-design

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

[style-guide] Format mutation expressions on their own line #663

Open dsyme opened 2 years ago

dsyme commented 2 years ago

I have a strong dislike of code where -> and <- appear on the same line:

let f x =
    let mutable keepGoing = false
    match x with
    | Some _ -> ()
    | None -> keepGoing <- false

Because this code is relatively rare I think we should consider a style guide entry to explicitly say "put this on two lines"

let f x =
    let mutable keepGoing = false
    match x with
    | Some _ ->
        ()
    | None ->
        keepGoing <- false
dsyme commented 2 years ago

@nojaf @charlesroddie thoughts?

dsyme commented 2 years ago

Likewise

someThing <- fun a -> expr

:) Which should at least be parenthesized:

someThing <- (fun a -> expr)
nojaf commented 2 years ago

This is detectable from an AST point of view, so I don't see any technical limitations here.

charlesroddie commented 2 years ago

These seem ad-hoc and I don't see any general principles that would justify them.

I have a strong dislike of code where -> and <- appear on the same line: | None -> keepGoing <- false

Perhaps this tries to attain a good visual appearance by avoiding spurious symmetries. I don't think there is any lack of clarity and I don't see any structural reason for preferring either form.

To make a slippery slope argument, if we go down this route we will end up flagging abc d cba with "avoid palindromes that convey spurious symmetry" or even detecting 666 and warning "try to avoid unlucky numbers and rewrite this as 66_6 or 665+1".

Likewise someThing <- fun a -> expr

Well this is not "likewise" as parenthesizing this would still break the first rule which is to avoid <- and -> on one line. This seems completely different to me, perhaps related to cases where, while precedence is unambiguous, a reader might be confused about it and take time to figure it out.

Adding brackets for clarity sometimes does help, especially in cases where precedence is more confusing than this. But I don't see how this idea could be formalized.

dsyme commented 2 years ago

@charlesroddie If you assume the beginner doesn't know the precedence of -> and <-, then a -> b <- c and a <- b -> c both look like some kind of "tie fighter" or "x-wing" ternary language constructs.

By putting these on separate lines, we clarify this automatically. It's also how we clarify nested if, match etc

dsyme commented 2 years ago

I will update the style guide to indicate this specific case, and also to make a general recommendation that assignment expressions a <- b should always be on their own line. That more general recommendation would also cover, for example

let f() =
    if condition then a <- b

which would always be formatted

let f() =
    if condition then
        a <- b

The reasoning is that a <- b is always part of the imperative programming whose construts we always format over multiple lines (while, for, sequentials, and to some extent match and if..then..else)

dsyme commented 2 years ago

@nojaf I'm labelling decided style guide issues using "style-gudie-decided"

When style guide decisions are made like this in this repo, you can assume that this is decided and we can adjust fantomas even before the actual style guide is updated 👍

dsyme commented 2 years ago

Example catalog (we can add to this):

let f() =
    a <- b

let f() =
    if condition then
        a <- b

let f() =
    let x = 1
    a <- b
nojaf commented 2 years ago

@dsyme are there any other constructs that fit this bill? Or is this solely about a <- b?

dsyme commented 2 years ago

I think it's just about this, in the sense that adding anything else would be very heuristic.

Ideally perhaps any unit-typed expression or void-returning interop call, but we don't have types available to fantomas....

charlesroddie commented 2 years ago

The change from "a -> b <- c" to "Format mutation expressions on their own line" seems good to me. The only exception to the rule I can think of is to allow do b <- c.