Prefer extension methods to intrinsic properties when arguments are provided #16032
Conversation
… property of same name exists and arguments are passed to it (assuming property is not a function value). Lots of things to sort out, this just takes care of the first layer of interaction between CheckExpression and NameResolution. Given impact on service / tooling, it is not clear that adding an entry in NameResolution.Item is the best choice, may reconsider into changing the return type of ResolveLongIdentInTypePrim instead.
…seems to work on toy example.
… are no extension methods in scope).
|
|
…SharpFunc as type. Tests: covering a bunch of allowed and disallowed shadowing cases.
adding tests for before the feature on all the previous positive tests
…estions#1327) making .net framework tests fail
…ithub.com:smoothdeveloper/visualfsharp into extension-method-priority-when-arguments-provided
|
the right information shows in the tooltip, but it doesn't behave the same as in C#:
|
…lear how many things will be broken, but it does what is expected in VS
…ithub.com:smoothdeveloper/visualfsharp into extension-method-priority-when-arguments-provided
…out extension method only, rather than the kind of all items (type and ctor method were being split, causing a test to fail and more entries in the completion list not related to accessing properties or extension methods).
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Pull request contains merge conflicts. |
…t be overtaken by extension methods.
…w both event and extension method (only event can be called). The new logic 100% preserves the previous behaviour in absence of the particular condition we are interested for in RFC-1137. In case the condition for RFC-1137 hits, we partition the items for those to be split apart (property and extension methods), taking care of discarding items as we yield the expected tuples that correspond to each item and the rest is processed as it was originally. There are still probably loopholes with items showing up separated, but this would only impact if there are actually extension method and property with same name.
…ithub.com:smoothdeveloper/visualfsharp into extension-method-priority-when-arguments-provided
|
@psfinaki I made a better implementation for Also created #16255 in case I have a chance of authoring tests around intellisense. I noticed that even currently, intellisense would list info for items that can't be called:
Somehow, Rider shows them in the more appropriate order for this particular case. To me, this shows that Aside, I'm sensing there are blind spots in the RFC and implementation, if we'd only want the feature to work for the expected use cases (extension methods as they are defined in C# and nothing else, etc.) due to type extension in F# allowing pretty much all kinds of constructs and me having taken the short route. I think this should get some scrutiny and having the team to decide if we keep the knobs set the way it is, or try to shut the door to those possibilities for sake of being conservative. Nonetheless, this implementation is probably passing the bar for a preview feature. |
…n-method-priority-when-arguments-provided
|
The current implementation LGTM, will take another look tomorrow with a fresher head. It would be probably helpful to list all the potential blind spots in one place (e.g. RFC ticket, or here) - so that we can refer to it in future discussions. |
...hecks/TypeExtensions/PropertyShadowing/E_CannotShadowFunctionPropertyWithExtensionMethod.fsx
Show resolved
Hide resolved
|
@psfinaki, would you mind sharing some ideas over more tests for the method resolution? I tired to have thorough checks, and didn't come up with more examples I wanted to add when I was implementing it. I can definitely add more tests in the PR if anyone wants new particular cases, just le me know. |
|
So relooking at the PR now - probably this is good as it is, especially given that you've done some manual testing as well. Let's have this in and see the changes in action. |
There are APIs that define extension methods that bear same name as properties, despite it is not possible to define method with same name as a property in C# or F# (and presumably VB.NET).
For cases such as:
We'd like the extension method to be used as the resolution, instead of a type checking error.
Things to check in tests:
x.Y.Z.ToString())Things to do:
openis going to work with the feature (when opening the namespace with a type after the namespace with the extension method for it, or doing the reverse)Related: fsharp/fslang-suggestions#1039