JasperFx / lamar

Fast Inversion of Control Tool and Successor to StructureMap
https://jasperfx.github.io/lamar
MIT License
563 stars 118 forks source link

ReflectionExtensions.IsEnumerable could not detect some types #253

Closed sm-g closed 2 years ago

sm-g commented 4 years ago

https://github.com/JasperFx/lamar/blob/master/src/LamarCodeGeneration/Util/ReflectionExtensions.cs#L28

i.e. IReadOnlyCollection<> will not be detected

jeremydmiller commented 3 years ago

@sm-g "I take pull requests" on that one.

rizi commented 2 years ago

@sm-g "I take pull requests" on that one.

@jeremydmiller I will try to send you a PR.

Br

rizi commented 2 years ago

@jeremydmiller I'm preparing a PR.

It was easy to extend the ReflectionExtensions class, however I'm not finding the related test classes. It also influences the EnumerablePolicy so that IReadOnlyCollection<> should be resolveable, could you point me to the correct test classes?

This could be the correct one: https://github.com/JasperFx/lamar/blob/master/src/Lamar.Testing/IoC/Acceptance/enumerable_instances.cs, but I'm not 100% sure.

br.

jeremydmiller commented 2 years ago

Most of the testing in Lamar is “sociable” tests. These tests are the important ones I’d need: https://github.com/JasperFx/lamar/blob/master/src/Lamar.Testing/IoC/Acceptance/enumerable_instances.cs https://github.com/JasperFx/lamar/blob/master/src/Lamar.Testing/IoC/Acceptance/enumerable_instances.cs

On Dec 30, 2021, at 11:48 AM, rizi @.***> wrote:

@jeremydmiller https://github.com/jeremydmiller I'm preparing a PR.

It was easy to extend the ReflectionExtensions class, however I'm not finding the related test classes. It also influences the EnumerablePolicy so that IReadOnlyCollection<> should be resolveable, could you point me to the correct test classes?

br.

— Reply to this email directly, view it on GitHub https://github.com/JasperFx/lamar/issues/253#issuecomment-1003125368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABLYCZUAUARPAXR67J4OTLUTSLQDANCNFSM4M2H3BCA. You are receiving this because you were mentioned.

rizi commented 2 years ago

@jeremydmiller it's fixed with #313, so I think we can close it. Br