[style-guide] when expressions in match clauses about fslang-design HOT 15 OPEN

Perhaps

1. Format on one line if possible
2. Format on two lines if possible with when at start of second line 4-spaces in
3. Otherwise format on multiple lines with when at start of second line 4-spaces in, and multi-line predicate starting on 3rd line 8-spaces in.

That would give:

I'm fairly inconsistent about this and jazz format a lot. A random selection of examples from dotnet/fsharp
```fsharp
        | ForEachThen (isFromSource, firstSourcePat, firstSource, JoinOrGroupJoinOrZipClause(nm, secondSourcePat, secondSource, keySelectorsOpt, pat3opt, mOpCore), innerComp) 
            when 
                (let _firstSourceSimplePats, later1 = 
                     use _holder = TemporarilySuspendReportingTypecheckResultsToSink cenv.tcSink
                     ...) ->
            Some ...

and

        | ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly
            when cenv.hasInternalsVisibleToAttrib -> true

and

        | OptionalSequential (JoinOrGroupJoinOrZipClause (_, _, _, _, _, mClause), _) 
            when firstTry = CompExprTranslationPass.Initial ->

and

                | Expr.App (Expr.Link {contents = Expr.App (Expr.Val (ValDeref v, _, _), _, tyargs, [], _) }, _, [], args, m)  
                     when localRep.IsValRepresentedAsMethod v && not (cenv.recUses.ContainsKey v) -> 

and

        match vspec.MemberInfo with
        | None
            when
                vspec.IsBaseVal ||
                vspec.IsMemberThisVal && vspec.LogicalName = "__" ->
            false
        | _ -> true

I think the expression on the right of the -> should always be on the next line for either (2) or (3)

from fslang-design.

My personal preference is to always (even if it's only one) have when on a separate line, I find it a bit easier to notice and read.
I do agree though, that if it gets complex, we should encourage people to use AP.

from fslang-design.

I'm fairly inconsistent about this and jazz format a lot. A random selection of examples from dotnet/fsharp

        | ForEachThen (isFromSource, firstSourcePat, firstSource, JoinOrGroupJoinOrZipClause(nm, secondSourcePat, secondSource, keySelectorsOpt, pat3opt, mOpCore), innerComp) 
            when 
               (let _firstSourceSimplePats, later1 = 
                    use _holder = TemporarilySuspendReportingTypecheckResultsToSink cenv.tcSink
                    ...) ->
            Some ...

and

        | ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly
            when cenv.hasInternalsVisibleToAttrib -> true

and

        | OptionalSequential (JoinOrGroupJoinOrZipClause (_, _, _, _, _, mClause), _) 
                      when firstTry = CompExprTranslationPass.Initial ->

and

                | Expr.App (Expr.Link {contents = Expr.App (Expr.Val (ValDeref v, _, _), _, tyargs, [], _) }, _, [], args, m)  
                     when localRep.IsValRepresentedAsMethod v && not (cenv.recUses.ContainsKey v) -> 

and

        match vspec.MemberInfo with
        | None when
            vspec.IsBaseVal ||
            vspec.IsMemberThisVal && vspec.LogicalName = "__" -> false
        | _ -> true

As you can see, almost every imaginable style

from fslang-design.

That sounds reasonable to me.
I might prototype this in Fantomas to verify there are no edge cases.
Something just comes to mind where the -> needed to be on the next line to remain valid.
Maybe this case doesn't occur anymore since the relaxations of F# 6.

from fslang-design.

Something just comes to mind where the -> needed to be on the next line to remain valid.

I'm not aware of any cases needing this. I checked this case where the expression after when is non-atomic (has no definite end token)

let f () =
   match 1 with
   | _
       when
           let x = 1
           x > x -> 3
   | _ -> 3

from fslang-design.

I raise you:

let f () =
   match 1 with
   | _
       when
           match () with
           | _ -> true ->
       2
   | _ -> 3

This could be an acceptable limitation, covered by the style guide.
I mean, people shouldn't write this kind of code.

from fslang-design.

Yes, you're right, well-spotted :)

from fslang-design.

Agreed this can be a known limitation

from fslang-design.

I find it more clear when when is on the pattern line, as it make it easier to see the condition:

| ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly when
    cenv.hasInternalsVisibleToAttrib -> true

It also goes much more inline with other parts of the language where we place the keyword on the preceding line, like:

while condition do
    expression

if condition then
    expression

match x with
| somePattern ->
    expression

and not like:

while condition
    do expression
       someOtherOne

while condition
    do
        expression

if condition
    then expression

match x with
| somePattern
    -> expression

It may be a problem when you want to place the right hand side expression on a new line, though, as it'll currently have the same indent as the when expression. I've seen a trick that further indents code in similar cases and found it working very nice with F# as well:

match f a b c with
| someLongPattern, andAnotherOne when
        let value = 1 + 2
        someCondition ->
    let a = someFunc someArg [] true
    anotherFunc a 123 false

If a style like this was automatically used by a formatter and by an IDE Enter typing assists, that could be a good option to consider.

from fslang-design.

The reasoning to keep the when after the pattern makes sense to me.
The double indent for the expression seems a bit foreign in comparison to the rest of the style guide.

match a with
| b when
    c ->
    d

If c is long, I'm not overly bothered that it aligns with d.
For the people that are, again partial active pattern is your friend.

from fslang-design.

For me the problem is that this leads to significant visual confusion between the expression on the right of the -> and the when expression:

| ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly when
    cenv.hasInternalsVisibleToAttrib ->
    a>b

compared to:

| ILMemberAccess.FamilyOrAssembly | ILMemberAccess.Assembly
    when cenv.hasInternalsVisibleToAttrib ->
    a>b

It only feels applicable if we decide we want small expressions on the right of the ->

from fslang-design.

It only feels applicable if we decide we want small expressions on the right of the ->

I would certainly vote for it! 🙂

from fslang-design.

I looked at this again and am still inclined to follow the rules here: #662 (comment)

There are real readability issues with the when at the end. In order to understand the role of the guard and target expressions easily and simply, you need to know they are guard and target expressions. That means seeing the when and the -> and having appropriate indentation to help

I'll leave this open to discuss further however as both @nojaf and @auduchinok have said "put the when at the end" :)

from fslang-design.

"when" at the start does also make sense to me.
The rules above work for me, I don't really have a strong opinion about any of this.

from fslang-design.

In the cases provided earlier in the thread, I believe that when is being compared with then, do, and with.

I mentally bucket when into a different category of keywords than the latter above, since I frame it as a branching qualifier/indicator, rather than a divisor between a block-validation construct and the subsequent block.

I think about how in some other languages, they can omit keywords like then, do, and with (see: python), but wouldn't be able to get away with getting rid of when unless it was replaced with some other sort of symbol or logical identifier.

from fslang-design.