10000 feature #21133 [DI] Optional class for named services (hason, nicolas… · symfony/symfony@306a060 · GitHub
[go: up one dir, main page]

Skip to content

Commit 306a060

Browse files
committed
feature #21133 [DI] Optional class for named services (hason, nicolas-grekas)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Optional class for named services | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Continues #20264: - makes the id-to-class mapping context-free (no `class_exist` check), - deals with ChildDefinition (which should not have this rule applied on them), - deprecates FactoryReturnTypePass as discussed on slack and reported in this comment: #19191 (comment) From @hason: > I prefer class named services (in applications) because naming things are too hard: ``` yaml services: Vendor\Namespace\Class: class: Vendor\Namespace\Class autowire: true ``` > This PR solves redundant parameter for class: ``` yaml services: Vendor\Namespace\Class: autowire: true ``` > Inspirations: https://laravel.com/docs/5.3/container, #18268, http://php-di.org/, Commits ------- a18c4b6 [DI] Add tests for class named services 71b17c7 [DI] Optional class for named services
2 parents 8725f69 + a18c4b6 commit 306a060

12 files changed

+204
-17
lines changed

src/Symfony/Component/DependencyInjection/Compiler/FactoryReturnTypePass.php

Lines changed: 20 additions & 4 6D40 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,21 @@
1717

1818
/**
1919
* @author Guilhem N. <egetick@gmail.com>
20+
*
21+
* @deprecated since version 3.3, to be removed in 4.0.
2022
*/
2123
class FactoryReturnTypePass implements CompilerPassInterface
2224
{
25+
private $resolveClassPass;
26+
27+
public function __construct(ResolveClassPass $resolveClassPass = null)
28+
{
29+
if (null === $resolveClassPass) {
30+
@trigger_error('The '.__CLASS__.' class is deprecated since version 3.3 and will be removed in 4.0.', E_USER_DEPRECATED);
31+
}
32+
$this->resolveClassPass = $resolveClassPass;
33+
}
34+
2335
/**
2436
* {@inheritdoc}
2537
*/
@@ -29,21 +41,22 @@ public function process(ContainerBuilder $container)
2941
if (!method_exists(\ReflectionMethod::class, 'getReturnType')) {
3042
return;
3143
}
44+
$resolveClassPassChanges = null !== $this->resolveClassPass ? $this->resolveClassPass->getChanges() : array();
3245

3346
foreach ($container->getDefinitions() as $id => $definition) {
34-
$this->updateDefinition($container, $id, $definition);
47+
$this->updateDefinition($container, $id, $definition, $resolveClassPassChanges);
3548
}
3649
}
3750

38-
private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $previous = array())
51+
private function updateDefinition(ContainerBuilder $container, $id, Definition $definition, array $resolveClassPassChanges, array $previous = array())
3952
{
4053
// circular reference
4154
if (isset($previous[$id])) {
4255
return;
4356
}
4457

4558
$factory = $definition->getFactory();
46-
if (null === $factory || null !== $definition->getClass()) {
59+
if (null === $factory || (!isset($resolveClassPassChanges[$id]) && null !== $definition->getClass())) {
4760
return;
4861
}
4962

@@ -58,7 +71,7 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $
5871
if ($factory[0] instanceof Reference) {
5972
$previous[$id] = true;
6073
$factoryDefinition = $container->findDefinition((string) $factory[0]);
61-
$this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $previous);
74+
$this->updateDefinition($container, strtolower($factory[0]), $factoryDefinition, $resolveClassPassChanges, $previous);
6275
$class = $factoryDefinition->getClass();
6376
} else {
6477
$class = $factory[0];
@@ -83,6 +96,9 @@ private function updateDefinition(ContainerBuilder $container, $id, Definition $
8396
}
8497
}
8598

99+
if (null !== $returnType && (!isset($resolveClassPassChanges[$id]) || $returnType !== $resolveClassPassChanges[$id])) {
100+
@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);
101+
}
86102
$definition->setClass($returnType);
87103
}
88104
}

src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ public function __construct()
4141

