Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 08ef799

Browse filesBrowse 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
Copy full SHA for 08ef799

File tree

5 files changed

+141
-75
lines changed
Filter options

5 files changed

+141
-75
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php
+89-1Lines changed: 89 additions & 1 deletion
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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+15-24Lines changed: 15 additions & 24 deletions
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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php
+7-40Lines changed: 7 additions & 40 deletions
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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+9-7Lines changed: 9 additions & 7 deletions
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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveNamedArgumentsPassTest.php
+21-3Lines changed: 21 additions & 3 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.