[DependencyInjection] Fix some edge cases with autowiring #16464
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -147,6 +148,11 @@ private function populateAvailableType($id, Definition $definition) | |||
| $this->types[$type] = $id; | |||
| } | |||
|
|
|||
| // Cannot use reflection if the class isn't set | |||
There was a problem hiding this comment.
This can be moved above the foreach-loop as it doesn't depend on the loop.
And improves performance a little.
There was a problem hiding this comment.
It depends of the class of the definition. It cannot be moved outside of the loop.
There was a problem hiding this comment.
I mean like this:
private function populateAvailableType($id, Definition $definition)
{
// Never use abstract services
if ($definition->isAbstract()) {
return;
}
// Cannot use reflection if the class isn't set
if (!$definition->getClass()) {
return;
}
foreach ($definition->getAutowiringTypes() as $type) {
$this->definedTypes[$type] = true;
$this->types[$type] = $id;
}
if ($reflectionClass = $this->getReflectionClass($id, $definition)) {
$this->extractInterfaces($id, $reflectionClass);
$this->extractAncestors($id, $reflectionClass);
}
}Check if a class is set, if not return before executing the loop.
There was a problem hiding this comment.
I've done that on purpose. Associating a service with a type using the autowiring_types key doesn't require reflection, so it doesn't require the class key to exist.
It allows to do things like associating types with a service definition where the class is associated in a compiler pass executed after the autowiring.
ac456ee to
52f5936
Compare
|
Rebased. |
52f5936 to
faefc60
Compare
|
Thank you @dunglas. |
fabpot
added a commit
that referenced
this pull request
Nov 7, 2015
…ing (dunglas) This PR was merged into the 2.8 branch. Discussion ---------- [DependencyInjection] Fix some edge cases with autowiring | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Enhance the autowiring system: - Works with parent definitions and decorator - Always exclude parent definitions It allows to autowire major services of the standard edition (tested with Swift Mailer, Monolog, Doctrine and Twig). Commits ------- faefc60 [DependencyInjection] Autowing: exclu 7A5C de abstract definitons 71d502a [DependencyInjection] Autowiring: support parent/decorators
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Enhance the autowiring system:
It allows to autowire major services of the standard edition (tested with Swift Mailer, Monolog, Doctrine and Twig).