fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
517 stars 144 forks source link

[style-guide] Single case union with private constructor and static members #762

Closed MangelMaxime closed 8 months ago

MangelMaxime commented 8 months ago

Hello,

I hope I understood the guidelines of Fantomas correctly and that I am at the right place to discuss code style.

When creating a single case union with a private constructor Fantomas format it as:

type QuestionBody = private QuestionBody of string

However if we try to add a static member on it, it becomes:

type QuestionBody =
    private
    | QuestionBody of string

    static member Test = ""

Which I feels a little strange to read because private is not on the same line as the "context it applies to".

By manually fiddling with the code it seems like we could write it like that:

// Option 1
type QuestionBody = private QuestionBody of string

    with static member Test = ""

// Option 2
type QuestionBody = 
    private QuestionBody of string

    with static member Test = ""

// Option 3
type QuestionBody =
    private QuestionBody of string 

    with

    static member Test = ""

// Option 4
type QuestionBody =
    private QuestionBody of string 

    with

        static member Test = ""

// Option 5
type QuestionBody =
    private QuestionBody of string with

    static member Test = ""

It is worth noting that in my project I am using fsharp_multiline_bracket_style = stroustrup which write record with static member using the withkeyword.

type User = {
    Age: int
} with

    static member Test = ""

What would be the recommended way to write such discriminated union? Should we keep the current formatting rule and make it explicit in the Guidelines or adhere to another formatting rule?

smoothdeveloper commented 8 months ago

Since the private applies to all constructors, and how avoiding with if a readable indentation approach is, I think, recommended, I feel the current behaviour is OK.

The fact that it keeps the code on a single line when there are no attached members may be an edge-case for single case DU; edge case which is favoured for conciseness of very simple type.

But this conciseness isn't the main concern when the DU also comes with members.

All in all, I feel the current behaviour is correct.

MangelMaxime commented 8 months ago

Since the private applies to all constructors, and how avoiding with if a readable indentation approach is, I think, recommended, I feel the current behaviour is OK.

Oh, I didn't know that private for DUs was applied for all constructors. I was thinking it was on a case by case basis like member.

With that in mind now, I think that indeed the current behaviour is probably correct.

I am going to close this issue but if maintainers of this repository think that a discussion is still needed, please feel free to re-open it.

nojaf commented 8 months ago

Hello, the with keyword is to be avoided and multiline body constructs typically start at the next line indented. This would be inconsistent with the rest of the guide.