Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwanted space before DU cases when pattern matching #2369

Closed
1 of 3 tasks
drhumlen opened this issue Jul 16, 2022 · 6 comments
Closed
1 of 3 tasks

Unwanted space before DU cases when pattern matching #2369

drhumlen opened this issue Jul 16, 2022 · 6 comments

Comments

@drhumlen
Copy link

drhumlen commented Jul 16, 2022

Issue created from fantomas-online

Code

type Type =
    | TyLam of Type * Type
    | TyVar of string
    | TyCon of string * Type list
    override this.ToString() =
        match this with
        | TyLam(t1, t2) -> sprintf "(%O -> %O)" t1 t2
        | TyVar a -> a
        | TyCon(s, ts) -> s

Result

type Type =
    | TyLam of Type * Type
    | TyVar of string
    | TyCon of string * Type list

    override this.ToString() =
        match this with
        | TyLam (t1, t2) -> sprintf "(%O -> %O)" t1 t2
        | TyVar a -> a
        | TyCon (s, ts) -> s

Problem description

Fantomas adds a space before the parenthesis for the Discriminated Union cases when pattern matching. According to the F# style guide, there should not be such a space: https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-pattern-matching

It also makes sense to not have it since you contruct your object like let myShape = Rect(4.0, 6.0), so the destructuring should look similar match myShape with Rect(w, h) -> .....

Also worth noting here is that the output from Fantomas does not match the example given in the README because of this behavior (see example above).

Extra information

  • The formatted result breaks my code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas master branch at 2022-07-15T14:30:45Z - 87cd212

Default Fantomas configuration

@nojaf
Copy link
Contributor

nojaf commented Jul 18, 2022

Hello, thank you for raising this issue. It might be me, and I haven't finished my first coffee on a Monday morning, but where does the guide mention this explicitly of the space?

It makes sense to change this but this can be a bit of an impactful change for many users, so I really would like to have crystal clear guidance to be able to justify this change. A code sample in the F# style guide without any explicit text isn't really reliable.

Currently, the setting fsharp_space_before_parameter is being used to determine if there should be a space or not. I believe it might make more sense to use the same rules as for function invocation.
There, depending on the capitalisation of the function name, fsharp_space_before_lowercase_invocation or fsharp_space_before_uppercase_invocation is used.

@drhumlen
Copy link
Author

drhumlen commented Jul 18, 2022

Hello, thank you for raising this issue. It might be me, and I haven't finished my first coffee on a Monday morning, but where does the guide mention this explicitly of the space?

Hmm on reading I can't find anything explicit actually, but all the examples are without the space. I also think of it as exactly the same the construction of something – ie let shape = Rect(2.0, 6.0), and not let shape = Rect (2.0, 6.0), so I think of it as covered "implicitly" by that rule. But some official statement on this would be nice.

Currently, the setting fsharp_space_before_parameter is being used to determine if there should be a space or not. I believe it might make more sense to use the same rules as for function invocation.
There, depending on the capitalisation of the function name, fsharp_space_before_lowercase_invocation or fsharp_space_before_uppercase_invocation is used.

Yea I think that makes sense. It should always follow the same spacing rule as the construction of a DU – which is very similar to an uppercase invocation.

@nojaf Is there anyone at Microsoft/F# committee we can confer with? Philip Carter?

@nojaf
Copy link
Contributor

nojaf commented Jul 19, 2022

Please raise this over at https://github.com/fsharp/fslang-design#style-guide.

@auduchinok
Copy link
Contributor

auduchinok commented Sep 26, 2022

The adding of space is very inconsistent to how expressions are handled. I'd expect Fantomas to use SpaceBeforeUppercaseInvocation setting here.

@auduchinok
Copy link
Contributor

Created an issue: fsharp/fslang-design#712.

@nojaf
Copy link
Contributor

nojaf commented Dec 5, 2022

This is available in the 5.1 series.

@nojaf nojaf closed this as completed Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants