fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
346 stars 21 forks source link

Func<> and Delegate implicit conversion #1131

Open lucasteles opened 2 years ago

lucasteles commented 2 years ago

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:

Please tick all that apply:

For Readers

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

charlesroddie commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 1 year ago

@jkone27 I believe this https://github.com/dotnet/aspnetcore/pull/46898 will do the work for that

charlesroddie commented 1 year ago

@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: https://github.com/dotnet/fsharp/issues/16219#issue-1973994013 .

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 commented 1 year ago

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

jkone27 commented 9 months ago

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

oleksandr-bilyk commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Just to note: attributes in lambdas is a separate can of worms, and are not currently allowed (see discussion here: https://github.com/fsharp/fslang-suggestions/issues/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.