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

Proposal: Expand Extension Methods visibility #7489

Closed
ghord opened this issue Dec 15, 2015 · 19 comments
Closed

Proposal: Expand Extension Methods visibility #7489

ghord opened this issue Dec 15, 2015 · 19 comments

Comments

@ghord
Copy link

ghord commented Dec 15, 2015

There is a pain point with extension methods that occurs when you are using a type and you don't have it's namespace referenced:

namespace Namespace1
{
    public interface IFoo
    {
        void Bar1();
    }

    public static class FooExtensions
    {
        public static void Bar2(this IFoo foo) { }
    }
}
//in some other namespace
var myFoo = SomeMethodToGetFoo();

//this obviously compiles even though Namespace1 is not referenced
myFoo.Bar1();

//must have using Namespace1 to compile
myFoo.Bar2();

This is especially painful in libraries like EntityFramework, where you get classes with a lot of extension methods like DbSet and you cannot use them unless you reference System.Data.Entity. It hinders discoverability of such extension methods.

I propose to relax the extension methods visibility rules when declared in the same namespace as extended type (or it's base types/interfaces), so that it is visible in file even if its namespace is not referenced.

This could cause some breaking changes in the rare cases of multiple extension methods of the same name, in which case I would recommend to consider old extensions methods introduced with using before those new "unreferenced" extension methods.

@paulomorgado
Copy link

This is not a breaking change as would make code that didn't compile before compile now.

However, if this were to be implemented, from that point on, adding a using directive to the code might break the code.

Rather than relaxing the language, I would favor tooling to add the needed using directive.

@ghord
Copy link
Author

ghord commented Dec 15, 2015

However, if this were to be implemented, from that point on, adding a using directive to the code might break the code.

You mean break compilation? Extension methods introduced with using would take precedence, as proposed.

@paulomorgado
Copy link

Not necessarily break compilation. It might still compile but using another extension method from another class.

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

What if there was two exact extension methods in the references assemblies? so without doing nothing (like using directive for both namespaces) there will be an ambiguity. For this to be more explicit and under control of the developer, I think it'd better to be the way it is now. As @paulomorgado said, it can be just a matter of tooling.

@ghord
Copy link
Author

ghord commented Dec 15, 2015

What if there was two exact extension methods in the references assemblies? so without doing nothing (like using directive for both namespaces) there will be an ambiguity.

That would be ambiguous match and would fail with a compile error. I see no problem there.

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

@ghord How am I supposed to prevent that?

@ghord
Copy link
Author

ghord commented Dec 15, 2015

@alrz Could you give an example of such case?

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

Currently this produces a compiler error,

using N1;
using N2;

namespace N1
{
    static class C
    {
        public static void Foo(this int obj) { }
    }
}
namespace N2
{
    static class C
    {
        public static void Foo(this int obj) { }
    }
}
namespace N3
{
    class C
    {
        static void Main()
        {
            1.Foo(); // CS0121
        }
    }
}

Well, I should remove the undesired using directive. But under your proposal, it will an error even without any using directive, and it seems that there is no way to prevent that.

@ghord
Copy link
Author

ghord commented Dec 15, 2015

If you notice, my proposal does not widen the visibility of all extensions methods. It only works on those in the same namesapce as extended type. I fear that otherwise it would cause problems with tooling/compilation. So the only case there if would pick up Foo method is if it were defined in System namespace (like System.Int32).

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

@ghord Still, issue remains,

namespace N1
{
    public interface IFoo
    {
        void Bar1();      
    }
    public static class FooExtensions
    {
        public static void Bar2(this IFoo foo) { }
    }
}

namespace N2
{
    public static class FooExtensions
    {
        public static void Bar2(this N1.IFoo foo) { }
    }
}

namespace N3
{
    using N1; // not needed under this proposal
    using N2;

    class A
    {
        static void Main()
        {
            N1.IFoo myFoo = null;
            myFoo.Bar2(); // error
        }
    }
}

Now, there is no way to use N2.FooExtensions.Bar2.

@ghord
Copy link
Author

ghord commented Dec 15, 2015

This could cause some breaking changes in the rare cases of multiple extension methods of the same name, in which case I would recommend to consider old extensions methods introduced with using before those new "unreferenced" extension methods.

@alrz Under new proposal you could drop using N1 and it would compile successfully or call N2.FooExtensions.Bar2(myFoo) directly.

Without it it would still be a compilation error now (right?)

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

call N2.FooExtensions.Bar2(myFoo) directly.

Then what is the point of namespaces and extension methods? I think an implicit using restricts possibilities instead of making new onces. Furthermore, in that case, this will be a breaking change, because in a similar scenario myFoo.Bar2() becomes a compiler error, right?

@ghord
Copy link
Author

ghord commented Dec 15, 2015

@alrz Correct me if I'm wrong, but myFoo.Bar2() would cause compiler error even without the proposal, so I don't see a breaking change.

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

@ghord, without using N1; it wouldn't. But when it becomes implicit as you suggested, it would. tada.

@ghord
Copy link
Author

ghord commented Dec 15, 2015

Again,

consider old extensions methods introduced with using before those new "unreferenced" extension methods.

Without using N1, N2.FooExtensions.Bar2 would have priority over implicit method.

@alrz
Copy link
Contributor

alrz commented Dec 15, 2015

@ghord Alright, that'll do it. 👍

@paulomorgado
Copy link

@ghord, the fact that, under this proposal, you wouldn't need a using directive to bring the extension methods into scope, doesn't mean that you can remove it. Other types used might need that same using directive.

@JohnnyBravo75
Copy link

For me it is more a matter of tooling.
e.g. Resharper already detecs the namespace, when you type a unknown function.

@gafter
Copy link
Member

gafter commented Mar 24, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.

I am not moving this particular issue because I don't have confidence that the LDM would likely consider doing this.

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

5 participants