-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
ActivatorUtilities.CreateInstance() should respect [ActivatorUtilitiesConstructor] #99175
Conversation
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection Issue DetailsAddress a case from #98959 where ordering of the constructors can cause a constructor that has
|
...libraries/Microsoft.Extensions.DependencyInjection/tests/DI.Tests/ActivatorUtilitiesTests.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I hope the breaking change will not cause any issue. At least now we are behaving as our docs describe.
The current approach of selecting constructors uses
[ActivatorUtilitiesConstructor]
in an odd manner that even when a constructor has the attribute, it may not be called depending on the ordering of constructors and the number of constructor parameters. This PR changes to always call the constructor that has that attribute.Current approach:
[ActivatorUtilitiesConstructor]
then that constructor is selected, no matter how many parameters are present. Only one constructor can have the attribute.[ActivatorUtilitiesConstructor]
was found, then that is selected.Proposed approach: from the current approach above, remove (4) which makes (1) no longer applicable.
This address a case from #98959 where ordering of the constructors can cause a constructor that has
[ActivatorUtilitiesConstructor]
to not be selected if comes before a different constructor with the same parameter count, and also fully addresses the issues raised in #42339 (comment).cc @tarekgh, @eerhardt