fsharp / fslang-design

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

[style-guide] Parameter owner patterns should be consistent with prefix application expressions #712

Open auduchinok opened 1 year ago

auduchinok commented 1 year ago

Union cases patterns should be consistent with their creation expressions, like this:

let _ = Foo x
let _ = Bar(1, 2)

match s with
| Foo x -> ()
| Bar(1, 2) -> ()

Currently Fantomas adds an extra space in the last pattern, which makes it inconsistent. I propose we should fix this.

dsyme commented 1 year ago

@nojaf My initial decision is to agree with this. Please let me know if you think this is wrong or inconsistent.

nojaf commented 1 year ago

Sounds reasonable. We currently have two settings that control the spaces before upper/lower case invocations (comment).

These could be re-used. Makes sense.

nojaf commented 1 year ago

Hi @dsyme, I've implemented this on Fantomas' side: https://github.com/fsprojects/fantomas/pull/2551 I think we should just go with this, it makes for a more consistent story.

dsyme commented 1 year ago

Great! thank you

dsyme commented 1 year ago

@nojaf @auduchinok Do we need a docs update for this?

nojaf commented 1 year ago

Well, there is no real section on formatting patterns, so maybe we Maybe we need a new section on the level of "Formatting expressions" and "Formatting declarations" called "Formatting patterns"? It could also serve to capture the eventual outcome of https://github.com/fsharp/fslang-design/issues/647.

kerams commented 1 year ago

This is a hill I am willing to die on. I propose we adjust union case construction expressions to mirror the status quo of pattern decomposition instead. The style guide mandates that there be a space between function and tupled arguments (I'd argue this should apply to method invocation too, but that's a different matter). Since class and case constructors can be used like functions, I don't see why the Bar(1, 2) <-> func (1, 2) distinction is desirable.

dawedawe commented 1 year ago

@kerams While being skeptical about changing such a long standing default behaviour, there's a setting that should help: https://fsprojects.github.io/fantomas/docs/end-users/Configuration.html#fsharp_space_before_uppercase_invocation

auduchinok commented 1 year ago

I've also tried to share some context in https://github.com/dotnet/fsharp/pull/15847#discussion_r1307322325.

smoothdeveloper commented 1 year ago

there is also this edge case https://github.com/dotnet/fsharp/issues/15780 for fluent notation that currently prevents method and arguments to be separate.