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]: Always available extension methods #4029

Open
1 of 4 tasks
YairHalberstadt opened this issue Oct 18, 2020 · 21 comments
Open
1 of 4 tasks

[Proposal]: Always available extension methods #4029

YairHalberstadt opened this issue Oct 18, 2020 · 21 comments

Comments

@YairHalberstadt
Copy link
Contributor

YairHalberstadt commented Oct 18, 2020

Always available extension methods

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

Summary

Currently it is impossible to make sure that an extension method is always in scope, except by putting it in the global namespace. We add a step to lookup where we look for extension methods in the same namespace as the receiver type.

Motivation

There are a number of cases where in order for extension methods to be useful we want them to be available under all circumstances, without having to import anything. In such cases the only option is to put them in the global namespace which clogs up the global namespace.

Here are 3 examples where I've wanted this:

Patterns

Many C# features are available if a type structurally matches a certain shape, and that shape can be provided by extension methods. For example GetAwaiter, Deconstruct, and GetEnumerator can all be extension methods, and allow you to await, deconstruct, and foreach a type respectively.

When providing such extension methods, you want usage of the feature to feel native. You don't want to have to import a namespace to have this work. For this reason I put my extension GetEnumerator on System.Range in the global namespace, allowing you to foreach (var i in 1..10) without having to import anything.

Also for these patterns code fixers to import the required extension method have to be implemented manually each time a new such pattern is added, and it can lag behind, meaning discoverability of these extension methods can be poor.

The extension method is a core part of the API.

In StrongInject the IContainer interface is defined as follows:

namespace StrongInject
{
    public interface IContainer<T> : IDisposable
    {
        [EditorBrowsable(EditorBrowsableState.Never)]
        TResult Run<TResult, TParam>(Func<T, TParam, TResult> func, TParam param);
    }
}

The single method that needs to be implemented is quite complex in order to maximize performance where necessary. Also since a container often implements IContainer<T> for multiple types, the implementations are often implicit, and hence awkward to call.

The solution to this has been to provide a number of extension methods. This makes it easy to call implicit implementations, and also provides more convenient but less performant API's wrapping the core API:

namespace StrongInject
{
    public static class ContainerExtensions
    {
        public static TResult Run<T, TResult, TParam>(this IContainer<T> container, Func<T, TParam, TResult> func, TParam param)
        {
            return container.Run(func, param);
        }

        public static TResult Run<T, TResult>(this IContainer<T> container, Func<T, TResult> func)
        {
            return container.Run(static (t, func) => func(t), func);
        }

        public static void Run<T>(this IContainer<T> container, Action<T> action)
        {
            container.Run(static (t, action) =>
            {
                action(t);
                return default(object);
            }, action);
        }
    }
}

These extension methods are meant to be the publicly visible API of the container. If you have access to the container, I want these extension methods to be clearly visible, because they are how you are meant to interact with it, not the method actually defined on IContainer. This unfortunately doesn't work if you haven't imported StrongInject, and you will find the API very difficult to use. Since a Run method does exist on IContainer tooling wont even help you import the extension methods.

Analyzers

There are a number of analyzers which warn if a type has a method, and it isn't called - for example https://www.nuget.org/packages/ConfigureAwaitChecker.Analyzer/ which makes sure you call ConfigureAwait(false) on any type which defines it.

Most of these check for the type using normal lookup rules. This means that if you define an extension ConfigureAwait, and it isn't imported you wont get a warning - potentially causing deadlocks and all other sorts of nastiness in your code. The safest solution is to put the extension method in the global namespace.

Detailed design

If we fail to find any extension methods under current rules, we look for any extension methods in the same namespace as the namespace of the receiver type.

This will allow us to put any extension methods we want to always be available in the same namespace as the type they extend, meaning they will almost always be available.

We update the spec as follows:

  • Starting with the closest enclosing namespace declaration, continuing with each enclosing namespace declaration, and ending with the containing compilation unit, successive attempts are made to find a candidate set of extension methods:
    • If the given namespace or compilation unit directly contains non-generic type declarations Ci with eligible extension methods Mj, then the set of those extension methods is the candidate set.
  • If types Ci imported by using_static_declarations and directly declared in namespaces imported by using_namespace_directives in the given namespace or compilation unit directly contain eligible extension methods Mj, then the set of those extension methods is the candidate set.
  • If the candidate set is empty, if the namespace of the type of the receiver contains non-generic type declarations Ci with eligible extension methods Mj, then the set of those extension methods is the candidate set.
  • If no candidate set is found a compile-time error occurs.
  • Otherwise, overload resolution is applied to the candidate set as described in (Overload resolution). If no single best method is found, a compile-time error occurs.
  • C is the type within which the best method is declared as an extension method.

Drawbacks

  1. This will work well for my Range Foreach use case and my ConfigureAwait use case but less well for the StrongInject use case. For Stronginject, if I have an instance of type IContainer<T> this rule will bring the extension methods into scope (which is useful since they are more convenient), but if I have an instance of type SomeNamespace.SomeContainer : IContainer<T> it will not.

  2. This will encourage people to put extension methods in the same namespace as the type they extend. Since it's very common to import the namespace of a type when you use a type, this will make it very awkward to call other, conflicting, extension methods defined in other namespaces.

In practice I think there's 3 sorts of places extension methods can be defined:

a. As part of the core API
b. In a nuget package focused just on providing these extension methods.
c. In a package imported for other purposes which happens to define some extension methods.

Scenario a isn't much of a problem, because it should be possible for an API to define it's own extension methods, and other people shouldn't be hiding them.

Scenario b also isn't much of a problem since if you don't want to use those provided extension methods, you shouldn't import the package.

c. is more problematic, but it is already bad practice to define public extension methods in a package designed for other purposes. It is doubly so to do so in the same namespace as the receiver. We need to make sure to put this message across strongly: do not put extension methods in the same namespace as the receiver just because. Do it if there's a very strong reason why these extension methods should always be available.

Alternatives

You can put the extension method in the global namespace. However this quickly clutters up the glabal namespace - one my extension methods is defined in a class called ContainerExtensions another RangeExtensions - hardly the most generic of names. How long till two people put a class just called Extensions in the global namespace, and then it is impossible to call either?

We could alternatively add some marker such as [AlwaysAvailableExtension] to mark extension methods that should be available everywhere. This would help with the StrongInject case, but may negatively impact compiler performance.

Unresolved questions

Design meetings

https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-11-11.md#always-available-extension-methods

@Unknown6656
Copy link
Contributor

Could this proposal made somehow compatible with F#'s [<AutoOpen>]-attribute?

@333fred
Copy link
Member

333fred commented Oct 18, 2020

I'm interested enough in this to see what the rest of the LDM thinks. There are certainly times that this could be useful in Roslyn.

@alrz
Copy link
Contributor

alrz commented Oct 19, 2020

For me foreach (var i in 1..10) example is compelling enough. It's awkward to add usings to make that work.

However, since everyone will be encouraged to put extension methods in the same namespace as the type, vs. MyNamespace.Extensions , the problem of conflicting names will get actually worse.

I've seen libraries define extension methods on System types (for internal usages) and make them public! Today this is not much problem because those are in a different namespace, but if they put them in the same namespace as the type, all of those extensions will be suddenly in scope and there is no easy way to favor one over the another.

@YairHalberstadt
Copy link
Contributor Author

@alrz An alternative would be to mark an extension method as always in scope explicitly using an attribute or similar. If the compiler is able to do this efficiently that would be ideal - the risk is that the lookup set becomes huge for every single extension method.

In practice I don't know how much of a problem that would be. Finding the extension methods could be quick since the compiler can cache the location of all such extension methods. Processing them would be slower, but since most will have the wrong name, the actual candidate set that needs to be looked at could be of a reasonable size.

@alrz
Copy link
Contributor

alrz commented Oct 19, 2020

If you're going to opt-in, global imports would do the same.

There's a limitation, however, currently you can't add a specific static class for its extensions e.g. using static SomeExtensions; won't work today. Assuming we can relax that, you don't need to use attributes or any other mechanism for this.

@YairHalberstadt
Copy link
Contributor Author

What do you mean by global imports?

@YairHalberstadt
Copy link
Contributor Author

I mean opt in by the author of the extension method, not the consumer

@alrz
Copy link
Contributor

alrz commented Oct 19, 2020

What do you mean by global imports?

#3428 (either a cli option as proposed, or something like #3428 (comment))

