[Idea] Generate ServiceProvider factory methods for convenience

[slang25]

Great job on stronginject @YairHalberstadt, I'm really impressed by the execution of this really clever idea.

A lot of frameworks and libraries today use Microsoft.Extensions.DependencyInjection, and offer extensions methods that hang off of ServiceCollection. I see the ASP.NET Core example here showing how to integrate the two worlds:

[Register(typeof(WeatherForecastController), Scope.InstancePerResolution)]
[Register(typeof(WeatherForecastProvider), Scope.InstancePerDependency, typeof(IWeatherForecastProvider))]
[Register(typeof(WeatherSummarizer), Scope.SingleInstance, typeof(IWeatherSummarizer))]
[Register(typeof(UsersController), Scope.InstancePerResolution)]
[Register(typeof(DatabaseUsersCache), Scope.SingleInstance, typeof(IUsersCache))]
[Register(typeof(MockDatabase), Scope.SingleInstance, typeof(IDatabase))]
[RegisterDecorator(typeof(DatabaseDecorator), typeof(IDatabase))]
public partial class Container : IContainer<WeatherForecastController>, IContainer<UsersController>
{
    private readonly IServiceProvider _serviceProvider;

    public Container(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    [Factory] private ILogger<T> GetLogger<T>() => _serviceProvider.GetRequiredService<ILogger<T>>();
}

I think it would be really nice if there was a simple attribute that could be used (such as ForwardToServiceProvider, or something to that effect), so that the generic code is auto-generated, and the factory is added automatically. So the sample would become something like:

[Register(typeof(WeatherForecastController), Scope.InstancePerResolution)]
[Register(typeof(WeatherForecastProvider), Scope.InstancePerDependency, typeof(IWeatherForecastProvider))]
[Register(typeof(WeatherSummarizer), Scope.SingleInstance, typeof(IWeatherSummarizer))]
[Register(typeof(UsersController), Scope.InstancePerResolution)]
[Register(typeof(DatabaseUsersCache), Scope.SingleInstance, typeof(IUsersCache))]
[Register(typeof(MockDatabase), Scope.SingleInstance, typeof(IDatabase))]
[RegisterDecorator(typeof(DatabaseDecorator), typeof(IDatabase))]
[ForwardToServiceProvider(typeof(ILogger<>))]
public partial class Container : IContainer<WeatherForecastController>, IContainer<UsersController>
{
}

[YairHalberstadt]

That's an interesting idea - I will definitely have to think about it!

One issue is that SourceGenerators can't call into other SourceGenerators, so this will have to be a core part of StrongInject, rather than an Asp.Net specific plugin. I'm not sure how comfortable I am with "polluting" core StrongInject with Asp.Net specific logic.

I think there's definitely room to explore using the full power of SourceGenerators to improve the Asp.Net integration, but I want to consider it a bit more to make sure I get it right. Maybe it might make for a 2.0 release?

[slang25]

Thanks for the quick response, yeah those are fair concerns.

To be a bit clearer, I'm not looking to erase the service provider code, but eliminate the factory methods, so you can declare some of the deps at the top with normal registered types. I see the problem being you might have 10-20 dependencies still coupled to ServiceCollection after initial adoption.

So something like this would also keep me happy:

[Register(typeof(WeatherForecastController), Scope.InstancePerResolution)]
[Register(typeof(WeatherForecastProvider), Scope.InstancePerDependency, typeof(IWeatherForecastProvider))]
[Register(typeof(WeatherSummarizer), Scope.SingleInstance, typeof(IWeatherSummarizer))]
[Register(typeof(UsersController), Scope.InstancePerResolution)]
[Register(typeof(DatabaseUsersCache), Scope.SingleInstance, typeof(IUsersCache))]
[Register(typeof(MockDatabase), Scope.SingleInstance, typeof(IDatabase))]
[RegisterDecorator(typeof(DatabaseDecorator), typeof(IDatabase))]
[RegisterWithNamedFactory(typeof(ILogger<>), "serviceprovider")]
public partial class Container : IContainer<WeatherForecastController>, IContainer<UsersController>
{
    private readonly IServiceProvider _serviceProvider;

    public Container(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    [Factory(Name = "serviceprovider")] private T GetService<T>() => _serviceProvider.GetRequiredService<T>();
}

Again, the names and types aren't important, just trying to convey the concept.

[YairHalberstadt]

That's definitely a more compelling design. Some way of naming factories and using those specifically for resolution.

[YairHalberstadt]

On issue is typeof(ILogger<>) can't actually be used in attributes. If https://github.com/dotnet/roslyn/pull/26337 ever lands, we should be able to do something here.

[dadhi]

On issue is typeof(ILogger<>) can't actually be used in attributes

You may consider to use ILogger<object> for this case - just like saying that the logger is for any object.

[YairHalberstadt]

One issue is typeof(ILogger<>) can't actually be used in attributes. If dotnet/roslyn#26337 ever lands, we should be able to do something here.

Turns out you can. I was getting confused with that you cant do this:

public class C<T> {
    [MyAttribute(typeof(T))]
    class D {}
}

So I think I decent design here is as follows:

Any generic method can be marked with any number of [FactoryOf(Type type, Scope scope = Scope.InstancePerResolution)] attributes instead of the [Factory] attribute.

If so it acts exactly like any other generic Factory, except it's only used to resolve types which are in FactoryOf. If the type is open generic, then we can resolve any type which is created by parameterizing the types in FactoryOf.

So your example would be:

[Register(typeof(WeatherForecastController), Scope.InstancePerResolution)]
[Register(typeof(WeatherForecastProvider), Scope.InstancePerDependency, typeof(IWeatherForecastProvider))]
[Register(typeof(WeatherSummarizer), Scope.SingleInstance, typeof(IWeatherSummarizer))]
[Register(typeof(UsersController), Scope.InstancePerResolution)]
[Register(typeof(DatabaseUsersCache), Scope.SingleInstance, typeof(IUsersCache))]
[Register(typeof(MockDatabase), Scope.SingleInstance, typeof(IDatabase))]
[RegisterDecorator(typeof(DatabaseDecorator), typeof(IDatabase))]
public partial class Container : IContainer<WeatherForecastController>, IContainer<UsersController>
{
    private readonly IServiceProvider _serviceProvider;

    public Container(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    [FactoryOf(typeof(ILogger<T>)] private T GetService<T>() => _serviceProvider.GetRequiredService<T>();
}

What do you think?

[slang25]

Yeah, that works and is really clean! I think in this example the ILogger<T> would just be ILogger<>, I've started a trend of copy-pasting redundant Ts apparently 😄.

I think on the generic attribute language feature thing, I'm hoping that lands in C# 10, then you can add versions of Register and family that take type args in stead of the good old Type parameters as is done today (of course keeping them for compat and choice), it will make everything look neater. Thankfully it shouldn't require any forward thinking from an API design perspective, as the type argument would distinguish them (if that makes sense), so we don't need to think about overload ambiguity etc...

[YairHalberstadt]

I've now implemented this and merged this in. You should be able to try it out in the latest preview nuget package. In a few days I'll hopefully cut a release.