Fix indentation ambiguities when attributes are involved

[abelbraaksma]

I propose we fix the issues that causes incorrect throwing of FS0058 warning on incorrect indentation, and that we accept the left-most point of a line as its indentation start-point.

Currently, if you write [<Literal>] let x = 12, the warning is raised on the the position after [<Literal>], placing several literals underneath each other is therefore impossible. However, the warning itself points to the wrong position, you can indent the next line by one column to remove the warning.

For attributes that involve parameters, the FS0058 error is raised on the wrong position entirely.

Let bindings repro

This throws FS0058 on "let"

module Literals =
    [<Literal>] let First = 1
    [<Literal>] let Second = 2
    //..........^.............    // location of the FS0058 warnings

Minimal indentation to prevent warning (which oddly suggests it is an inner let-binding, but this is not how it gets compiled)

module Literals =
    [<Literal>] let First = 1
     [<Literal>] let Second = 2

Let bindings with parens repro

module Literals =
    [<CompiledName("Boolean")>]  let boolean = true
    [<CompiledName("MyString")>] let myString = "test"
    //............^..............^....................   // locations of the FS0058 warnings

Minimal indentation to prevent both warnings (to prevent only the second, one space is enough):

module Literals =
    [<CompiledName("Boolean")>]  let boolean = true
                    [<CompiledName("MyString")>] let myString = "test"

Pros and Cons

The advantages of letting the indentation position start at the [ of the attribute is that your code becomes more legible. It also removes the ambiguity that it gets the position wrong in the warning itself.

All in all it simply creates a more orthogonal coding experience: the indentation rule is that the next line must start on the same position as the previous line (or indented if it is part of the scope of the previous line).

I don't see disadvantages at the moment, but feel free to comment on it if there are. Since it fixes an error scenario, there are no backward incompatibility issues.

Extra information

I have to admit that I don't know the complexity of fixing this is.

I raised this originally as a bug (and I think that in part it still is a bug) here: dotnet/fsharp#1649

Related Suggestions

• Make the single-line use of [] consistent
• Allow Attributes to follow the member keyword

Affidavit

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 would be willing to help implement and/or test this
• I or my company would be willing to help crowdfund F# Software Foundation members to work on this

[cloudRoutine]

actually it's already possible

let [<Literal>] First = 1
let [<Literal>] Second = 2

[financial-engineer]

this code is much more readable and elegant

[<Literal>] let First = 1
[<Literal>] let Second = 2

I support abelbraaksma's suggestion.

[abelbraaksma]

@cloudRoutine, I know that that's an alternative syntax, but that doesn't remove the ambiguity in the current indentation rules if you don't abide by that syntax, and as @AlOdeychuk mentions, orthogonality in syntax is a good thing, and indeed more readable.

If the ground rule is that the let must start on the same column, then that should apply here as well, but presently it doesn't (and I believe there is something to say for letting the column of the current statement start at the beginning of the attribute).

[cloudRoutine]

placing several literals underneath each other is therefore impossible

@abelbraaksma this is all i was responding to

I agree regarding the indentation rule, I've made the same point regarding generic signatures

[abelbraaksma]

FWIW, the warning is still there in VS2017 latest preview version:

[abelbraaksma]

@dsyme, I think this could work if the offset is not taken into account when inside a an attribute. In other words, [<SomeAttribute>] let x should have offset for let as if it is at [, which allows a more natural flow:

[<SomeAttribute>] let x =
    12 + 30

Or:

[<SomeAttribute>] let x = 12 + 30
[<SomeAttribute>] let y = 12 + 30

Or

[<SomeAttribute>] let x = 12 + 30
let y = 12 + 30

would then all be valid. I'd be interested in resolving this, but then this lang-suggestion should first get some "approved-in-principle" or the like from you or someone else at the team (@KevinRansom, @cartermp perhaps?). I am not sure it requires a full RFC, as I think the spec is currently fuzzy about the position-count in these respects, but I have no problem writing one.

Perhaps fixing this could also help me get some pointers about fixing the problems with static type constraints (this too requires far right-indent, several issues have been reported about it).

[cartermp]

IMO this feels more like a bug than anything else, though @dsyme has the final word here.

[abelbraaksma]

@cartermp, yes, and it was first reported as a bug ;). In the OP I apparently wrote (but then forgot about dotnet/fsharp#1649):

I raised this originally as a bug (and I think that in part it still is a bug)

But I think this summarizes the reason why @dsyme wanted to see this done through an RFC:

I'm more referring to the risk of cascading changes to other code, which is best assessed through an RFC process I think.

[dsyme]

@cartermp It's certainly a glitch/bug. That said I'm always concerned about the potential for fixes to destabilize things in the parsing area, so we have to be very careful about a change. It's also possible that the change would be to deprecate existing constructs (e.g. inline access) if a safe change to enable orthogonal behaviour is not easy

[abelbraaksma]

@dsyme, talking about consistency, it currently works properly for types, but not (yet) for let-bindings. This suggests to me that the necessary machinery is already available in the parser, it just doesn't currently consider attributes, whereas it does consider them for types:

[<AutoOpen>] 
module M =      
    [<AbstractClass>] type C() = class end
    [<AbstractClass>] type D() = class end  // no indentation warning
    [<AbstractClass>] type E() = class end  // no indentation warning
    [<Literal>] let X = 3 
    [<Literal>] let Y = 3    // indentation warning
    [<Literal>] let Z = 3    // indentation warning

Using an attribute with module also leads to an indentation warning. Solving this orthogonally throughout would also fix #757 for the general case (the issue there is: wanting to put the attributes on one line with module declarations).

[dsyme]

Looking through the examples above, they all involve one-line [<Attr>] let which is really not a standard coding style.

I don't think we need to specifically do anything here. let [<Attr>] f ... is available, as is the more normal multi-line forms of these