I mean opt in by the author of the extension method, not the consumer

That just restrict what's possible. Personally I'd prefer if we just relax using static to work with extensions and with global imports you can decide if you want to import any extension by default in your code, not to wait for the author to make that change for everyone who may or may not want to import those.

Not sure if we want attributes to affect binding anyways.

@alrz
Copy link
Contributor

alrz commented Oct 19, 2020

Could this proposal made somehow compatible with F#'s [<AutoOpen>]-attribute?

@Unknown6656 AutoOpen is different and still requires you to import the enclosing namespace. It's more similar to VB modules. Extensions on the other hand, are already "AutoOpen" in that sense, because once you import the namespace, you can use any extension defined there.

@YairHalberstadt
Copy link
Contributor Author

In the three use cases I suggested here, it would be better for the library author to control this than the consumer. I want people to just install my extension GetEnumerator and be done, not have to edit the csproj to add a global import.

@yaakov-h
Copy link
Member

You could include the global import from a custom .props file in your package, but I would prefer that this should also "just work" with assembly references.

@ghord
Copy link

ghord commented Oct 23, 2020

Also discussed here: dotnet/roslyn#7489

@333fred 333fred modified the milestones: Working Set, Backlog Nov 11, 2020
@Pyrdacor
Copy link

How about an attribute on the extension class like [VisibleWith(typeof(MyClass))]. So if MyClass is visible through usings the extensions are also available. This attribute could be used multiple times on the extension class to make it available with multiple types.

Or maybe something like [VisibleWithAny(typeof(Class1), typeof(Interface1), typeof(Class2))].

@egil
Copy link

egil commented Jan 5, 2021

I can relate to the problem described here.

That said, should we be looking more towards default implemented interfaces to solve this problem perhaps? E.g.:

public interface IContainer<T> : IDisposable
{
    public TResult Run<TResult, TParam>(Func<T, TParam, TResult> func, TParam param);

    public TResult Run<TResult>(Func<T, TResult> func)
        => Run((t, func) => func(t), func);    

    public void Run(Action<T> action)
        => Run((t, action) =>
        {
            action(t);
            return default(object);
        }, action);    
}

@YairHalberstadt
Copy link
Contributor Author

@egil Default interface methods won't work here since they require casting the container to the interface. Extension methods solve that issue.

@egil
Copy link

egil commented Jan 5, 2021

@egil Default interface methods won't work here since they require casting the container to the interface. Extension methods solve that issue.

Indeed, if the users have a reference to a concrete type and not an interface. It's one of my complaints about default implemented interfaces, one that would be great if was addressed.

@siegfriedpammer
Copy link

Isn't this already solved now that we have global usings and implicit usings via the csproj (and nuget packages)?

@CyrusNajmabadi
Copy link
Member

@siegfriedpammer no. The point of this proposal is that the consumer doesn't need to do anything extra, not that there is no using in the file. The idea is that once you have an instance of a type (no matter how you got it) that you can call extensions on it from the same namespace. Thanks!

@TahirAhmadov
Copy link

As an improvement for the global namespace workaround, I name my extension classes differently than other classes. For IEnumerable, the extensions class is _IEnumerable_Extensions. This gives enough visual feedback to let me know it's not a standard class and all of them are grouped together with the underscore.
Having said that, I think this improvement is worthwhile, but not sufficiently so to justify implementing it for existing this extension methods; instead, it should be implemented together with the "roles and extensions" proposal - "not to get up twice".

@wrexbe
Copy link

wrexbe commented Apr 26, 2024

// StringHelpers.cs

// Links MyLibary.Extensions to the System namespace
namespace MyLibary.Extensions : System;

public static class StringHelpers
{
    public static string TruncateRight(this string item, int maxLength) =>
        item.Length > maxLength ? item[..maxLength] : item;
}

// SomeOtherFile.cs

// If you need to opt out
!using MyLibrary.Extensions;

@BlinD-HuNTeR
Copy link

This proposal is from 2020 and there is no point in ressurecting it now, since we can already use global using to achieve that. Plus, global usings can be added not only from source but also from the MSBuild <Using> itemgroup, which means that one can even create a NuGet package that provides both the extension methods and a .targets file to create the appropriate global using, therefore making those methods always available without any user action other than just referencing the package.

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