Skip to content

Fix indentation ambiguities when attributes are involved #514

@abelbraaksma

Description

@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

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions