Skip to content

Commit ec11187

Browse files
nicolas-grekasnicolas-grekas
committed
feature #24301 [DI] Add AutowireRequiredMethodsPass to fix bindings for @required methods (nicolas-grekas)
This PR was merged into the 3.4 branch. Discussion ---------- [DI] Add AutowireRequiredMethodsPass to fix bindings for `@required` methods | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | yes | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Spotted while doing a SF4 workshop :) Discovery of `@required` methods should be split from AutowirePass so that bindings can apply to these methods also when autowiring is enabled. Commits ------- dc55dd2 [DI] Add AutowireRequiredMethodsPass to fix bindings for `@required` methods
2 parents 0c0a052 + dc55dd2 commit ec11187

File tree

9 files changed

+535
-401
lines changed

9 files changed

+535
-401
lines changed

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ protected function getConstructor(Definition $definition, $required)
9090
{
9191
if (is_string($factory = $definition->getFactory())) {
9292
if (!function_exists($factory)) {
93-
throw new RuntimeException(sprintf('Unable to resolve service "%s": function "%s" does not exist.', $this->currentId, $factory));
93+
throw new RuntimeException(sprintf('Invalid service "%s": function "%s" does not exist.', $this->currentId, $factory));
9494
}
9595
$r = new \ReflectionFunction($factory);
9696
if (false !== $r->getFileName() && file_exists($r->getFileName())) {
@@ -108,7 +108,7 @@ protected function getConstructor(Definition $definition, $required)
108108
$class = $definition->getClass();
109109
}
110110
if ('__construct' === $method) {
111-
throw new RuntimeException(sprintf('Unable to resolve service "%s": "__construct()" cannot be used as a factory method.', $this->currentId));
111+
throw new RuntimeException(sprintf('Invalid service "%s": "__construct()" cannot be used as a factory method.', $this->currentId));
112112
}
113113

114114
return $this->getReflectionMethod(new Definition($class), $method);
@@ -117,14 +117,14 @@ protected function getConstructor(Definition $definition, $required)
117117
$class = $definition->getClass();
118118

119119
if (!$r = $this->container->getReflectionClass($class)) {
120-
throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class));
120+
throw new RuntimeException(sprintf('Invalid service "%s": class "%s" does not exist.', $this->currentId, $class));
121121
}
122122
if (!$r = $r->getConstructor()) {
123123
if ($required) {
124-
throw new RuntimeException(sprintf('Unable to resolve service "%s": class%s has no constructor.', $this->currentId, sprintf($class !== $this->currentId ? ' "%s"' : '', $class)));
124+
throw new RuntimeException(sprintf('Invalid service "%s": class%s has no constructor.', $this->currentId, sprintf($class !== $this->currentId ? ' "%s"' : '', $class)));
125125
}
126126
} elseif (!$r->isPublic()) {
127-
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)));
127+
throw new RuntimeException(sprintf('Invalid service "%s": %s must be public.', $this->currentId, sprintf($class !== $this->currentId ? 'constructor of class "%s"' : 'its constructor', $class)));
128128
}
129129

130130
return $r;
@@ -145,20 +145,20 @@ protected function getReflectionMethod(Definition $definition, $method)
145145
}
146146

147147
if (!$class = $definition->getClass()) {
148-
throw new RuntimeException(sprintf('Unable to resolve service "%s": the class is not set.', $this->currentId));
148+
throw new RuntimeException(sprintf('Invalid service "%s": the class is not set.', $this->currentId));
149149
}
150150

151151
if (!$r = $this->container->getReflectionClass($class)) {
152-
throw new RuntimeException(sprintf('Unable to resolve service "%s": class "%s" does not exist.', $this->currentId, $class));
152+
throw new RuntimeException(sprintf('Invalid service "%s": class "%s" does not exist.', $this->currentId, $class));
153153
}
154154

155155
if (!$r->hasMethod($method)) {
156-
throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
156+
throw new RuntimeException(sprintf('Invalid service "%s": method "%s()" does not exist.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
157157
}
158158

159159
$r = $r->getMethod($method);
160160
if (!$r->isPublic()) {
161-
throw new RuntimeException(sprintf('Unable to resolve service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
161+
throw new RuntimeException(sprintf('Invalid service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
162162
}
163163

164164
return $r;

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

Lines changed: 3 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ private function doProcessValue($value, $isRoot = false)
130130
return $value;
131131
}
132132

133-
$autowiredMethods = $this->getMethodsToAutowire($reflectionClass);
134133
$methodCalls = $value->getMethodCalls();
135134

136135
try {
@@ -143,7 +142,7 @@ private function doProcessValue($value, $isRoot = false)
143142
array_unshift($methodCalls, array($constructor, $value->getArguments()));
144143
}
145144

146-
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods);
145+
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls);
147146

148147
if ($constructor) {
149148
list(, $arguments) = array_shift($methodCalls);
@@ -161,61 +160,18 @@ private function doProcessValue($value, $isRoot = false)
161160
}
162161

163162
/**
164-
* Gets the list of methods to autowire.
165-
*
166163
* @param \ReflectionClass $reflectionClass
167-
*
168-
* @return \ReflectionMethod[]
169-
*/
170-
private function getMethodsToAutowire(\ReflectionClass $reflectionClass)
171-
{
172-
$methodsToAutowire = array();
173-
174-
foreach ($reflectionClass->getMethods() as $reflectionMethod) {
175-
$r = $reflectionMethod;
176-
177-
if ($r->isConstructor()) {
178-
continue;
179-
}
180-
181-
while (true) {
182-
if (false !== $doc = $r->getDocComment()) {
183-
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
184-
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
185-
break;
186-
}
187-
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
188-
break;
189-
}
190-
}
191-
try {
192-
$r = $r->getPrototype();
193-
} catch (\ReflectionException $e) {
194-
break; // method has no prototype
195-
}
196-
}
197-
}
198-
199-
return $methodsToAutowire;
200-
}
201-
202-
/**
203-
* @param \ReflectionClass $reflectionClass
204-
* @param array $methodCalls
205-
* @param \ReflectionMethod[] $autowiredMethods
164+
* @param array $methodCalls
206165
*
207166
* @return array
208167
*/
209-
private function autowireCalls(\ReflectionClass $reflectionClass, array $methodCalls, array $autowiredMethods)
168+
private function autowireCalls(\ReflectionClass $reflectionClass, array $methodCalls)
210169
{
211170
foreach ($methodCalls as $i => $call) {
212171
list($method, $arguments) = $call;
213172

214173
if ($method instanceof \ReflectionFunctionAbstract) {
215174
$reflectionMethod = $method;
216-
} elseif (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) {
217-
$reflectionMethod = $autowiredMethods[$lcMethod];
218-
unset($autowiredMethods[$lcMethod]);
219175
} else {
220176
$reflectionMethod = $this->getReflectionMethod(new Definition($reflectionClass->name), $method);
221177
}
@@ -227,16 +183,6 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
227183
}
228184
}
229185

230-
foreach ($autowiredMethods as $lcMethod => $reflectionMethod) {
231-
$method = $reflectionMethod->name;
232-
233-
if (!$reflectionMethod->isPublic()) {
234-
$class = $reflectionClass->name;
235-
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
236-
}
237-
$methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
238-
}
239-
240186
return $methodCalls;
241187
}
242188

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

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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\Definition;
15+
16+
/**
17+
* Looks for definitions with autowiring enabled and registers their corresponding "@required" methods as setters.
18+
*
19+
* @author Nicolas Grekas <p@tchwork.com>
20+
*/
21+
class AutowireRequiredMethodsPass extends AbstractRecursivePass
22+
{
23+
/**
24+
* {@inheritdoc}
25+
*/
26+
protected function processValue($value, $isRoot = false)
27+
{
28+
$value = parent::processValue($value, $isRoot);
29+
30+
if (!$value instanceof Definition || !$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
31+
return $value;
32+
}
33+
if (!$reflectionClass = $this->container->getReflectionClass($value->getClass(), false)) {
34+
return $value;
35+
}
36+
37+
$alreadyCalledMethods = array();
38+
39+
foreach ($value->getMethodCalls() as list($method)) {
40+
$alreadyCalledMethods[strtolower($method)] = true;
41+
}
42+
43+
foreach ($reflectionClass->getMethods() as $reflectionMethod) {
44+
$r = $reflectionMethod;
45+
46+
if ($r->isConstructor() || isset($alreadyCalledMethods[strtolower($r->name)])) {
47+
continue;
48+
}
49+
50+
while (true) {
51+
if (false !== $doc = $r->getDocComment()) {
52+
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
53+
$value->addMethodCall($reflectionMethod->name);
54+
break;
55+
}
56+
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
57+
break;
58+
}
59+
}
60+
try {
61+
$r = $r->getPrototype();
62+
} catch (\ReflectionException $e) {
63+
break; // method has no prototype
64+
}
65+
}
66+
}
67+
68+
return $value;
69+
}
70+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public function __construct()
5858
new CheckDefinitionValidityPass(),
5959
new RegisterServiceSubscribersPass(),
6060
new ResolveNamedArgumentsPass(),
61+
new AutowireRequiredMethodsPass(),
6162
new ResolveBindingsPass(),
6263
$autowirePass = new AutowirePass(false),
6364
new ResolveServiceSubscribersPass(),

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,11 @@ protected function processValue($value, $isRoot = false)
6161
}
6262
}
6363

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));
64+
throw new InvalidArgumentException(sprintf('Invalid service "%s": method "%s()" has no argument named "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
6565
}
6666

6767
if (null !== $argument && !$argument instanceof Reference && !$argument instanceof Definition) {
68-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": the value of argument "%s" of method "%s()" must be null, an instance of %s or an instance of %s, %s given.', $this->currentId, $key, $class !== $this->currentId ? $class.'::'.$method : $method, Reference::class, Definition::class, gettype($argument)));
68+
throw new InvalidArgumentException(sprintf('Invalid service "%s": the value of argument "%s" of method "%s()" must be null, an instance of %s or an instance of %s, %s given.', $this->currentId, $key, $class !== $this->currentId ? $class.'::'.$method : $method, Reference::class, Definition::class, gettype($argument)));
6969
}
7070

7171
foreach ($parameters as $j => $p) {
@@ -76,7 +76,7 @@ protected function processValue($value, $isRoot = false)
7676
}
7777
}
7878

79-
throw new InvalidArgumentException(sprintf('Unable to resolve service "%s": method "%s()" has no argument type-hinted as "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
79+
throw new InvalidArgumentException(sprintf('Invalid service "%s": method "%s()" has no argument type-hinted as "%s". Check your service definition.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method, $key));
8080
}
8181

8282
if ($resolvedArguments !== $call[1]) {

0 commit comments

Comments
 (0)