khellang / Scrutor

Assembly scanning and decoration extensions for Microsoft.Extensions.DependencyInjection
MIT License
3.64k stars 239 forks source link

Breaking change - Open generic types #44

Closed kakone closed 6 years ago

kakone commented 6 years ago

Hello,

I have a problem with the changes due to the commit f2bd7d7. In my assembly, I have a class like this :

public class OpenGeneric<T> : IOpenGeneric<T> { }
public interface IOpenGeneric<T> : IOtherInheritance { }
public interface IOtherInheritance { }

When I do .AsImplementedInterfaces(), I now obtain an error (this class was excluded before the changes) : 'Cannot instantiate implementation type 'Scrutor.Tests.OpenGeneric1[T]' for service type 'Scrutor.Tests.IOtherInheritance'.'

So, perhaps it would be nice to have an option to disable autoregistration for open generic types. Otherwise (better option), the IOtherInheritance interface should be excluded from the registration.

khellang commented 6 years ago

Ugh. Yeah, I was afraid of this happening, as mentioned in https://github.com/khellang/Scrutor/issues/42#issuecomment-369618193 😢

Can you tell me a bit more about the scenario you're describing? Those are types from the Scrutor test assembly. Is that really the types you're having trouble with?

Why is it problematic to register the generic class? Is it not meant to be created? Perhaps you can make it an abstract class?

In general, it isn't problematic to register stuff that's never resolved. Why are you resolving the type if you didn't expect it to be registered? If you didn't expect it to be registered, why is it implementing the interface?

I need to think long and hard about what to do here. I don't really want to introduce a myriad of flags to configure the scanning (there's already public/private, and in-/excluding nested types has come up). I'd rather have people tweak their scanning filters. Not including open generic types was an oversight and should've been included from the start.

Maybe I'll let this issue sit here for a bit to see if others are experiencing issues as well. You should still be able to run on v2.1 fine 😊

khellang commented 6 years ago

Hmm. After looking at the types again, I think that's a bug :bug: There should be no way to register an open generic type to a non-generic interface as there would be no way to close the type by resolving through the interface.

khellang commented 6 years ago

Ok, I've pushed v2.2.2 to NuGet that will hopefully fix this, but it might take a couple of minutes for it to be indexed.

Wanna try it out and let me know if that fixes it for you?

kakone commented 6 years ago

Yes, perfect, it fixes my problem.

Thank you very much for the responsiveness (and for sharing this useful project).

khellang commented 6 years ago

Thank you for confirming the fix 😁