-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[DI] Optional class for named services #21133
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,21 @@ | |
|
|
||
| /** | ||
| * @author Guilhem N. <[email protected]> | ||
| * | ||
| * @deprecated since version 3.3, to be removed in 4.0. | ||
| */ | ||
| class FactoryReturnTypePass implements CompilerPassInterface | ||
| { | ||
| private $resolveClassPass; | ||
|
|
||
| public function __construct(ResolveClassPass $resolveClassPass = null) | ||
| { | ||
| if (null === $resolveClassPass) { | ||
| @trigger_error('The '.__CLASS__.' class is deprecated since version 3.3 and will be removed in 4.0.', E_USER_DEPRECATED); | ||
| } | ||
| $this->resolveClassPass = $resolveClassPass; | ||
| } | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
|
|
@@ -29,21 +41,22 @@ public function process(ContainerBuilder $container) | |
| if (!method_exists(\ReflectionMethod::class, 'getReturnType')) { | ||
| return; | ||
| } | ||
| $resolveClassPassChanges = null !== $this->resolveClassPass ? $this->resolveClassPass->getChanges() : array(); | ||
|
|
||
| foreach ($container->getDefinitions() as $id => $definition) { | ||
| $this->updateDefinition($container, $id, $definition); | ||
| $this->updateDefinition($container, $id, $definition, $resolveClassPassChanges); | ||
| } | ||
| } | ||
|
|
||
| private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $previous = array()) | ||
| private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $resolveClassPassChanges, array $previous = array()) | ||
| { | ||
| // circular reference | ||
| if (isset($previous[$id])) { | ||
| return; | ||
| } | ||
|
|
||
| $factory = $definition->getFactory(); | ||
| if (null === $factory || null !== $definition->getClass()) { | ||
| if (null === $factory || (!isset($resolveClassPassChanges[$id]) && null !== $definition->getClass())) { | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -58,7 +71,7 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $ | |
| if ($factory[0] instanceof Reference) { | ||
| $previous[$id] = true; | ||
| $factoryDefinition = $container->findDefinition((string) $factory[0]); | ||
| $this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $previous); | ||
| $this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $resolveClassPassChanges, $previous); | ||
| $class = $factoryDefinition->getClass(); | ||
| } else { | ||
| $class = $factory[0]; | ||
|
|
@@ -83,6 +96,9 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $ | |
| } | ||
| } | ||
|
|
||
| if (null !== $returnType && (!isset($resolveClassPassChanges[$id]) || $returnType !== $resolveClassPassChanges[$id])) { | ||
| @trigger_error(sprintf('Relying on its factory\'s return-type to define the class of service "%s" is deprecated since Symfony 3.3 and won\'t work in 4.0. Set the "class" attribute to "%s" on the service definition instead.', $id, $returnType), E_USER_DEPRECATED); | ||
| } | ||
| $definition->setClass($returnType); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Component\DependencyInjection\Compiler; | ||
|
|
||
| use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
| use Symfony\Component\DependencyInjection\ChildDefinition; | ||
|
|
||
| /** | ||
| * @author Nicolas Grekas <[email protected]> | ||
| */ | ||
| class ResolveClassPass implements CompilerPassInterface | ||
| { | ||
| private $changes = array(); | ||
|
|
||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function process(ContainerBuilder $container) | ||
| { | ||
| foreach ($container->getDefinitions() as $id => $definition) { | ||
| if ($definition instanceof ChildDefinition || $definition->isSynthetic() || null !== $definition->getClass()) { | ||
| continue; | ||
| } | ||
| if (preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $id)) { | ||
| $this->changes[$id] = $container->getCaseSensitiveId($id); | ||
| $definition->setClass($this->changes[$id]); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @internal | ||
| * | ||
| * @deprecated since 3.3, to be removed in 4.0. | ||
| */ | ||
| public function getChanges() | ||
| { | ||
| $changes = $this->changes; | ||
| $this->changes = array(); | ||
|
|
||
| return $changes; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -303,7 +303,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file) | |
| if (false !== $nodes = $xpath->query('//container:argument[@type="service"][not(@id)]|//container:property[@type="service"][not(@id)]')) { | ||
| foreach ($nodes as $node) { | ||
| // give it a unique name | ||
| $id = sprintf('%s_%d', hash('sha256', $file), ++$count); | ||
| $id = sprintf('%d_%s', ++$count, hash('sha256', $file)); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just made these changes so that anonymous classes won't have their class set to a random string (+ added corresponding regexp check in ResolveClassPass)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why hashing it? Wouldn't it be more explicit with the plain file name?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not related to this PR :) |
||
| $node->setAttribute('id', $id); | ||
|
|
||
| if ($services = $this->getChildren($node, 'service')) { | ||
|
|
@@ -321,15 +321,15 @@ private function processAnonymousServices(\DOMDocument $xml, $file) | |
| if (false !== $nodes = $xpath->query('//container:services/container:service[not(@id)]')) { | ||
| foreach ($nodes as $node) { | ||
| // give it a unique name | ||
| $id = sprintf('%s_%d', hash('sha256', $file), ++$count); | ||
| $id = sprintf('%d_%s', ++$count, hash('sha256', $file)); | ||
| $node->setAttribute('id', $id); | ||
| $definitions[$id] = array($node, $file, true); | ||
| } | ||
| } | ||
|
|
||
| // resolve definitions | ||
| krsort($definitions); | ||
| foreach ($definitions as $id => list($domElement, $file, $wild)) { | ||
| uksort($definitions, 'strnatcmp'); | ||
| foreach (array_reverse($definitions) as $id => list($domElement, $file, $wild)) { | ||
| if (null !== $definition = $this->parseDefinition($domElement, $file)) { | ||
| $this->container->setDefinition($id, $definition); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,8 @@ | |
|
|
||
| /** | ||
| * @author Guilhem N. <[email protected]> | ||
| * | ||
| * @group legacy | ||
| */ | ||
| class FactoryReturnTypePassTest extends \PHPUnit_Framework_TestCase | ||
| { | ||
|
|
@@ -103,17 +105,16 @@ public function testCircularReference() | |
| $this->assertNull($factory2->getClass()); | ||
| } | ||
|
|
||
| /** | ||
| * @requires function ReflectionMethod::getReturnType | ||
| * @expectedDeprecation Relying on its factory's return-type to define the class of service "factory" is deprecated since Symfony 3.3 and won't work in 4.0. Set the "class" attribute to "Symfony\Component\DependencyInjection\Tests\Fixtures\FactoryDummy" on the service definition instead. | ||
| */ | ||
| public function testCompile() | ||
| { | ||
| $container = new ContainerBuilder(); | ||
|
|
||
| $factory = $container->register('factory'); | ||
| $factory->setFactory(array(FactoryDummy::class, 'createFactory')); | ||
|
|
||
| if (!method_exists(\ReflectionMethod::class, 'getReturnType')) { | ||
| $this->setExpectedException(\RuntimeException::class, 'Please add the class to service "factory" even if it is constructed by a factory since we might need to add method calls based on compile-time checks.'); | ||
| } | ||
|
|
||
| $container->compile(); | ||
|
|
||
| $this->assertEquals(FactoryDummy::class, $container->getDefinition('factory')->getClass()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of the Symfony package. | ||
| * | ||
| * (c) Fabien Potencier <[email protected]> | ||
| * | ||
| * For the full copyright and license information, please view the LICENSE | ||
| * file that was distributed with this source code. | ||
| */ | ||
|
|
||
| namespace Symfony\Component\DependencyInjection\Tests\Fixtures; | ||
|
|
||
| class CaseSensitiveClass | ||
| { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> | ||
| <services> | ||
| <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass" /> | ||
| </services> | ||
| </container> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| services: | ||
| Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass: | ||
| autowire: true |
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.
Do we actually need the entry for aliases or could we just remove existing entries here?
Uh oh!
There was an error while loading. Please reload this page.
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.
For completeness yes: we track any id, wherever it comes from