Skip to content

Commit 08ef799

Browse files
fabpotfabpot
committed
feature #22277 [DI] Add "factory" support to named args and autowiring (nicolas-grekas)
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Add "factory" support to named args and autowiring | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - To me, factories are expected to work named arguments. Doing so also unlocks supporting them when autowiring, and will benefit #22187 soon. Commits ------- 27470de [DI] Add "factory" support to named args and autowiring
2 parents 6e54cdf + 27470de commit 08ef799

File tree

5 files changed

+141
-75
lines changed

5 files changed

+141
-75
lines changed

‎src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php

+89-1
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
namespace Symfony\Component\DependencyInjection\Compiler;
1313

1414
use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
15-
use Symfony\Component\DependencyInjection\Definition;
1615
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Definition;
17+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
18+
use Symfony\Component\DependencyInjection\Reference;
1719

1820
/**
1921
* @author Nicolas Grekas <p@tchwork.com>
@@ -73,4 +75,90 @@ protected function processValue($value, $isRoot = false)
7375

7476
return $value;
7577
}
78+
79+
/**
80+
* @param Definition $definition
81+
* @param bool $required
82+
*
83+
* @return \ReflectionFunctionAbstract|null
84+
*
85+
* @throws RuntimeException
86+
*/
87+
protected function getConstructor(Definition $definition, $required)
88+
{
89+
if (is_string($factory = $definition->getFactory())) {
90+
if (!function_exists($factory)) {
91+
throw new RuntimeException(sprintf('Unable to resolve service "%s": function "%s" does not exist.', $this->currentId, $factory));
92+
}
93+
$r = new \ReflectionFunction($factory);
94+
if (false !== $r->getFileName() && file_exists($r->getFileName())) {
95+
$this->container->fileExists($r->getFileName());
96+
}
97+
98+
return $r;
99+
}
100+
101+
if ($factory) {
102+
list($class, $method) = $factory;
103+
if ($class instanceof Reference) {
104+
$class = $this->container->findDefinition((string) $class)->getClass();
105+
} elseif (null === $class) {
106+
$class = $definition->getClass();
107+
}
108+
if ('__construct' === $method) {
109+
throw new RuntimeException(sprintf('Unable to resolve service "%s": "__construct()" cannot be used as a factory method.', $this->currentId));
110+
}
111+
112+
return $this->getReflectionMethod(new Definition($class), $method);
113+
}
114+
115+
$class = $definition->getClass();
116+
117+
if (!$r = $this->container->getReflectionClass($class)) {
118+
throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class));
119+
}
120+
if (!$r = $r->getConstructor()) {
121+
if ($required) {
122+
throw new RuntimeException(sprintf('Unable to resolve service "%s": class%s has no constructor.', $this->currentId, sprintf($class !== $this->currentId ? ' "%s"' : '', $class)));
123+
}
124+
} elseif (!$r->isPublic()) {
125+
throw new RuntimeException(sprintf('Unable to resolve service "%s": %s must be public.', $this->currentId, sprintf($class !== $this->currentId ? 'constructor of class "%s"' : 'its constructor', $class)));
126+
}
127+
128+
return $r;
129+
}
130+
131+
/**
132+
* @param Definition $definition
133+
* @param string $method
134+
*
135+
* @throws RuntimeException
136+
*
137+
* @return \ReflectionFunctionAbstract
138+
*/
139+
protected function getReflectionMethod(Definition $definition, $method)
140+
{
141+
if ('__construct' === $method) {
142+
return $this->getConstructor($definition, true);
143+
}
144+
145+
if (!$class = $definition->getClass()) {
146+
throw new RuntimeException(sprintf('Unable to resolve service "%s": the class is not set.', $this->currentId));
147+
}
148+
149+
if (!$r = $this->container->getReflectionClass($class)) {
150+
throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class));
151+
}
152+
153+
if (!$r->hasMethod($method)) {
154+
throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
155+
}
156+
157+
$r = $r->getMethod($method);
158+
if (!$r->isPublic()) {
159+
throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
160+
}
161+
162+
return $r;
163+
}
76164
}

‎src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

+15-24
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,6 @@ protected function processValue($value, $isRoot = false)
9595
if (!$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
9696
return parent::processValue($value, $isRoot);
9797
}
98-
if ($value->getFactory()) {
99-
throw new RuntimeException(sprintf('Service "%s" can use either autowiring or a factory, not both.', $this->currentId));
100-
}
10198
if (!$reflectionClass = $this->container->getReflectionClass($value->getClass())) {
10299
$this->container->log($this, sprintf('Skipping service "%s": Class or interface "%s" does not exist.', $this->currentId, $value->getClass()));
103100

@@ -107,8 +104,8 @@ protected function processValue($value, $isRoot = false)
107104
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass);
108105
$methodCalls = $value->getMethodCalls();
109106

110-
if ($constructor = $reflectionClass->getConstructor()) {
111-
array_unshift($methodCalls, array($constructor->name, $value->getArguments()));
107+
if ($constructor = $this->getConstructor($value, false)) {
108+
array_unshift($methodCalls, array($constructor, $value->getArguments()));
112109
}
113110

114111
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods);
@@ -143,13 +140,13 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass)
143140
$found = array();
144141
$methodsToAutowire = array();
145142

146-
if ($reflectionMethod = $reflectionClass->getConstructor()) {
147-
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
148-
}
149-
150143
foreach ($reflectionClass->getMethods() as $reflectionMethod) {
151144
$r = $reflectionMethod;
152145

146+
if ($r->isConstructor()) {
147+
continue;
148+
}
149+
153150
while (true) {
154151
if (false !== $doc = $r->getDocComment()) {
155152
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
@@ -183,19 +180,13 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
183180
foreach ($methodCalls as $i => $call) {
184181
list($method, $arguments) = $call;
185182

186-
if (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) {
183+
if ($method instanceof \ReflectionFunctionAbstract) {
184+
$reflectionMethod = $method;
185+
} elseif (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) {
187186
$reflectionMethod = $autowiredMethods[$lcMethod];
188187
unset($autowiredMethods[$lcMethod]);
189188
} else {
190-
if (!$reflectionClass->hasMethod($method)) {
191-
$class = $reflectionClass->name;
192-
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
193-
}
194-
$reflectionMethod = $reflectionClass->getMethod($method);
195-
if (!$reflectionMethod->isPublic()) {
196-
$class = $reflectionClass->name;
197-
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
198-
}
189+
$reflectionMethod = $this->getReflectionMethod($this->currentDefinition, $method);
199190
}
200191

201192
$arguments = $this->autowireMethod($reflectionMethod, $arguments);
@@ -210,7 +201,7 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
210201

211202
if (!$reflectionMethod->isPublic()) {
212203
$class = $reflectionClass->name;
213-
throw new RuntimeException(sprintf('Cannot autowire service "%s": method %s() must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
204+
throw new RuntimeException(sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
214205
}
215206
$methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
216207
}
@@ -221,16 +212,16 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
221212
/**
222213
* Autowires the constructor or a method.
223214
*
224-
* @param \ReflectionMethod $reflectionMethod
225-
* @param array $arguments
215+
* @param \ReflectionFunctionAbstract $reflectionMethod
216+
* @param array $arguments
226217
*
227218
* @return array The autowired arguments
228219
*
229220
* @throws RuntimeException
230221
*/
231-
private function autowireMethod(\ReflectionMethod $reflectionMethod, array $arguments)
222+
private function autowireMethod(\ReflectionFunctionAbstract $reflectionMethod, array $arguments)
232223
{
233-
$class = $reflectionMethod->class;
224+
$class = $reflectionMethod instanceof \ReflectionMethod ? $reflectionMethod->class : $this->currentId;
234225
$method = $reflectionMethod->name;
235226
$parameters = $reflectionMethod->getParameters();
236227
if (method_exists('ReflectionMethod', 'isVariadic') && $reflectionMethod->isVariadic()) {

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php

+7-40
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,11 @@ protected function processValue($value, $isRoot = false)
3030
return parent::processValue($value, $isRoot);
3131
}
3232

33-
$parameterBag = $this->container->getParameterBag();
34-
35-
if ($class = $value->getClass()) {
36-
$class = $parameterBag->resolveValue($class);
37-
}
38-
3933
$calls = $value->getMethodCalls();
4034
$calls[] = array('__construct', $value->getArguments());
4135

4236
foreach ($calls as $i => $call) {
4337
list($method, $arguments) = $call;
44-
$method = $parameterBag->resolveValue($method);
4538
$parameters = null;
4639
$resolvedArguments = array();
4740

@@ -51,10 +44,14 @@ protected function processValue($value, $isRoot = false)
5144
continue;
5245
}
5346
if ('' === $key || '$' !== $key[0]) {
54-
throw new InvalidArgumentException(sprintf('Invalid key "%s" found in arguments of method "%s" for service "%s": only integer or $named arguments are allowed.', $key, $method, $this->currentId));
47+
throw new InvalidArgumentException(sprintf('Invalid key "%s" found in arguments of method "%s()" for service "%s": only integer or $named arguments are allowed.', $key, $method, $this->currentId));
5548
}
5649

57-
$parameters = null !== $parameters ? $parameters : $this->getParameters($class, $method);
50+
if (null === $parameters) {
51+
$r = $this->getReflectionMethod($value, $method);
52+
$class = $r instanceof \ReflectionMethod ? $r->class : $this->currentId;
53+
$parameters = $r->getParameters();
54+
}
5855

5956
foreach ($parameters as $j => $p) {
6057
if ($key === '$'.$p->name) {
@@ -64,7 +61,7 @@ protected function processValue($value, $isRoot = false)
6461
}
6562
}
6663

67-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" has no argument named "%s". Check your service definition.', $this->currentId, $class, $method, $key));
64+
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s()" has no argument named "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
6865
}
6966

7067
if ($resolvedArguments !== $call[1]) {
@@ -84,34 +81,4 @@ protected function processValue($value, $isRoot = false)
8481

8582
return parent::processValue($value, $isRoot);
8683
}
87-
88-
/**
89-
* @param string|null $class
90-
* @param string $method
91-
*
92-
* @throws InvalidArgumentException
93-
*
94-
* @return array
95-
*/
96-
private function getParameters($class, $method)
97-
{
98-
if (!$class) {
99-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": the class is not set.', $this->currentId));
100-
}
101-
102-
if (!$r = $this->container->getReflectionClass($class)) {
103-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class));
104-
}
105-
106-
if (!$r->hasMethod($method)) {
107-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" does not exist.', $this->currentId, $class, $method));
108-
}
109-
110-
$method = $r->getMethod($method);
111-
if (!$method->isPublic()) {
112-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s::%s" must be public.', $this->currentId, $class, $method->name));
113-
}
114-
115-
return $method->getParameters();
116-
}
11784
}

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

+9-7
Original file line numberDiff line numberDiff line change
@@ -611,20 +611,19 @@ public function testEmptyStringIsKept()
611611
$this->assertEquals(array(new Reference('a'), '', new Reference('lille')), $container->getDefinition('foo')->getArguments());
612612
}
613613

614-
/**
615-
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
616-
* @expectedExceptionMessage Service "a" can use either autowiring or a factory, not both.
617-
*/
618614
public function testWithFactory()
619615
{
620616
$container = new ContainerBuilder();
621617

622-
$container->register('a', __NAMESPACE__.'\A')
623-
->setFactory('foo')
618+
$container->register('foo', Foo::class);
619+
$definition = $container->register('a', A::class)
620+
->setFactory(array(A::class, 'create'))
624621
->setAutowired(true);
625622

626623
$pass = new AutowirePass();
627624
$pass->process($container);
625+
626+
$this->assertEquals(array(new Reference('foo')), $definition->getArguments());
628627
}
629628

630629
/**
@@ -662,7 +661,7 @@ public function provideNotWireableCalls()
662661
{
663662
return array(
664663
array('setNotAutowireable', 'Cannot autowire service "foo": argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() has type "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" but this class does not exist.'),
665-
array(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'),
664+
array(null, 'Cannot autowire service "foo": method "Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod()" must be public.'),
666665
);
667666
}
668667

@@ -745,6 +744,9 @@ public function __construct(Foo $foo)
745744

746745
class A
747746
{
747+
public static function create(Foo $foo)
748+
{
749+
}
748750
}
749751

750752
class B extends A

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php

+21-3
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,23 @@ public function testProcess()
3737
$this->assertEquals(array(array('setApiKey', array('123'))), $definition->getMethodCalls());
3838
}
3939

40+
public function testWithFactory()
41+
{
42+
$container = new ContainerBuilder();
43+
44+
$container->register('factory', NoConstructor::class);
45+
$definition = $container->register('foo', NoConstructor::class)
46+
->setFactory(array(new Reference('factory'), 'create'))
47+
->setArguments(array('$apiKey' => '123'));
48+
49+
$pass = new ResolveNamedArgumentsPass();
50+
$pass->process($container);
51+
52+
$this->assertSame(array(0 => '123'), $definition->getArguments());
53+
}
54+
4055
/**
41-
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
56+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
4257
*/
4358
public function testClassNull()
4459
{
@@ -52,7 +67,7 @@ public function testClassNull()
5267
}
5368

5469
/**
55-
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
70+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
5671
*/
5772
public function testClassNotExist()
5873
{
@@ -66,7 +81,7 @@ public function testClassNotExist()
6681
}
6782

6883
/**
69-
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
84+
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
7085
*/
7186
public function testClassNoConstructor()
7287
{
@@ -96,4 +111,7 @@ public function testArgumentNotFound()
96111

97112
class NoConstructor
98113
{
114+
public static function create($apiKey)
115+
{
116+
}
99117
}

0 commit comments

Comments
 (0)