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

Func<> and Delegate implicit conversion #1131

Open
3 of 5 tasks
lucasteles opened this issue Apr 18, 2022 · 10 comments
Open
3 of 5 tasks

Func<> and Delegate implicit conversion #1131

lucasteles opened this issue Apr 18, 2022 · 10 comments

Comments

@lucasteles
Copy link

lucasteles commented Apr 18, 2022

I'm introducing F# to some C# people using just .NET common frameworks and libs (Asp.net, EF Core, etc), I know that we have a lot of very good libs made to be functional friendly. But I saw a bit of cognitive load to people that already know very well common .NET stacks and are learning functional programming and having to learn other libs with it.

This proposal/discussion is about one pain point I saw in this process, which is how F# deals with Func<>, Delegates, and Action<>, which are the common way to express lambdas and some functional style APIs in .NET.

I think the easiest case to show is trying to use the new ASP.NET 6 minimal API.

    let main args =
        let builder = WebApplication.CreateBuilder(args)
        let app = builder.Build()
        
        app.MapGet("/hello/{userId}", (fun (userId: int)  -> $"hello {userId}")) |> ignore // first try, not compile
        app.MapGet("/hello/{userId}", Func<_,_>(fun (userId: int) -> $"hello {userId}")) |> ignore // works, ugly
        
        app.Run()
        0

This kinda thing happens when doing stuff with other libs like EF too.

Other problem I see are some cases which at first glance looks inconsistence:

    type Foo =
        static member func(f: Func<int, int>) = f.Invoke(1)
        static member deleg(f: Delegate) = f.DynamicInvoke(1)
    
    
    let func (f: Func<int,int>) = f.Invoke(1)
    let deleg (f: Delegate) = f.DynamicInvoke(1)
    
    let inc x = x + 1
    
    func inc // ok 
    func (fun x -> x + 1) // error
    func (Func<_,_>(fun x -> x + 1)) // ok, ugly
    deleg inc // error
    deleg (Func<_,_>(fun x -> x + 1)) // ok, ugly
    Foo.func inc // ok
    Foo.func(fun x -> x + 1) // ok
    Foo.deleg(fun x -> x + 1) // error
    Foo.deleg(Func<_,_>(fun x -> x + 1)) // ok, ugly

So, the idea of this proposal is to minimize or remove this friction with the BCL. Allowing these functions to be cast implicitly.

() -> 'b => Func<'b>
'a -> 'b => Func<'a, 'b>
'a -> () => Action<'a>
'a -> 'b => Delegate

I know that we dislike this kind of implicit casting, but I feel in this case It would be good, almost the same case as task and async

UPDATE:

For the same purpose I would like to put Expression in this discussion to:

    type Foo =
        static member funcExp (f: Expression<Func<int,int>>) = f.Compile().Invoke(1)
    
    let funcExp (f: Expression<Func<int,int>>) = f.Compile().Invoke(1)
       
    let inc x = x + 1
       
    funcExp inc // ok 
    funcExp (fun x -> x + 1) // error
    funcExp(<@ fun x -> x + 1  @>) // error
    Foo.funcExp inc // ok
    Foo.funcExp(fun x -> x + 1) // ok
    Foo.funcExp(<@ fun x -> x + 1  @>) // error
    
    let ExprToExpression body = // Expressions<> are kinda common, I believe this should have less attrition
        let lambda = (Microsoft.FSharp.Linq.RuntimeHelpers.LeafExpressionConverter.QuotationToExpression body :?> MethodCallExpression).Arguments[0] :?> LambdaExpression
        Expression.Lambda<Func<'a, 'b>>(lambda.Body, lambda.Parameters) 
    
    Foo.funcExp (ExprToExpression <@ fun x -> x + 1  @>) // ok
    funcExp(ExprToExpression <@ fun x -> x + 1  @>) // ok

Pros and Cons

The advantages of making this adjustment to F# are we are removing learning noise and friction within .NET

The disadvantages of making this adjustment to F# are that it would be more implicit things.

Extra information

Estimated cost (XS, S, M, L, XL, XXL):

Related suggestions: (put links to related suggestions here)

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.

@charlesroddie
Copy link

charlesroddie commented Apr 18, 2022

op_Implicit after https://github.com/fsharp/fslang-design/blob/main/FSharp-6.0/FS-1093-additional-conversions.md is applied to functions and methods, so that would create a precedent for extending Func<'b> and Func<'a, 'b>, and Action<'a> from methods to functions.

Aside from consistency, I don't think it's a large advantage as applying to methods tends to push the F#/.Net conversions to the boundary of F# code which is a good thing.

