Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix protected ctor resolution regression #138

Merged

Conversation

gkarabin
Copy link
Contributor

@gkarabin gkarabin commented Jun 16, 2020

I have been working with an older commit of TinyIoC in one of my projects. Updating to the latest master commit I ran into a regression with CanResolve on a class with a protected constructor.

I rely on CanResolve returning false on a base class with only protected constructors, so that I know that the consumer of my library has not registered an implementation of that base class, letting me provide my default implementation.

That behavior changed here: 3891ab9 - the original use of Type.GetConstructors did not return protected constructors, but the new implementation did.

This fix filters out protected constructors from the set that allows CanResolve to return true.

gkarabin added 2 commits June 15, 2020 22:37
Add tests to exercise construction and checking resolution of classes that have
only protected constructors. A new test that asserts that CanResolve of a class with only a protected constructor will return false will fail.  The regression
was introduced in
grumpydev@3891ab9.
The original use of type.GetConstructors() only returend public
constructors - no protected ones.
Resolution of a protected constructor must throw and checking for resolution must also fail.  On the other hand, resolving a concrete subclass of such a class should succeed if the subclass has a public constructor that has its dependencies met.
@niemyjski niemyjski merged commit 6ef738a into grumpydev:master Jun 16, 2020
@niemyjski
Copy link
Collaborator

Thanks for the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants