dadhi / DryIoc

DryIoc is fast, small, full-featured IoC Container for .NET
MIT License
1.01k stars 123 forks source link

Fix GetWrappedType implementation(s) to not return null #560

Closed Metadorius closed 1 year ago

Metadorius commented 1 year ago

Motivation for this change: I tried to resolve IContainer through third-party Splat.DryIoc dependency resolver in my app (bad practice but WinForms is a bad practice by itself, so eh). It errored with a NullReferenceException here (none of the params were generic): https://github.com/dadhi/DryIoc/blob/e6a4e33e55470fa0d187b4d651bd5bbef4aabc56/src/DryIoc/Container.cs#L5072-L5077

Upon further inspection I found out that the methods shouldn't ever return null, according to documentation: https://github.com/dadhi/DryIoc/blob/e6a4e33e55470fa0d187b4d651bd5bbef4aabc56/src/DryIoc/Container.cs#L14054 And here: https://github.com/dadhi/DryIoc/blob/e6a4e33e55470fa0d187b4d651bd5bbef4aabc56/src/DryIoc/Container.cs#L14059

Yet one of the branches was seemingly erratically returning null, so I fixed that.

dadhi commented 1 year ago

Thanks, I will look a bit later.

Metadorius commented 1 year ago

Any updates on that? It should be a pretty small oversight, as the documentation (and code usage) assumes the methods will never return null, and it clearly looks like an oversight that it returns null in one case when in another it returns the initial type properly. It's breaking in some of the usage scenarios of the container, particularly resolving the container through Splat which calls the method in question.

dadhi commented 1 year ago

I am on vacation till 8th of March, so I will look when I have time.

dadhi commented 1 year ago

Hi, thanks for PR, merging.

dadhi commented 1 year ago

@Metadorius The v5.3.4 containing the PR is out. Thanks!