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

Allow piping into methods #1118

Closed
4 of 5 tasks
pbiggar opened this issue Feb 18, 2022 · 12 comments
Closed
4 of 5 tasks

Allow piping into methods #1118

pbiggar opened this issue Feb 18, 2022 · 12 comments

Comments

@pbiggar
Copy link

pbiggar commented Feb 18, 2022

I propose we allow piping into method calls. For example this code:

let configureApp (appBuilder : WebApplication) =
  appBuilder
  |> fun app -> app.UseServerTiming()
  |> configureStaticContent
  |> fun app -> app.UseHttpsRedirection()
  |> addRoutes
  |> fun app -> app.UseRouting()
  |> LibService.Kubernetes.configureApp LibService.Config.apiServerKubernetesPort
  |> addRoutes
  |> ignore<IApplicationBuilder>

would become:

let configureApp (appBuilder : WebApplication) =
  appBuilder
  |> .UseServerTiming()
  |> configureStaticContent
  |> .UseHttpsRedirection()
  |> addRoutes
  |> .UseRouting()
  |> LibService.Kubernetes.configureApp LibService.Config.apiServerKubernetesPort
  |> addRoutes
  |> ignore<IApplicationBuilder>

.NET code often is designed with for a "fluent" approach that allows for easy method chaining of C# code. It is more idiomatic in F# to use the pipe operator and pipelines instead of method chaining. However, using pipelines is awkward when it runs into common C#/.NET-designed interfaces.

The code above is adapted from configuring an Aspnetcore application on .Net 6, but I have felt this pain in lots of places, and have many pipelines that contain fun x -> x.method().

Another class of places I have felt this pain is:

let instant = (toZonedDateTime dt).ToInstant()

where I need to wrap an F# function call expression in parentheses in order to call a method on it. This would be much nicer as:

let instant = dt |> toZonedDateTime |> .ToInstant()

This is particularly frustrating as I need to move my cursor to the start of the expression to add the opening (, while I could simply type |> otherwise.

This proposal is based on Elm, which has a delightful record access syntax:

The second way to access records is with a special function .name that is equivalent to (\r -> r.name), making things a bit more concise.

Pros and Cons

The advantages of making this adjustment to F# are less verbose code, and better integration of fluent-style APIs into F#.

The disadvantages of making this adjustment to F# are a new syntactic tool, which would initially look a little weird to newcomers.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S? Feels like it could mostly happen in the early stages of compilation. But I really don't know.

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • 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.

Please tick all that apply:

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

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

@cartermp
Copy link
Member

There's a bit of relevant discussion here that may be worth looking at: #506 (comment)

It's an enormous thread so I don't expect anyone to read years of commentary.

In theory I would like to have this supported. But I can't seem to land on a syntax I'd like.

@pbiggar
Copy link
Author

pbiggar commented Feb 18, 2022

Thanks @cartermp. My searches did not bring up that proposal (nor #159, which I now see). Is there value in this or should I close it as a dup?

@cartermp
Copy link
Member

Value for now I think 🙂

@Astrinus
Copy link

@cartermp Do you think that qualifying the method with the class name (and without parentheses) would be better?

let configureApp (appBuilder : WebApplication) =
  appBuilder
  |> WebApplication.UseServerTiming
  |> configureStaticContent
  |> WebApplication.UseHttpsRedirection
  |> addRoutes
  |> WebApplication.UseRouting
  |> LibService.Kubernetes.configureApp LibService.Config.apiServerKubernetesPort
  |> addRoutes
  |> ignore<IApplicationBuilder>

@pbiggar
Copy link
Author

pbiggar commented Feb 20, 2022

Do you think that qualifying the method with the class name (and without parentheses) would be better?

If I'm calling a method, then I shouldn't have to know what class name it belongs to - this kinda defeats the purpose of methods.

@vzarytovskii
Copy link

Do you think that qualifying the method with the class name (and without parentheses) would be better?

If I'm calling a method, then I shouldn't have to know what class name it belongs to - this kinda defeats the purpose of methods.

Not to mention inheritance can somewhat complicate things.

@dsyme
Copy link
Collaborator

dsyme commented Feb 24, 2022

That thread Phillip linked is indeed huge. This looks ok for this sample:

let configureApp (appBuilder : WebApplication) =
  appBuilder
  |> _.UseServerTiming()
  |> configureStaticContent
  |> _.UseHttpsRedirection()
  |> addRoutes
  |> _.UseRouting()
  |> LibService.Kubernetes.configureApp LibService.Config.apiServerKubernetesPort
  |> addRoutes
  |> ignore<IApplicationBuilder>

@pbiggar
Copy link
Author

pbiggar commented Feb 24, 2022

That thread Phillip linked is indeed huge. This looks ok for this sample:

let configureApp (appBuilder : WebApplication) =
  appBuilder
  |> _.UseServerTiming
  |> configureStaticContent
  |> _.UseHttpsRedirection
  |> addRoutes
  |> _.UseRouting
  |> LibService.Kubernetes.configureApp LibService.Config.apiServerKubernetesPort
  |> addRoutes
  |> ignore<IApplicationBuilder>

I'm suprised it doesn't include the (). From the lack of a (), I imagine that _.UseServerTiming returns a field, not calls a method.

@dsyme
Copy link
Collaborator

dsyme commented Feb 24, 2022

I'm suprised it doesn't include the (). From the lack of a (), I imagine that _.UseServerTiming returns a field, not calls a method.

Yes you're right it should, I've edited

@cartermp
Copy link
Member

I think the _.Method() syntax would be pretty neat.

One thing that I'll add is that this would imply some interesting tooling work so that meaningful completion lists can be generated. Related to the general issue of "meaningful completion within pipelines" wherein it would be good to read the type from the LHS of the pipeline operator and use that to inform what the completion list on the RHS does.

@pbiggar
Copy link
Author

pbiggar commented Feb 28, 2022

One thing that I'll add is that this would imply some interesting tooling work so that meaningful completion lists can be generated. Related to the general issue of "meaningful completion within pipelines" wherein it would be good to read the type from the LHS of the pipeline operator and use that to inform what the completion list on the RHS does.

This is what we do in @darklang in this situation, and it's quite effective.

@dsyme
Copy link
Collaborator

dsyme commented Jun 16, 2022

I'll close as a duplicate of #506 - this is an informative discussion which can inform some choices there

@dsyme dsyme closed this as not planned Won't fix, can't repro, duplicate, stale Jun 16, 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

5 participants