Raise a warning if an extension method is ignored

[abelbraaksma]

Every now and then this pops up again. Since it confuses users massively, even seasoned programmers, I suggest we raise a warning if an extension method is ignored (that is, if it has an intrinsic method on the original type).

Suggested text to be some variant of:

warning FS0XXX: Extension method X.Y.Z is hidden by a definition of a static method on the type X.Y that has the same signature.

To raise a warning was suggested before by @dsyme at the bottom of his comment here.

Repro steps

Consider the following snippet:

type Single with
    /// An override to Single.Parse with fixed InvariantCulture
    static member Parse value = Single.Parse(value, floatNumberStyles, CultureInfo.InvariantCulture)

Expected behavior

Most users would expect the code above to be called when using Single.Parse "4.2". But that never happens, the original static member on the type will be called instead.

Actual behavior

The intrinsic member is called instead.

Known workarounds

Either specify the full namespace to the extension method (assuming it's different) or use a different member name.

Related information

This applies to all kinds of methods, properties etc. Static or otherwise.

[gusty]

Thanks @abelbraaksma for taking the time to materialize the suggestion of the warning.

Regarding your comments

Let's put it this way, by doing a static analysis of your code we can detect that it has a defect.
There we disagree (though that wasn't part of my point of view).

A compiler cannot possibly determine it is defective, simply because the code is legal and the method can be used, albeit only with an FQN (fully-qualified name, i.e. including full namespace).

Well, if a compiler cannot detect it, how can it possibly emits this warning?

The change you seem to be proposing applies to all and any extension methods. Even though the chance is tiny in real-life code out there, it means that people need to be educated / informed and that they need to change their coding habits. It's a language change vs. a library change.

I partially agree, certainly breaking one single method is not the same breaking change as changing the overload resolution of everything. But lets keep in mind that it's not a single method, it's the universe of all methods that will/would be potentially added to the F# core, until someone decides to change this behavior for once.

These are hard decisions and need a lot of consideration, is not that you change it just because someone like me popped in and suggested it ;) ... but I think there should be place to discuss, analyze and eventually introduce this breaking (and at the same time "stop-breaking") change.

As I said, the C# guys changed the overload resolution sometime ago, and they have a very huge code base compared to us.

[abelbraaksma]

Well, if a compiler cannot detect it, how can it possibly emits this warning?

It's about the word "defective". When something is a defect, a compiler can raise an error. If it is something that is maybe defective, the compiler can issue a warning. The programmer can then make a decision to ignore the warning, or add a #nowarn to the code, or fix the code if it is indeed a defect.

Though it'd be interesting how this warning should fare with referenced libraries. The same could apply, but before you know it you'll have a whole sea of warnings (my 2p: it probably shouldn't analyze references).

until someone decides to change this behavior for once.

But how? Letting extension members become the first choice over an existing library member has similar ramifications as the inverse (the current situation). I don't think there's a "good all-encompassing" solution to this (unless we borrow from the famous Eiffel language, which solved this issue in a rather unique way, where the user of a library could make its decision. Similarly, XSLT allows you to pick the "members" you want, and hide the "members" that might cause a conflict).

As I said, the C# guys changed the overload resolution sometime ago, and they have a very huge code base compared to us.

Certainly true. But this started out, historically, for the same reason we are discussing this potential change. When they introduced extension members, the (hidden) signature of such members were different. They were (and are) static classes with the first argument being the this-pointer. Thus, automatically, they didn't typically collide with existing members.

Of course, they could have designed it differently. But it seems to me that, unless we introduce new syntax, we cannot reliably change this behavior in this phase of the language.

[abelbraaksma]

but I think there should be place to discuss, analyze and eventually introduce this breaking (and at the same time "stop-breaking") change.

Certainly! But we should gauge what others at the F# team think of this, then we could spend some time in coming up with a proposal or RFC. I personally think new syntax is a better approach and easier to drive forward, but that doesn't mean we shouldn't look into a way to make this work without new syntax.

[dsyme]

"others at the F# team "

We are all "the F# team" :) Especially when it comes to the language design.

But lets keep in mind that it's not a single method, it's the universe of all methods that will/would be potentially added to the F# core, until someone decides to change this behavior for once.

We very, very rarely add methods to FSharp.Core. (.NET is a different matter) But if a good warning were implemented I think it would solve the problem.

[abelbraaksma]

We very, very rarely add methods to FSharp.Core.

@dsyme, do you mean methods on the type as opposed to extension methods? Because from F# 4.0 to F# 4.1 we saw quite a few new methods in Seq, List and Array, the Result type was added and Option was extended. That doesn't srike me as "very, very rare" ;). And some of these broke my existing codebase when trying to merge to F# 4.1, simply because I had my own versions of these methods, leading to conflicts.

Nothing serious, but just saying that any new function or method introduces a potential backward-compat issue.

[dsyme]

@dsyme, do you mean methods on the type as opposed to extension methods?

Yes - methods on a types, as opposed to either new module-defined functions (common) or new extension methods (vanishingly rare)

[gusty]

But how? Letting extension members become the first choice over an existing library member

Yes, definitely that's the best approach.

has similar ramifications as the inverse (the current situation). I don't think there's a "good all-encompassing" solution to this

No, it's not the same problem if you invert the situation. Let's think about both scenarios:

a) Methods in the type have priority over methods defined as extensions (current situation)

As I said already, this is a source of breaking changes. Dev A develops a library with Type A, then Dev B uses that library and creates extension method B for type A.

In a future release with "no breaking changes", Dev A adds method B to type A. Dev B picks up the new update since it contains "no breaking changes" but now his method B will never execute. It might break or might become a time-bomb, that will break eventually in a specific scenario, as in your example, when using a different language.

This demonstrates that adding any method to a type becomes a breaking change. Even if it's not the F# core library.

b) Extensions methods have priority over methods in the type.

Now Dev A can safely add method B.
Dev B might not be aware of the new method, since it will be overriden in his application. But that's not a breaking change, both libraries will continue to work as before.

Also if, Dev B adds more extensions methods, they will not break anything in Dev A's library, since the dependency is in the other sense.

And that's the key: priority arrows should follow the dependency arrow, that way there will never be a conflict.

Additional considerations: right now the extension method which has priority is the last one that was opened. So, if we consider the whole timeline:

time: past -> present -> future  

priority: lower -> higher 

Type A defined with method A <- ext method A -> ext method A -> ext method A

See? all arrows goes in one direction, except the first one which goes in the wrong way.

Also this is the same philosophy the compiler assumes in almost all situation, I mean values are shadowed, modules, types. Always the last wins, and it makes totally sense.

I propose, let's agree at least that with no doubts this is the best situation. Once we agree with this we can discuss the problems of doing this change or not.

Unless I'm missing something, if so please let me know.

[dsyme]

@gutsy Nice write up. I think the C# team chose the first option for some of these additional reasons

• If you make a reflective call, and it succeeds, you expect it to call the same thing as the static version of a call

• There would no way of calling the original once the extension member was in scope.

• They were nervous about security or code review problems caused by adding an extension method for an existing well known method.

• They were nervous about whether adding a "using" changed the meaning of existing code (obviously that can happen in F# though so it's not really a consideration)

• Adding overloads is known to be a breaking change in any case.

In general C# extension members were seen as "a bit dodgy" when they were added. They were more successful than people expected.

But TBH if C# had gone the other way I would have as well. Your arguments are indeed convincing - except perhaps that I'm not that interested in "theoretical" breaking changes - only their actual incidence in practice - and whether there is a warning given does matter to me.

Given the cost of doing it, I'd be surprised if we made a change here. Probably the best way forward is to collect anecdotes and other evidence showing that people hit the breaking change problem in practice.

[abelbraaksma]

I propose, let's agree at least that with no doubts this is the best situation. Once we agree with this we can discuss the problems of doing this change or not.

I'd love to agree with you, and wanted to, really, but given it more thought I think it is not as obvious as you explain it (great explanation, though!).

An instance method always has precedence over an extension method (see: Closer is Better by Eric Lippert).

You state that it makes sense to let extension methods override instance methods. While there is something to say for this in F#, in more OO-oriented languages it never makes sense. Again, Eric Lippert in this excellent post where he claims that Extension Methods are not Object-Oriented.

In short: instance methods operate on the instance of the type and can modify state (an ugly thing in the functional world, but we cannot bypass this fact). Extension methods do not operate on the type and they cannot change state, instead, they get an instance of the type (or null) as a reference.

Why does this matter? Because it is the OO principle of data + behavior/data-manipulation in one object (totally opposing the principles of functional programming, I know). The instance method has intimate knowledge of the object and can access its internals (private, protected data and members), the extension method does not.

If we were to allow an extension method to override this, we allow an extension method to change the state and (internal) behavior of objects by virtue of not being able to change the state of objects. See the conundrum? Say you have a SetId method and this sets the internal ID. Then any library that would override this with an extension method could change the behavior of this object to become meaningless.

In OO there's a clear way of defining whether or not an object can be extended or not (sealed, virtual), and extension methods were meant to not break this principle.

Taken to an extreme, if you create an ToString() extension method on object or T, would that change all and every ToString() methods? Or worse, GetType()? Or GetHashCode?

I'm not saying that such a paradigm is impossible. In fact, there are languages that support it (JavaScript with their prototype-based paradigm, for instance), but it's a design decision that didn't fit well with the underlying OO system. And if you would allow that, you should also allow extension methods to actually change state. But then we already know how to do that: by extension of the object and overriding protected and virtual members.

Even if we had the possibility to go into another direction today, I believe we shouldn't. And yes, I very much sympathize with the idea and with the possibilities it would bring, and how some problems would be easier resolved, but I do not see the "future - past" analogy, as an extension method is not a future or past of the object, it is a utility function that is easily accessible by a language construct and has (from a language standpoint) little or nothing to do with the actual object. To take the stand: there is no is-a or has-a relation between the object and the object-with-extension-method. Not a grand-father or child, not a nephew or a cousin, there's no interconnection in the relation whatsoever.

BTW: That whole argument is about extension methods from the OO point of view, and I don't know of a .NET language that supports static extension methods other than F#. I do think that extension methods have a whole different meaning in F#, as they are typically static methods on the type (or module let-bindings with a module of the same type), and therefore they don't operate on the type. However, changing extension method resolution for static methods and not for instance methods seems to be a no-go, as that would treat two similar things in different ways, leading to even more tresspassings on the Principle of Least Surprise.

BTW (2): we should consider this SO post by (again) Eric Lippert when we think about the design for the warning. It is not as clear-cut as I would have thought.

[matthid]

I have to say that I support this suggestion.
Obviously, changing priorities is not an option at this point as the decision has been already made.

Though I have to agree that there is something strange about the current situation. From a F# Language standpoint it kind of makes sense to "hide" existing stuff (just like operators for example) in order to provide a more functional API on top. Thinking about ToString and GetHashCode kind of "kills" that feeling (but honestly you can say the same about operators and build-in functions which I have shadowed time and time again...)

As changing the priorities is out of the question I support this proposal as it clearly indicates an error if an extension-method is not taken because there is already an existing method in the object itself. The fix is to change or remove the extension method.

[abelbraaksma]

@mattis, an error? No. A warning? Yes. It can just as well mean that both the extension and original method are fine (I.e. an Option.withDefault in a library to be used in pre 4.1 and 4.1 onwards). With an error, such 'soft' overloads would make using such library impossible.

So I strongly suggest we make it a warning.

[gutsy]

@gusty There was a comment up there for you, but they accidentally tagged me.

[matthid]

@abelbraaksma Yes let me clarify, I never actually said the compiler should emit an error, yes it needs to be a warning.
I said such a situation "indicates" a (programming) error and therefore the compiler should emit a warning (because the compiler cannot actually know that it is an error).

Sorry for being a bit unspecific about that (It's basically what I meant with "I support this suggestion")

[gusty]

Thanks @gutsy however I'm subscribed to the post and I receive it in any case.

@dsyme Thanks for giving some context regarding the historical reasons why we have this situation right now.

I have some comments about those reasons, probably you have the same remarks:

Regarding the reflective call I think you did a breaking change in F# 4.1 which broke some code I had but I welcomed that change and encouraged you to apply it, since it makes more sense. And am still happy with that change.

There would be no way of calling the original once the extension member was in scope.

Well, the solution is easy, just don't bring it in scope :)

They were nervous about whether adding a "using" changed the meaning of existing code (obviously that can happen in F# though so it's not really a consideration)

Yes, in F# you get used to latter is better rather than closer is better, as stated in Eric Lippert's post.

Now, I understand about your concern about the actual code that was/would be broken. On my side I can tell you that since list slices were added to F# it broke a feature I loved that I implemented long time ago in F#+ which btw it was later requested by someone else in the user voice: negative slices and became a very popular one.

@abelbraaksma You gave an interesting explanation and pointed to very interesting articles. The fact that you still disagree with me makes me think I'm giving very bad explanations :) for instance it's about dependencies rather than past/future, I mean who knows who?

I can not have a problem wanting to have more priority than a code that will use my code, simply because I don't know that code. But I don't depend on that code so what's the problem? How does it affects me?

In your example about the ToString, I define my type with a ToString, my code will work well on its own. If someone will take my code and creates an extension method it's his responsibility but it's not breaking my code which will still compile fine and do what it was expected to do. It's his code who will be doing something funny. It doesn't affect me, I'm not forced to use his code.

That's why I consider this priority not a feature, it's rather an un-feature, what is it buying me by assuring that nobody will override my method? Nothing.

While there is something to say for this in F#, in more OO-oriented languages it never makes sense.

Yes, but the opposite priority doesn't makes sense either in OOP, because as explained, it's the whole concept of extension methods which don't fit in pure OOP.

Also note that I never proposed to allow extension methods to know about the internals of the type, so I really don't care about it and OOPs purists would not care either, since they don't consider extension methods at all.

However, changing extension method resolution for static methods and not for instance methods seems to be a no-go, as that would treat two similar things in different ways, leading to even more tresspassings on the Principle of Least Surprise.

I agree but I don't propose that, if we change it we change it everywhere.

Regarding breaking the Principle of Least Surprise, my experience was that the current situation really surprised me when I realized my code stopped working because of a change in new F# version, but for sure the current situation breaks the open-closed principle. And yes, that you mention the keyword sealed, it also breaks that principle, but at least you break it explicitly and for a good reason.

As we know and @matthid gave an example with operators, F# is a language where you can redefine many things, do a lot of things wrong and confuse everybody who uses your library. So writing libraries in F# implies assuming a compromise with this. I personally try not to confuse the users of my libraries, for instance many times when I re-define existing stuff like operators I force them to explicitly open a namespace, with a name/description suggesting what it will do once in scope.

[matthid]

@dsyme It really is worrying how often @gusty has been broken in practice and in the past because of this issue. And he definitely has an argument here that he will probably be broken in the future just because his libraries seem to do a lot of core stuff, which eventually get added with different semantics to FSharp.Core (as the past has shown).

A warning is the absolute minimum to get into a sane state again. (as stated before I'd prefer the priority change as well - but I can understand that there is ?probably? just no way to get that in?)

[kurtschelfthout]

Here's some evidence this comes up in practice ;)

https://github.com/dotnet/corefx/issues/24442