YairHalberstadt / stronginject

compile time dependency injection for .NET
MIT License
845 stars 24 forks source link

Support decorator factories that aren't gratuitously generic #191

Closed jnm2 closed 2 years ago

jnm2 commented 2 years ago

Edit: it's not gratuitous once you take into account that the generated code behind the scenes would otherwise have to insert a cast which could fail at runtime. Generics makes it fail at compile time which is better.

This works, but there's no real reason for the method to be generic:

using System;
using StrongInject;

using var container = new Container();
container.Run(a => { }); // Prints 3 times: Decorated!

[Register(typeof(A)), Register(typeof(B)), Register(typeof(C)), Register(typeof(D))]
partial class Container : IContainer<A>
{
    [DecoratorFactory]
    private static T Test<T>(T component) where T : Component
    {
        Console.WriteLine("Decorated!");

        component.Disposed += (_, _) => { /*...*/ };
        return component;
    }
}

record A(B B, C C, D D);
record B : Component;
record C : Component;
record D : Component;
record Component { public event EventHandler Disposed; }

This does not work, and StrongInject also provides no errors or warnings:

using System;
using StrongInject;

using var container = new Container();
container.Run(a => { }); // Prints nothing

[Register(typeof(A)), Register(typeof(B)), Register(typeof(C)), Register(typeof(D))]
partial class Container : IContainer<A>
{
    [DecoratorFactory]
    private static Component Test(Component component)
    {
        Console.WriteLine("Decorated!");

        component.Disposed += (_, _) => { /*...*/ };
        return component;
    }
}

record A(B B, C C, D D);
record B : Component;
record C : Component;
record D : Component;
record Component { public event EventHandler Disposed; }
YairHalberstadt commented 2 years ago

How is that meant to work? There's no guarantee that the decorator returns an instance of the same type it passed in...

jnm2 commented 2 years ago

Oh, I see, a cast would have to be emitted after the call to this decorator and then we have a runtime failure if you return the wrong type instead of a compile-time one. That's a very good point.

Would it be possible in theory to provide an error or warning when you don't think of using generics and it is being silently ignored? I didn't think of generics right away.

YairHalberstadt commented 2 years ago

@jnm2 I'm not sure how I could detect that...

jnm2 commented 2 years ago

I'm noticing IDE notifications that the method is unused, so I think this is actually good enough.