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

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

Open dsyme opened this issue 3 years ago • 11 comments

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 avatar May 18 '22 17:05 dsyme

@nojaf @charlesroddie thoughts?

dsyme avatar May 18 '22 17:05 dsyme

Likewise

someThing <- fun a -> expr

:) Which should at least be parenthesized:

someThing <- (fun a -> expr)

dsyme avatar May 18 '22 17:05 dsyme

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

nojaf avatar May 18 '22 19:05 nojaf

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.

charlesroddie avatar May 18 '22 20:05 charlesroddie

@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 avatar May 20 '22 14:05 dsyme

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 avatar May 26 '22 17:05 dsyme

@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 avatar May 26 '22 17:05 dsyme

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

dsyme avatar May 26 '22 18:05 dsyme

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

nojaf avatar May 30 '22 15:05 nojaf

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

dsyme avatar May 31 '22 17:05 dsyme

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.

charlesroddie avatar Jul 04 '22 08:07 charlesroddie