I'm strongly against any implicit conversion to Delegate. The fact that Delegate requires extra effort is a good thing, since any use of nongeneric Delegates is a code smell (potential for type errors) and this should be visible. Ideally the extra code should have the word Delegate but Func<> is still better than nothing. Even better - in the future when we have analyzers - an analyzer warning.

@lucasteles
Copy link
Author

@charlesroddie I had the same feeling about Delegate. but I changed my mind because I don't see it being used a lot even in C#.
I start to think about Delegate like an obj for functions.

@jkone27
Copy link

jkone27 commented Sep 15, 2022

this would remove the need for typing in aspnetcore pipeline correct? as the delegate would be inferred automatically, that would be awesome..

what about Task and Task ?
is there a suggestion for that too?

would be easier also to have "automatic translation" of Task :> Task as many dotnet apis use Task only

.Use(fun httpContext (next: RequestDelegate) ->
      task {
          do! next.Invoke(httpContext)
       } :> Task
)

@lucasteles
Copy link
Author

@jkone27 I believe this dotnet/aspnetcore#46898 will do the work for that

@charlesroddie
Copy link

charlesroddie commented Nov 11, 2023

@charlesroddie I had the same feeling about Delegate. but I changed my mind because I don't see it being used a lot even in C#. I start to think about Delegate like an obj for functions.

Use of obj (for improper purposes, i.e. for purposes other than ToString/GC) is highly suspect and F# is looking into analyzer strategies to allow users to be warned about uses of obj apis: dotnet/fsharp#16219 (comment) .

Delegate is worse as it doesn't have the valid uses that obj has. Delegate should not be allowed to creep in to F# code more than necessary - only at the edges where required by bad apis. Implicit conversion works against this so should not be allowed. Any allowance of this needs to be off by default or warn by default.

@smoothdeveloper
Copy link
Contributor

If we do this, which will change the semantic around method resolution with delegates arguments, it would make sense to tackle dotnet/fsharp#11534.

@jkone27
Copy link

jkone27 commented Mar 4, 2024

is this going to make it somehow in F# 9? is there any general workaround or external library to adress this?

@oleksandr-bilyk
Copy link

oleksandr-bilyk commented Sep 10, 2024

Please make implicit function cast. Here is the sample of ASP.NET minimal API.

app.MapGet("azureDevOpsTaskApi/GetApprovedDocumentByReleaseV2", 
    Func<
        string, 
        string, 
        string, 
        string, 
        string, 
        string, 
        string, 
        AzureDevOpsApprovalTaskApiHandlerService, 
        Task<IResult>
        >(
        fun ([<FromHeader>]authorization: string)
            ([<FromQuery>]system_TeamFoundationCollectionUri: string)
            ([<FromQuery>]system_TeamProjectId: string)
            ([<FromQuery>]system_HostType: string)
            ([<FromQuery>]build_BuildId: string)
            ([<FromQuery>]release_DefinitionId: string)
            ([<FromQuery>]release_ReleaseId: string)
            ([<FromServices>]service: AzureDevOpsApprovalTaskApiHandlerService) -> 
            let input: RequestQueryV2 = {
                System_AccessToken = authorization
                System_TeamFoundationCollectionUri = system_TeamFoundationCollectionUri
                System_TeamProjectId = system_TeamProjectId
                System_HostType = system_HostType
                Build_BuildId = build_BuildId
                Release_DefinitionId = release_DefinitionId
                Release_ReleaseId = release_ReleaseId
            }
            service.GetApprovedDocumentByReleaseV2(input)
    )
) |> ignore

@dsyme
Copy link
Collaborator

dsyme commented Sep 10, 2024

Marking as approved for the additional F# function to Func/Action conversions, with Expression also taken into account.

Some of these are present in F# already and careful examination will be needed to determine why they are not applying in all practical cases

There are also some other workarounds possible, see https://github.com/mchylek/Endpoints.FSharp.Interop/blob/main/samples/FSharpWeb/Program.fs for example.

The conversions to Delegate are approved too if further examples indicate they are really necessary, though @charlesroddie is right that, like obj, it's a total code smell that needs extra justification and a warning should perhaps be emitted by default. Anyone designing frameworks that routinely accept Delegate need to have a serious think about their choices in life - and reflect on whether they have earned the moral right to inflict that lack of strong typing on the universe.

@vzarytovskii
Copy link

vzarytovskii commented Sep 10, 2024

Just to note: attributes in lambdas is a separate can of worms, and are not currently allowed (see discussion here: #984). So if we are to make conversion, it will still not going to work with attribute-based minimal APIs.

We might need to allow attributes for lambdas which are to be converted to delegates but not on the rest.

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

7 participants