-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] [CompilerPass] [AutoWire] : fix order service definition problem #22170
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
Conversation
The principle:
Unit test fails. I will check why but normally, this proposing code has no effect on the existing behavior and fix the problem announced in the issue. WDYT ? |
Not having any test covering this issue makes it impossible to accept this PR |
@stof , right. will fix quickly |
this should be done (and discussed) on 2.8 first |
@nicolas-grekas, @stof , rebase to 2.8 done. First Point: service_test:
class: MyClass
autowire: true class MyClass {
public function __construct(Interface RepoInterface $RI, Repo $R)
}
class Repo implements RepoInterface {} It treats Second Point: Tell me WDYT |
I think we should be stricter here and not turn auto-registered services into type-candidates. |
Closing in favor of #22254 |
…andidates (nicolas-grekas) This PR was merged into the 2.8 branch. Discussion ---------- [DI] Don't use auto-registered services to populate type-candidates | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no (every bug fix is a bc break, isn't it?) | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22162, ~~#21658~~ | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Alternative to #22170 and ~~#21665~~. The core issue fixed here is that auto-registered services should *not* be used as type-candidates. The culprit is this line: `$this->populateAvailableType($argumentId, $argumentDefinition);` Doing so creates a series of wtf-issues (the linked ones), with no simple fixes (the linked PRs are just dealing with the simplest cases, but the real fix would require a reboot of autowiring for every newly discovered types.) The other changes accommodate for the removal of the population of the `types` property, and fix a few other issues found along the way: - variadic constructors - empty strings injection - tail args removal when they match the existing defaults Commits ------- 992c677 [DI] Don't use auto-registered services to populate type-candidates
This is a PR proposal to fix service order problem definition during AutoWirePass.