fsharp / fslang-suggestions

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

Prefer extension methods to intrinsic properties when arguments are provided #1039

Closed laenas closed 9 months ago

laenas commented 3 years ago

I propose we add support for C#-style extension methods which share a name but not a type with a property on a type. Some libraries (for example, Spectre.Console) use this behavior to enable extensions methods to create fluent APIs for the setting of values on their types.

Consider the following abstracted type definition:

open System.Runtime.CompilerServices

type Foo() = 
    member val X : int = 0 with get,set

[<Extension>]
type FooExt =
    [<Extension>]
    static member X (f: Foo, i: int) = f.X <- i; f

let f = Foo()
f.X(1)

What we should be able to do is dot through the assignment:

printfn "%i" f.X  //This should print 1
printfn "%i" (f.X(2)) //This would now print 2

While the benefits are not immediately apparent in an F#-centric approach, consider from an interop perspective (both consuming and being consumed from C#) and in the case of multiple properties:

//Every C# dev knows this pain:
var f = new Foo();
f.X = 1;
f.Y = 2;

//But this change enables this - which is why libraries use it:
var f = new Foo().X(1).Y(2);

The existing way of approaching this problem in F# is: To achieve the same effect in F#, you need to create an entirely new set of wrapper functions - either type extensions that have names which don't conflict with existing properties, or a module of helper functions that can be pipelined instead of called fluently.

Pros and Cons

The advantages of making this adjustment to F# are a greater degree of interop with behaviors made possible in the CLR - and which are already being seen in the wild in C# projects and libraries which F# consumers have a use for, as well as enabling F# to expose similar behaviors to C# consumers who don't have the ability to pipeline in an F#-native way.

The disadvantages of making this adjustment to F# are the additional complexity of overload resolution, both from a compiler standpoint and from the possibility of introducing subtle confusion into compiler error messages when a user accidentally uses the property's getter instead of invoking the method of the same name, or visa versa.

Extra information

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

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.

Additional References

dsyme commented 3 years ago

@laenas I think the proposal you want it "prefer extension methods to intrinsic properties when arguments are given"

Could you provide a code sample too please?

laenas commented 3 years ago

@dsyme Updated, but I'm curious about the concept of preference here - in the case that we're defining an extension method on top of an intrinsic property of a function-type, for example, I'd think we want to prefer the intrinsic (and possibly just disallow or warn about the extension method being hidden for this very reason) - am I underthinking something? 😅

type Bar() =
    member val B : (int -> unit) = (fun i -> printfn "%i" i) with get,set

[<Extension>]
type BarExt =
    [<Extension>]
    static member B (b:Bar, _:int) = "Nothing yet"

let b = Bar()
b.B(1)

Works as I'd expect, for example - just ignoring the extension method and printing 1, because the intrinsic takes priority.

dsyme commented 3 years ago

Updated, but I'm curious about the concept of preference here - in the case that we're defining an extension method on top of an intrinsic property of a function-type, for example, I'd think we want to prefer the intrinsic (and possibly just disallow or warn about the extension method being hidden for this very reason) - am I underthinking something? 😅

Yes, this would be a breaking change for this case. I doubt it occurs in practice (why would anyone have defined the unusable extension method?), but we could code a specific rule to prefer a function-type intrinsic property just in case.

laenas commented 3 years ago

Yes, this would be a breaking change for this case. I doubt it occurs in practice (why would anyone have defined the unusable extension method?), but we could code a specific rule to prefer a function-type intrinsic property just in case.

But that's just it - I'm not really advocating for changing the preference here - so much as just enabling us to properly consume non-conflicting extension methods (that overload a name). If they happen to get hidden by intrinsics in these edge cases, well, maybe some analysis and a compiler warning - but that's at least solvable. The fact that we can't even consume them as intended is problematic, and seems to be owing to that very sharp divide made of deciding immediately whether a given identifier belongs to a Property or a Method on a type. We check properties first - which is on the surface fine - but in the case that the typecheck fails, rather than aborting out it seems to make sense that we should fallback and at least see if there is something else available for that identifier that would fit.

dsyme commented 3 years ago

and seems to be owing to that very sharp divide made of deciding immediately whether a given identifier belongs to a Property or a Method on a type.

Both C# and F# make this distinction early - we do not allow overloading involving both properties and methods, and indeed non-indexer properties can't be involved in overloading at all.

I don't think we really want to modify this restriction to allow this in the door.

laenas commented 3 years ago

I must be conflating and misusing some terminology somewhere here 😅 Here's the simplest interactive/REPL-able side-by-side repro of the problem, in the wild: In C#, this functions:

#r "nuget: Spectre.Console"

using Spectre.Console;

var table = new Table();
table.Width = 100;
Console.WriteLine(table.Width); //100
table.Width(200);
Console.WriteLine(table.Width); //200

(ref: https://github.com/spectreconsole/spectre.console/blob/865552c3f2046282519b85e543ed516142f7785a/src/Spectre.Console/Widgets/Table/Table.cs#L55 & https://github.com/spectreconsole/spectre.console/blob/865552c3f2046282519b85e543ed516142f7785a/src/Spectre.Console/Extensions/TableExtensions.cs#L152 )

In F#, compiler error because it doesn't 'see' that you're actually attempting to access the extension method.

#r "nuget: Spectre.Console"

open Spectre.Console

let table = Table()
table.Width <- 100
printfn "%A" table.Width
table.Width(200) //FS0003 - This value is not a function...
printfn "%A" table.Width

If you try to define a property and method of the same name intrinsic to a class, you do get an error in C# - in alignment with the error for doing the same thing in F#: you get divergent behavior once extension methods are taken into account. C# will absolutely tolerate this sort of overloading, and I use Spectre as an example because it's something I've seen come up in community discussions a few times now since their documentation uses this feature and causes issues for folks trying to interop from F#.

dsyme commented 3 years ago

I believe the difference between C# and F# is that C# see table.Width(100), decides it is a method call (though it could be an invoke of a delegate-valued property), so goes looking to resolve to methods.

F# looks at table.Width (ignoring the arguments) and decides it is better to resolve to an intrinsic property.

laenas commented 3 years ago

Yep, that's a reasonable description of what's going on in the typechecker when this throw the error. And we do 'know' towards the top of this process that the next thing 'in line' is going to be an App, so it just feels like we should probably be able to resolve this in a similar manner without actually breaking any present behavior - albeit with some possible shaking up of the typechecker call chain.

hvester commented 3 years ago

When I encountered this some time ago I wasn't able to figure out a workaround without asking in Stackoverflow: https://stackoverflow.com/questions/66368514/how-to-call-extension-method-when-there-is-a-property-with-same-name

charlesroddie commented 3 years ago

This feature is supposed to create an alternative to:

//Every C# dev knows this pain:
var f = new Foo();
f.X = 1;
f.Y = 2;

However in both C# and F# there is better syntax for this.

var f = new Foo(X=1, Y=2)

That said, the feature may be required for C# compat as in @hvester 's post.

laenas commented 3 years ago

This feature is supposed to create an alternative to:

//Every C# dev knows this pain:
var f = new Foo();
f.X = 1;
f.Y = 2;

However in both C# and F# there is better syntax for this.

var f = new Foo(X=1, Y=2)

That said, the feature may be required for C# compat as in @hvester 's post.

public void Foo(Bar b)
{
  b.PropA = 1;
  b.PropB = 2;
}

//becomes
public void Foo(Bar b)
{
  b.A(1).B(2);
}

Constructor-equivalence isn't the point, it's the ability to effectively 'extend' properties with fluent setters.

baronfel commented 2 years ago

From a practical perspective - Spectre.Console is pretty much the terminal library in .Net, and if we can't use it (or make it easy for newcomers to use it) that looks very bad for F#. I don't think this is something we can just ignore.

dsyme commented 2 years ago

It's unfortunate. C# only supports extension methods. It always prefers methods on the original type. But if there's a property with that name, it doesn't care and prefers the extension method anyway. TBH no one should be designing APIs where an extension method conflicts with an intrinsic property on the existing type.

I'm not at all sure what the resolution of this should be. In the case where arguments are give, I guess we could prefer the extension method over intrinsic properties. But even that would be a breaking change in some corner cases (the property would have to have F# function type - which is not going to happen for .NET-defined types)

Note that as a workaround you can call the extension method using the explicit call

FooExt.X(foo, 4)

or use this technique to define a new extension method giving a different name:

    [<Extension>]
    static member X2 (f: Foo, i: int) = FooExt.X(f, i)
maciej-izak commented 1 year ago

Any info on this? I have an increasing number of C# libraries where this pattern occurs, and the resulting cooperation problems between F# and C# are becoming very annoying. :(

gusty commented 1 year ago

Not the same at all, but slightly related issue: https://github.com/dotnet/fsharp/issues/3692

smoothdeveloper commented 1 year ago

@maciej-izak could you show some examples of C# and client F# code?

If it is about setting properties, you could make a single extension method in F# for the type, and pass it the property assignment:

open System.Runtime.CompilerServices

type Foo() = 
    member val X : int = 0 with get,set

[<Extension>]
type FooExt =
    [<Extension>]
    static member Set (f: Foo) = f

let f = 
  Foo((* somehow the user doesn't want to set properties here*))
    .Set(X=1)
printfn $"%i{f.X}"

@maciej-izak, assuming @dsyme would concede to the pattern (despite it is not nice one, why not use the SetPropertyName or WithPropertyName convention rather than rely on obscure overload resolution) and approve the suggestion despite breaking change; would you like to implement it?

@gusty, do you know if it is possible to make a function that is like flip that would work on any method, and puts the first argument as the last and untuple the arguments? It could be used to turn the extension methods into functions that you can pipe into, passing the this last.

gusty commented 1 year ago

Yes, it's possible. I just tried modifying the generic curryN I did for F#+ and it works,

maciej-izak commented 1 year ago

@smoothdeveloper sure here you can check the problem, fsharp can't consume extension methods with the same name as properties, there was needed new package and special fix to solve F# limitations (!), here is detailed description :

https://github.com/wieslawsoltes/NXUI/issues/21

ExtensionsGenerator.cs

Whole NXUI is one big example :). I found this problem also few times for other libs like dnLib and cecil

smoothdeveloper commented 1 year ago

I did some digging/note to self:

https://github.com/dotnet/fsharp/blob/3df945aa3a85b2562864b393aabad39506a13f72/src/Compiler/Checking/NameResolution.fs#L2609 is the place where property is looked up first

a bit after is the check for methods / extension methods:

https://github.com/dotnet/fsharp/blob/3df945aa3a85b2562864b393aabad39506a13f72/src/Compiler/Checking/NameResolution.fs#L2626-L2631

I'll ponder some more on a good way to make the resolution more explicit, it currently relies on "head of the list" being picked when there are several things, I think wrapping both MethInfo and PropInfo with some more metadata, in a single case of HierarchyItem would help make things more explicit, even if we end up putting all the stuff in the right order.

smoothdeveloper commented 1 year ago

Another source of ambiguity is indexed properties, they allow notation such as

"abc".Chars 0

type Foo() =
   member _.X 
      with get (x,y) = x,y 
      and set (x,y) value = ()

let f = Foo()
f.X (1,"")
f.X (1,"") <- 3
edgarfgp commented 9 months ago

Can this be closed based on https://github.com/dotnet/fsharp/pull/16032 ? cc @vzarytovskii

vzarytovskii commented 9 months ago

Can this be closed based on https://github.com/dotnet/fsharp/pull/16032 ? cc @vzarytovskii

I'd say so, yeah, thanks for noticing