4242
$this->optimizationPasses = array(array(
4343
new ExtensionCompilerPass(),
44+
$resolveClassPass = new ResolveClassPass(),
4445
new ResolveDefinitionTemplatesPass(),
4546
new DecoratorServicePass(),
4647
new ResolveParameterPlaceHoldersPass(),
47-
new FactoryReturnTypePass(),
48+
new FactoryReturnTypePass($resolveClassPass),
4849
new CheckDefinitionValidityPass(),
4950
new ResolveReferencesToAliasesPass(),
5051
new ResolveInvalidReferencesPass(),
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\ContainerBuilder;
15+
use Symfony\Component\DependencyInjection\ChildDefinition;
16+
17+
/**
18+
* @author Nicolas Grekas <p@tchwork.com>
19+
*/
20+
class ResolveClassPass implements CompilerPassInterface
21+
{
22+
private $changes = array();
23+
24+
/**
25+
* {@inheritdoc}
26+
*/
27+
public function process(ContainerBuilder $container)
28+
{
29+
foreach ($container->getDefinitions() as $id => $definition) {
30+
if ($definition instanceof ChildDefinition || $definition->isSynthetic() || null !== $definition->getClass()) {
31+
continue;
32+
}
33+
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)) {
34+
$this->changes[$id] = $container->getCaseSensitiveId($id);
35+
$definition->setClass($this->changes[$id]);
36+
}
37+
}
38+
}
39+
40+
/**
41+
* @internal
42+
*
43+
* @deprecated since 3.3, to be removed in 4.0.
44+
*/
45+
public function getChanges()
46+
{
47+
$changes = $this->changes;
48+
$this->changes = array();
49+
50+
return $changes;
51+
}
52+
}

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

Lines changed: 34 additions & 3 deletions
638
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
103103
*/
104104
private $envCounters = array();
105105

106+
/**
107+
* @var array a map of case less to case sensitive ids
108+
*/
109+
private $caseSensitiveIds = array();
110+
106111
/**
107112
* Sets the track resources flag.
108113
*
@@ -367,14 +372,18 @@ public function getCompiler()
367372
*/
368373
public function set($id, $service)
369374
{
370-
$id = strtolower($id);
375+
$caseSensitiveId = $id;
376+
$id = strtolower($caseSensitiveId);
371377

372378
if ($this->isFrozen() && (isset($this->definitions[$id]) && !$this->definitions[$id]->isSynthetic())) {
373379
// setting a synthetic service on a frozen container is alright
374380
throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a frozen container is not allowed.', $id));
375381
}
376382

377383
unset($this->definitions[$id], $this->aliasDefinitions[$id]);
384+
if ($id !== $caseSensitiveId) {
385+
$this->caseSensitiveIds[$id] = $caseSensitiveId;
386+
}
378387

379388
parent::set($id, $service);
380389
}
@@ -628,7 +637,8 @@ public function setAliases(array $aliases)
628637
*/
629
public function setAlias($alias, $id)
630639
{
631-
$alias = strtolower($alias);
640+
$caseSensitiveAlias = $alias;
641+
$alias = strtolower($caseSensitiveAlias);
632642

633643
if (is_string($id)) {
634644
$id = new Alias($id);
@@ -641,6 +651,9 @@ public function setAlias($alias, $id)
641651
}
642652

643653
unset($this->definitions[$alias]);
654+
if ($alias !== $caseSensitiveAlias) {
655+
$this->caseSensitiveIds[$alias] = $caseSensitiveAlias;
656+
}
644657

645658
$this->aliasDefinitions[$alias] = $id;
646659
}
@@ -778,9 +791,13 @@ public function setDefinition($id, Definition $definition)
778791
throw new BadMethodCallException('Adding definition to a frozen container is not allowed');
779792
}
780793

781-
$id = strtolower($id);
794+
$caseSensitiveId = $id;
795+
$id = strtolower($caseSensitiveId);
782796

783797
unset($this->aliasDefinitions[$id]);
798+
if ($id !== $caseSensitiveId) {
799+
$this->caseSensitiveIds[$id] = $caseSensitiveId;
800+
}
784801

785802
return $this->definitions[$id] = $definition;
786803
}
@@ -839,6 +856,20 @@ public function findDefinition($id)
839856
return $this->getDefinition($id);
840857
}
841858

859+
/**
860+
* Returns the case sensitive id used at registration time.
861+
*
862+
* @param string $id
863+
*
864+
* @return string
865+
*/
866+
public function getCaseSensitiveId($id)
867+
{
868+
$id = strtolower($id);
869+
870+
return isset($this->caseSensitiveIds[$id]) ? $this->caseSensitiveIds[$id] : $id;
871+
}
872+
842873
/**
843874
* Creates a service for a service definition.
844875
*

src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ private function processAnonymousServices(\DOMDocument $xml, $file)
303303
if (false !== $nodes = $xpath->query('//container:argument[@type="service"][not(@id)]|//container:property[@type="service"][not(@id)]')) {
304304
foreach ($nodes as $node) {
305305
// give it a unique name
306-
$id = sprintf('%s_%d', hash('sha256', $file), ++$count);
306+
$id = sprintf('%d_%s', ++$count, hash('sha256', $file));
307307
$node->setAttribute('id', $id);
308308

309309
if ($services = $this->getChildren($node, 'service')) {
@@ -321,15 +321,15 @@ private function processAnonymousServices(\DOMDocument $xml, $file)
321321
if (false !== $nodes = $xpath->query('//container:services/container:service[not(@id)]')) {
322322
foreach ($nodes as $node) {
323323
// give it a unique name
324-
$id = sprintf('%s_%d', hash('sha256', $file), ++$count);
324+
$id = sprintf('%d_%s', ++$count, hash('sha256', $file));
325325
$node->setAttribute('id', $id);
326326
$definitions[$id] = array($node, $file, true);
327327
}
328328
}
329329

330330
// resolve definitions
331-
krsort($definitions);
332-
foreach ($definitions as $id => list($domElement, $file, $wild)) {
331+
uksort($definitions, 'strnatcmp');
332+
foreach (array_reverse($definitions) as $id => list($domElement, $file, $wild)) {
333333
if (null !== $definition = $this->parseDefinition($domElement, $file)) {
334334
$this->container->setDefinition($id, $definition);
335335
}

src/Symfony/Component/DependencyInjection/Tests/Compiler/FactoryReturnTypePassTest.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
/**
2222
* @author Guilhem N. <egetick@gmail.com>
23+
*
24+
* @group legacy
2325
*/
2426
class FactoryReturnTypePassTest extends \PHPUnit_Framework_TestCase
2527
{
@@ -103,17 +105,16 @@ public function testCircularReference()
103105
$this->assertNull($factory2->getClass());
104106
}
105107

108+
/**
109+
* @requires function ReflectionMethod::getReturnType
110+
* @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.
111+
*/
106112
public function testCompile()
107113
{
108114
$container = new ContainerBuilder();
109115

110116
$factory = $container->register('factory');
111117
$factory->setFactory(array(FactoryDummy::class, 'createFactory'));
112-
113-
if (!method_exists(\ReflectionMethod::class, 'getReturnType')) {
114-
$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.');
115-
}
116-
117118
$container->compile();
118119

119120
$this->assertEquals(FactoryDummy::class, $container->getDefinition('factory')->getClass());

src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
3333
use Symfony\Component\Config\Resource\FileResource;
3434
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
35+
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
3536
use Symfony\Component\ExpressionLanguage\Expression;
3637

3738
class ContainerBuilderTest extends \PHPUnit_Framework_TestCase
@@ -926,6 +927,44 @@ public function testClosureProxyOnInvalidException()
926927

927928
$container->get('foo');
928929
}
930+
931+
public function testClassFromId()
932+
{
933+
$container = new ContainerBuilder();
934+
935+
$unknown = $container->register('unknown_class');
936+
$class = $container->register(\stdClass::class);
937+
$autoloadClass = $container->register(CaseSensitiveClass::class);
938+
$container->compile();
939+
940+
$this->assertSame('unknown_class', $unknown->getClass());
941+
$this->assertEquals(\stdClass::class, $class->getClass());
942+
$this->assertEquals(CaseSensitiveClass::class, $autoloadClass->getClass());
943+
}
944+
945+
/**
946+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
947+
* @expectedExceptionMessage The definition for "123_abc" has no class.
948+
*/
949+
public function testNoClassFromNonClassId()
950+
{
951+
$container = new ContainerBuilder();
952+
953+
$definition = $container->register('123_abc');
954+
$container->compile();
955+
}
956+
957+
/**
958+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
959+
* @expectedExceptionMessage The definition for "\foo" has no class.
960+
*/
961+
public function testNoClassFromNsSeparatorId()
962+
{
963+
$container = new ContainerBuilder();
964+
965+
$definition = $container->register('\\foo');
966+
$container->compile();
967+
}
929968
}
930969

931970
class FooClass
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
13+
14+
class CaseSensitiveClass
15+
{
16+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<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">
3+
<services>
4+
<service id="Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass" />
5+
</services>
6+
</container>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
services:
2+
Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass:
3+
autowire: true

src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\DependencyInjection\Loader\IniFileLoader;
2121
use Symfony\Component\Config\Loader\LoaderResolver;
2222
use Symfony\Component\Config\FileLocator;
23+
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
2324
use Symfony\Component\ExpressionLanguage\Expression;
2425

2526
class XmlFileLoaderTest extends \PHPUnit_Framework_TestCase
@@ -582,6 +583,16 @@ public function testAutowireAttributeAndTag()
582583
$loader->load('services28.xml');
583584
}
584585

586+
public function testClassFromId()
587+
{
588+
$container = new ContainerBuilder();
589+
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
590+
$loader->load('class_from_id.xml');
591+
$container->compile();
592+
593+
$this->assertEquals(CaseSensitiveClass::class, $container->getDefinition(CaseSensitiveClass::class)->getClass());
594+
}
595+
585596
/**
586597
* @group legacy
587598
* @expectedDeprecation Using the attribute "class" is deprecated for the service "bar" which is defined as an alias %s.

0 commit comments

Comments
 (0)
0