Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dsyme opened this issue May 18, 2022 · 11 comments
Open

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

dsyme opened this issue May 18, 2022 · 11 comments

Comments

@dsyme
Copy link
Contributor

dsyme commented May 18, 2022

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
Copy link
Contributor Author

dsyme commented May 18, 2022

@nojaf @charlesroddie thoughts?

@dsyme
Copy link
Contributor Author

dsyme commented May 18, 2022

Likewise

someThing <- fun a -> expr

:) Which should at least be parenthesized:

someThing <- (fun a -> expr)

@nojaf
Copy link
Contributor

nojaf commented May 18, 2022

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

@charlesroddie
Copy link
Contributor

charlesroddie commented May 18, 2022

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
Copy link
Contributor Author

dsyme commented May 20, 2022

@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
Copy link
Contributor Author

dsyme commented May 26, 2022

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
Copy link
Contributor Author

dsyme commented May 26, 2022

@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
Copy link
Contributor Author

dsyme commented May 26, 2022

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 dsyme changed the title [style-guide] "a -> b <- c" [style-guide] Format mutation expressions on their own line May 26, 2022
@nojaf
Copy link
Contributor

nojaf commented May 30, 2022

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

@dsyme
Copy link
Contributor Author

dsyme commented May 31, 2022

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants