Warn when undentation of operators causes ambiguity when following a match or if-else block

[abelbraaksma]

Warn for undentation ambiguities caused by operators

Note: this is undentation, or indentation-relaxation, not indentation.

I propose we raise a warning when undentation relaxation for operators will lead to ambiguities. This warning can be raised when the parser detects that after undenting an operator for the width of the operator, would make it belong to the previous block instead of the last statement. The warning could be:

Warning FS9999: Operator undentation ambiguity detected for operator "<%%>" causes it to belong to the previous line, instead of the previous block. To avoid this warning, indent the operator to the same indentation level as the previous line, or outdent it to have it belong to the block instead.

Rationale

The issue discussed here is notoriously hard to spot in a code base. I've been bitten by it numerous times over the cause of many years F# programming, and I am presently unsure whether my code silently has bugs caused by the indentation-relaxation rules for operators.

It is well established that F# is a whitespace-sensitive language, where generally related statements must start at the same indentation level.

For operators, there's an exception, where the parser will allow the operator to be undented until the rh-side of the operator is at the same line as the previous line. When the operator changes type, this is not a problem and wrongful use will lead to errors:

// this example works
let f v =
   match v with
   | None -> []
   | Some i -> [1..i]
   |> Seq.sum

// this example works
let f v =
   match v with
   | None -> 
       []
   | Some i -> 
       [1..i]
   |> Seq.sum

// this example errors:
let f v =
   match v with
   | None ->
      []
   | Some i -> 
      [1..i]
   |> Seq.sum    // caused by `Seq.sum` starting on the same level as `[1..i]`
                 // error: The type ''a list' does not match the type 'int'

In the examples above, it is clear, because the type inference will raise an exception (though the exception by itself will be hard to understand as it will be about the type, not the undentation error).

If we change this a little and create an operator for logging, it becomes must harder to detect:

// this example uses the default of 4 spaces for indentation
let (<%>) x (s: string) = Console.WriteLine s; x
let f v =
    match v with
    | None -> []
    | Some i -> 
        [1..i]
    <%> "logging"    // only hit when match is Some, never hits for None

// it becomes even harder to spot in an if-else statement
// operators like these are common in FParsec and other libraries
let (<??>) x (s: string) = Console.WriteLine s; x
let f v =
    if condition then []
    else [1..i]
    <%%> "Tested condition"

The problem is most apparent with operators that don't change type, because otherwise the chance of detection is much larger as it will likely raise a compile-time error.

The existing way of approaching this problem in F# is: to live with it and spot such errors visually (and as multiple reports have shown, this can be rather hard, even for seasoned developers).

Pros and Cons

The advantages are:

• an improved coding experience and less extremely-hard-to-find bugs.
• awareness with new F# programmers about the fact that operators count as whitespace w.r.t. the indentation rules
• F# is a very clear language and it's strength lies in the fact that type inference and strong, strict typing rules lead to writing correct code very easily and with little debugging. This improves on that experience.
• A warning allows the programmer to switch it off, or to improve his coding style for readability.
• Or, with warnings-as-errors, programmers can opt-in to treat this ambiguity as an error (should be a good setting for any project, imo)

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XS (I really hope that's true)

Related suggestions:

• Background on the offside rule: https://fsharpforfunandprofit.com/posts/fsharp-syntax/
• Confusion caused by undentation: https://stackoverflow.com/questions/42940157/f-type-error-with-one-level-of-indentation-not-with-another
• User reporting issues about wrong behavior of compiler due to use of |> operator in match block: https://stackoverflow.com/questions/45272356/which-match-with-indentation-rule-is-at-play-here
• Discussion on relaxing indentation rules in certain scenarios: #470
• Original bug report that triggered this suggestion: dotnet/fsharp#6136
• Older (2016) thread with the same issue, showing an even scarier bug by @smoothdeveloper: dotnet/fsharp#1019
• And ... (I'll update, I've seen several reports in the past, or add them in the comments)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

• This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
• I have searched both open and closed suggestions on this site and believe this is not a duplicate
• This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

• This is not a breaking change to the F# language design
• I or my company would be willing to help implement and/or test this

If this is going to be accepted and someone can point me to the location where the indentation-relaxation of operators takes place, I'll gladly take a look to see if I can have that code raise a warning for ambiguous scenarios.

[abelbraaksma]

To quote @dsyme from this above referenced thread of 2016, a bug report by @smoothdeveloper (thanks for pointing it out to me):

@smoothdeveloper Please consider opening a suggestion on http://fslang.uservoice.com to request a warning for the first of these cases. It would be great if you could formulate an exact proposed rule (e.g. one that would cover the interaction with "match", but no more, unless you identify other similar interactions).

To answer that last line, I think we can do one of two things:

• special-case branch instructions where this is relevant (if .. then .. else, match and match!)
• or generalize the case where the parser does a test each time it encounters an operator. The test should be something like: upon encountering a left-most operator, that starts at the same col position as the previous line, if I erase the operator and the trailing whitespace, does this line then still belong to the same previous block?

Not sure whether either of above options is easy or hard to detect in practice and whether corner cases exist that are not covered by such a simple rule.

(side note, @cartermp, I don't know how relevant these labels are, but I don't think my suggestion has to do with either type-checking or inference. I'm not trying to suggest a change in either. If anything, it is probably syntax related, or improving user experience, or the F# tokenizer/parser)

[charlesroddie]

I propose we raise a warning when undentation relaxation for operators will lead to ambiguities.

These undentations are bad in general surely. That's what the ambiguities show. Rather than warning only when bad undentations cause ambiguities, we should warn whenever they are made. That would be a lot clearer to users and also simpler for the compiler.

I.e. in

<spaces> <code1>
<spaces> <operator> <code2>

operator (and not just code2) should align at least with code1.

This code is currently explicitly allowed in the F# language spec:

let x =
    expr
  + expr
let x =
    expr
 |> f expr

I don't see why though. Is this considered good style? The behaviour in this thread seems to refute this.

[abelbraaksma]

I don't see why though. Is this considered good style?

@charlesroddie I don't think anybody considers it good style. At best it's mentioned in some text books. But in practice, I see everyone align on the start of the operator.

The only exceptions to the rule are, however, any form of parens, brackets, curlies. Here, the first item starting with an opening paren, bracket or curly, is undented one character, so that the actual code inside the brackets aligns.

Most bracket constructs use one character, but there's also [| and {|. I agree it's a good idea to warn always, but than with the exception of all kinds of bracket.

[charlesroddie]

Shouldn't the scope of this issue be undentation of operators, since bracket undentation doesn't give the ambiguities mentioned?