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

ActivatorUtilities.CreateInstance() should respect [ActivatorUtilitiesConstructor] #99175

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

steveharter
Copy link
Member

@steveharter steveharter commented Mar 1, 2024

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:

  1. Reads constructors in order declared (according to the compiler and the order metadata was written).
  2. Picks the constructor with the most parameters.
  3. If there is a constructor with [ActivatorUtilitiesConstructor] then that constructor is selected, no matter how many parameters are present. Only one constructor can have the attribute.
  4. If there is another constructor with the same or more parameters after [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

@ghost
Copy link

ghost commented Mar 1, 2024

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection
See info in area-owners.md if you want to be subscribed.

Issue Details

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.

cc @tarekgh, @eerhardt

Author: steveharter
Assignees: steveharter
Labels:

area-Extensions-DependencyInjection

Milestone: -

Copy link
Member

@tarekgh tarekgh left a 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.

@steveharter steveharter merged commit bb919c2 into dotnet:main Mar 6, 2024
111 checks passed
@steveharter steveharter deleted the DiCtor branch March 6, 2024 14:58
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-DependencyInjection breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants