[style-guide] Extra space in fluent code

[dsyme]

I notice that a sequence of fluent applications is getting an extra space in the last application, so for

xs.map(fun a -> a + 1)
  .filter(fun a -> a > 1)

we get

xs.map(fun a -> a + 1).filter (fun a -> a > 1)

Note the space after the "filter".

I understand why this is happening, and that in some sense it's the consistent application of rules. However I think I'd expect

xs.map(fun a -> a + 1).filter(fun a -> a > 1)

That is, two or more fluent applications all get no space. A single application would still get a space, per the usual rules, e.g.

xs.map(fun a -> a + 1)

gets

xs.map (fun a -> a + 1)

That said, it's a tricky business and stems from the fact that consecuitive fluent notation is only supported in F# by omitting spaces that are otherwise natural.

Issue created from fantomas-online

Code

xs.map(fun a -> a + 1)
  .filter(fun a -> a > 1)

Result

xs.map(fun a -> a + 1).filter (fun a -> a > 1)

Problem description

Please describe here the Fantomas problem you encountered.
Check out our Contribution Guidelines.

Extra information

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

Options

Fantomas 4.6 branch at 10/05/2021 16:21:15 - 835872c91bf490f6aeffb1efc77f2b47f517aec0

Default Fantomas configuration

_{Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?}

[auduchinok]

@dsyme These lowercase method names look very non-idiomatic to me.

The current approach is to add space for lowercase-starting names, since it's the style normally used for curried functions, and not to add space before uppercase-starting names that normally are references to methods. Unfortunately, there's no way to distinguish functions and method invocations based on syntax, and I think it's a very sensible approach with these constraints.

[nojaf]

This is a tricky one. We currently have some settings to control whether there should be a space or not.
fsharp_space_before_lowercase_invocation
fsharp_space_before_uppercase_invocation

So using the default settings, if the function names are uppercase they don't have the space.
Example

If we were to change the current behaviour, it would require input from the style guide as the next person will lovely open an issue that there has been a regression. It might be a good idea to put out some rules when to have a space and when not.

For example, in this case, does the lambda remove the need for a space?
Or would a plain unit also require no space xs.map(fun a -> a + 1).filter(fun a -> a > 1).dump ()?

[dsyme]

The current approach is to add space for lowercase-starting names, since it's the style normally used for curried functions, and not to add space before uppercase-starting names that normally are references to methods. Unfortunately, there's no way to distinguish functions and method invocations based on syntax, and I think it's a very sensible approach with these constraints.

