Happypig375 commented 5 years ago

RFC https://github.com/fsharp/fslang-design/blob/main/preview/FS-1107-Allow-attributes-after-the-module-keyword.md

Allow attributes after the module keyword

Consider:

[<AutoOpen>]
module M =
    [<AbstractClass>]
    type C() = class end
    [<Literal>]
    let X = 3

The Literal attribute can be embedded into the let:

[<AutoOpen>]
module M =
    [<AbstractClass>]
    type C() = class end
    let [<Literal>] X = 3 // ✓

Same for the AbstractClass:

[<AutoOpen>]
module M =
    type [<AbstractClass>] C() = class end // ✓
    let [<Literal>] X = 3

But not the AutoOpen.

module [<AutoOpen>] M = // FS0010: Unexpected start of structured construct in definition. Expected identifier, 'global' or other token.
    type [<AbstractClass>] C() = class end // FS0010: Unexpected keyword 'type' in implementation file
    let [<Literal>] X = 3

The existing way of approaching this problem in F# is leaving the AutoOpen above the module.

Pros and Cons

The advantages of making this adjustment to F# are

Consistency - modules are just specialized types, so placing attributes after the keyword should be allowed as well.
Conciseness - we can utilize the horizontal space to our advantage, thus scrolling less.

The disadvantage of making this adjustment to F# is having more ways to do the same thing. However, precedence has been already made for types and lets.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Related suggestions:

33

I would say that this should be allowed for members as well.

// Not using a module to ensure that the module suffix is not added
type MyList() =
    // Fields omitted
    [<CompiledName("Map")>]
    static member map f l =
        // Implementation omitted

Thanks for the suggestion However I will decline this: what we have works, and the proposed saving is only one line, and the lines become loooong. In balance I don't see a net benefit.

Sure, what we have works, but inconsistent language features are worse than having multiple ways to do the same thing. (Recall: Unification of List, Array, Seq functions in F# 4.0)

Whether the lines become long depends on the length of the name of members and attributes. If they are short enough, they deserve to be combined to utilize more horizontal space.

Affidavit (please submit!)

Please tick this by placing a cross in the box:

[x] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
[x] I have searched both open and closed suggestions on this site and believe this is not a duplicate
[x] 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. (This has sort of been decided for members, but not modules. Module definitions tend to be much shorter than member definitions anyways.)

Please tick all that apply:

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

charlesroddie commented 5 years ago

The style guide suggests "place the attribute on its own line" (for [<Literal>]s but this naturally extends to other attributes). Consistency is good here.

cartermp commented 5 years ago

@charlesroddie although the style guide does mention that, at the end of the day it's only style. I think having consistency in the language is key here; we want people to adopt that guide not because they have to, but because they agree with its contents.

dsyme commented 5 years ago

Allowing attributes on module declarations would be a good addition, marking as approved.

abelbraaksma commented 4 years ago

Just to add, you can already place the attribute on the same line as module: in front of it, like so:

[<AutoOpen>] module M =...

However, the indentation rules are iffy when it comes to prepending with attributes, sometimes the attribute is counted, sometimes it is not, as can be seen in this example:

[<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

In the case of module, the indentation rules consider the position after the attribute(s), which in turn would mean everything needs to be indented to the right. Not an ideal workaround ;).

BentTranberg commented 3 years ago

I know I'm late here, but...

This is not only good for style, but also safety. I have mentioned this elsewhere, but it bears repeating. When attributes are placed above the code they affect, there is a risk that code creep will introduce unintended changes that go unnoticed. The possibility to place attributes inside the code elements they affect, will considerably reduce the risk of code creep.

Code creep is the term used to describe what can happen with source code when attributes, directives, pragmas - whatever they're called in your favorite language - are used. While editing, it is far too easy to overlook the relationship between these kinds of source elements and the structured code that they affect, resulting in cut and paste of code snippets without their attributes. The code may still compile, with the attributes now affecting the wrong elements.

Niklaus Wirth banned these kinds of elements from his programming language Oberon. We can't do that in F#, but I think it would have been nice to have optional warnings for attributes placed before code elements rather than inside. Just an idea.

fsharp / fslang-suggestions