I understand this perspective. That said, in the context of projects like DiffSharp and TorchSharp, lower case fluent is becoming more common (for good reasons - they are mimicking a Python API and it's the right choice in balance). FSharp.Core.Fluent also allows this naming though that's a slightly separate matter.

For example, in this case, does the lambda remove the need for a space?
Or would a plain unit also require no space xs.map(fun a -> a + 1).filter(fun a -> a > 1).dump ()?

I would imagine the rule would be "a chain of fluent of length two or more means no space on application throughout" so xs.whack(a).mole(b) rather than xs.whack(a).mole (b).

I notice that in the default settings for uppercase the presence of parens in the original source is also relevant

I guess chained lower case application (not just List.map(f) but an actual chain xs.map(f).map(g)) is just extremely rare, and that when it it present it represents fluent notation, so removing that last space will be right.

There's no rush with this, I just noticed it when experimenting with fluent notations.

@dsyme These lowercase method names look very non-idiomatic to me.

I get that. The problem is they look really idomatic once you're in the F# world of DiffSharp and TorchSharp (or if coming fresh from Python, Javascript or Scala). And then you come back to .NET-focused programming and think about doing it. Leaky I know.

[auduchinok]

I get that. The problem is they look really idomatic once you're in the F# world of DiffSharp and TorchSharp (or if coming fresh from Python, Javascript or Scala).

Wouldn't it be better to have idiomatic .NET/F# wrappers for these libraries instead of trying to apply a not very compatible Python code style to F#, a different and a mature language, only making experience of using F# inconsistent (thus, harder to learn)?

[nojaf]

I notice that in the default settings for uppercase the presence of parenthesis in the original source is also relevant

Well, no parenthesis leaves us with no other options than having the space anyway right?

xs.Prop.Map b // xs.Prop.Mapb is no longer a function call
xs.Prop.Map(b)

or are you referring to something else?

I would imagine the rule would be "a chain of fluent of length two or more means no space on application throughout" so xs.whack(a).mole(b) rather than xs.whack(a).mole (b).

That could work out although we should probably come up with sufficient examples as the scope will be larger than that.

xs.[x].whack (a) // space or not?
// what happens with long constructs?
xs
   .whack( //
      a)
   .mole(
       // long expression
        b)

The current support now isn't always perfect, check out DotIndexedGetTests.fs and DotIndexedGetTests.fs.
All I'm trying to say is that there might be a bit more than meets the eye here. I'm open to change as long as we consider all things.

[knocte]

Sorry to chime in, but I still don't get why the style guide (and hence, the fantomas defaults) recommends different spacing depending if the function is lowercase or not. (Disclaimer: I'm myself not a fan either of the decision of having F# conventions use lowercase for functions, given that it goes against .NET API style guidelines; and yes I know about the attribute that you can use to decorate your functions to become accessible with an uppercase...) I think the whole thing is very confusing, especially to newcomers (I will not put a "IMHO" here because my opinion is not that humble, I've mentored most of the developers of my company to learn F# so I know a thing or two about that).

[auduchinok]

Sorry to chime in, but I still don't get why the style guide (and hence, the fantomas defaults) recommends different spacing depending if the function is lowercase or not.

@knocte Historically, the space is usually added for curried F# functions applications, like in M.f a b or M.f (a, b) c, and no space is added for .NET methods calls, like in x.M(a) or x.M(a, (b, c)).

The problem is functions and methods applications share the same syntax, and you can't use a single formatting rule for these two different styles without making half of the cases look bad. The current formatting rule was the best heuristic proposed for the problem: normally, functions are lowercase and methods are uppercase (and are not curried), so it works OK in the most of the cases for both functions and methods. The only cases where formatter produces unexpected results are when naming conventions are violated, like here with the lowercase methods.

[knocte]

@knocte Historically, the space is usually added for curried F# functions applications, like in M.f a b or M.f (a, b) c, and no space is added for .NET methods calls, like in x.M(a) or x.M(a, (b, c)).

From the perspective of an F# developer, the only difference between x.foo bar baz and x.Foo (bar, baz) is that the latter is just receiving the arguments via a tuple. One cannot know if Foo is a C# method (.NET?) or an F# method that doesn't use currification.

In fact, the best case in point is when using one only argument: as there is no tuple and no currification, then what should be used here, uppercase or lowercase? The standard/convention would just be much less confusing if an space was added always regardless if it's uppercase or lowercase.

[nojaf]

I still don't get why the style guide (and hence, the fantomas defaults) recommends different spacing depending if the function is lowercase or not.

Sorry for being silent here, I guess I don't really have a good response for this remark. This is a historical thing I guess.

The standard/convention would just be much less confusing if an space was added always regardless if it's uppercase or lowercase.

Yes, for newcomers this would be a lot less confusing. But a lot of people will find a.ToString () very alienating I think. That being said, personally, if you give these things enough time, you eventually get over them. Feel free to raise this in the style guide, open an issue, perhaps it is worth discussing this again.

[knocte]

But a lot of people will find a.ToString () very alienating I think.

It never felt alienating to me, that's why I enable all fantomas' space_before settings in my repos. But the reason might be that I'm biased because in my first .NET years when coding in C# I contributed to many repositories in the Mono ecosystem, and most of them shared some coding guidelines that advocated for adding a space to every invocation: https://www.mono-project.com/community/contributing/coding-guidelines/

perhaps it is worth discussing this again

Let's imagine we opened an issue and after some debate we agreed to change the standard to make it less confusing for newcomers (making the change in the style guide and so fantomas defaulting fsharp_space_before_uppercase_invocation to true), despite the alienating feeling expressed above: we would have solved only one piece of the cake, because then we would still have the inconsistency that fantomas could not add a space when doing fluent calls (and then we would still see the strangeness raised in this bug filed by Don).

So I just realised that, despite my dislike of it, the best thing to do here is actually do the opposite: remove the space when it's not needed. And bringing up xs.Prop.Mapb not being a function call misses the point here, because, obviously, there always needs to be a separation between the function and the argument, but that separation can be a parenthesis instead of a space, and it turns out that in F#, two braces together () is already the magical unit argument, but that doesn't mean that we need a separator for it, because it's a special argument which is not composed of alphanumeric characters so there is no need to treat it like every other possible argument.

Therefore, the idea would be the following (let's see if I don't miss any of the possible cases):

• Invocation with no arguments (no space needed): foo() or Foo()
• Invocation with one argument (a space needed, unless parenthesis are already there): foo bar or foo(bar) or Foo bar or Foo(bar)
• Currified invocation of two arguments (spaces needed, unless parenthesis already there): foo bar baz or foo(bar)(baz) or Foo bar baz or Foo(bar)(baz)
• Non-currified invocation of two arguments, or invocation of one tuple-argument (parens are already there, therefore no space needed): foo(bar, baz) or Foo(bar, baz)
• (The most convoluted case: ) Currified invocation two arguments, both arguments being a tuple (despite spaces are not needed, we can just add them to make it more visually appealing and consistent with foo bar baz): foo (bar1, bar2) (baz1, baz2) or Foo (bar1, bar2) (baz1, baz2)

If the coding style recommended the above style, there would be consistency across uppercase/lowercase invocations and across fluent calls (so, we could just make fantomas default to this, and converge the settings fsharp_space_before_uppercase_invocation and fsharp_space_before_lowercase_invocation into just a single one fsharp_always_space_before_invocation which would default to false, and only switching it to true would mean seeing some inconsistencies).

If it sounds too good to be true, let me know what did I miss, but if not, I can raise this in the style guideline as an issue or a PR.

[knocte]

we can just add them to make it more visually appealing and consistent with foo bar baz

And when I wrote this I fell into my own trap, because then the last case above that I described as most convoluted case, wouldn't be able to be used in a fluent call: Foo (bar1, bar2) (baz1, baz2).Foo (bar3, bar4) (baz3, baz4) wouldn't work AFAIU, so then we need to drop the spaces here too: Foo(bar1, bar2)(baz1, baz2).Foo(bar3, bar4)(baz3, baz4) just in case (even if the above would be very rare).

[smoothdeveloper]

a not very compatible Python code style to F#

I'd say it is subjective (like my feedback), I think the idea of F# being it's own, in terms of feel of the language, irrespective of being firstly a dotnet language, doesn't make the style incompatible/not very compatible to me.

At some points, those conventions, that were set in framework design guidelines, they pertain to a very OO centric approach, with few distinctions, to make it look like "not a 1 to 1 of java", and I don't feel enforcing those same guidelines, in essence, is what is going to foster the ecosystem the most, for adoption, versus other concerns in the grand scheme.

[dsyme]

Getting back to the OP, I will make a proposal to the style guide:

chained-lowercase-dot-notation-applications of length >= 2 do not add spacing on the final application

I think this is consistent - we either remove all the spaces (for N > 2) or none. The current situation of removing all but one of the spaces is just odd.

[edgarfgp]

In Fabulous v2 we have a new DSL a la SwiftUI . the existing way causing some formatting issues . We added

fsharp_space_before_parameter = false
fsharp_space_before_lowercase_invocation = false

to out .editorconfig to make it more consistent

(HStack(16.) {
    Label("New address")
        .textColor(Colors.StandardTextColor.Light, Colors.StandardTextColor.Dark)
        .alignStartHorizontal(true)
        .verticalTextAlignment(TextAlignment.Center)

    TextButton(AppResources.Cancel, CancelManualAddress, true)
        .alignEndHorizontal()
}).gridRow(0)