Skip to content

Commit

Permalink
feature #24301 [DI] Add AutowireRequiredMethodsPass to fix bindings f…
Browse files Browse the repository at this point in the history 
…or `@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
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Sep 25, 2017
2 parents 0c0a052 + dc55dd2 commit ec11187
Show file tree
Hide file tree
Showing 9 changed files with 535 additions and 401 deletions.
18 changes: 9 additions & 9 deletions src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php
View file
Expand Up @@ -90,7 +90,7 @@ protected function getConstructor(Definition $definition, $required)
{
if (is_string($factory = $definition->getFactory())) {
if (!function_exists($factory)) {
throw new RuntimeException(sprintf('Unable to resolve service "%s": function "%s" does not exist.', $this->currentId, $factory));
throw new RuntimeException(sprintf('Invalid service "%s": function "%s" does not exist.', $this->currentId, $factory));
}
$r = new \ReflectionFunction($factory);
if (false !== $r->getFileName() && file_exists($r->getFileName())) {
Expand All @@ -108,7 +108,7 @@ protected function getConstructor(Definition $definition, $required)
$class = $definition->getClass();
}
if ('__construct' === $method) {
throw new RuntimeException(sprintf('Unable to resolve service "%s": "__construct()" cannot be used as a factory method.', $this->currentId));
throw new RuntimeException(sprintf('Invalid service "%s": "__construct()" cannot be used as a factory method.', $this->currentId));
}

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

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

return $r;
Expand All @@ -145,20 +145,20 @@ protected function getReflectionMethod(Definition $definition, $method)
}

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

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

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

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

return $r;
Expand Down
60 changes: 3 additions & 57 deletions src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
View file
Expand Up @@ -130,7 +130,6 @@ private function doProcessValue($value, $isRoot = false)
return $value;
}

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

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

$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls, $autowiredMethods);
$methodCalls = $this->autowireCalls($reflectionClass, $methodCalls);

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

/**
* Gets the list of methods to autowire.
*
* @param \ReflectionClass $reflectionClass
*
* @return \ReflectionMethod[]
*/
private function getMethodsToAutowire(\ReflectionClass $reflectionClass)
{
$methodsToAutowire = array();

foreach ($reflectionClass->getMethods() as $reflectionMethod) {
$r = $reflectionMethod;

if ($r->isConstructor()) {
continue;
}

while (true) {
if (false !== $doc = $r->getDocComment()) {
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
$methodsToAutowire[strtolower($reflectionMethod->name)] = $reflectionMethod;
break;
}
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
break;
}
}
try {
$r = $r->getPrototype();
} catch (\ReflectionException $e) {
break; // method has no prototype
}
}
}

return $methodsToAutowire;
}

/**
* @param \ReflectionClass $reflectionClass
* @param array $methodCalls
* @param \ReflectionMethod[] $autowiredMethods
* @param array $methodCalls
*
* @return array
*/
private function autowireCalls(\ReflectionClass $reflectionClass, array $methodCalls, array $autowiredMethods)
private function autowireCalls(\ReflectionClass $reflectionClass, array $methodCalls)
{
foreach ($methodCalls as $i => $call) {
list($method, $arguments) = $call;

if ($method instanceof \ReflectionFunctionAbstract) {
$reflectionMethod = $method;
} elseif (isset($autowiredMethods[$lcMethod = strtolower($method)]) && $autowiredMethods[$lcMethod]->isPublic()) {
$reflectionMethod = $autowiredMethods[$lcMethod];
unset($autowiredMethods[$lcMethod]);
} else {
$reflectionMethod = $this->getReflectionMethod(new Definition($reflectionClass->name), $method);
}
Expand All @@ -227,16 +183,6 @@ private function autowireCalls(\ReflectionClass $reflectionClass, array $methodC
}
}

foreach ($autowiredMethods as $lcMethod => $reflectionMethod) {
$method = $reflectionMethod->name;

if (!$reflectionMethod->isPublic()) {
$class = $reflectionClass->name;
throw new AutowiringFailedException($this->currentId, sprintf('Cannot autowire service "%s": method "%s()" must be public.', $this->currentId, $class !== $this->currentId ? $class.'::'.$method : $method));
}
$methodCalls[] = array($method, $this->autowireMethod($reflectionMethod, array()));
}

return $methodCalls;
}

Expand Down
70 changes: 70 additions & 0 deletions src/Symfony/Component/DependencyInjection/Compiler/AutowireRequiredMethodsPass.php
View file
@@ -0,0 +1,70 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* 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\Definition;

/**
* Looks for definitions with autowiring enabled and registers their corresponding "@required" methods as setters.
*
* @author Nicolas Grekas <p@tchwork.com>
*/
class AutowireRequiredMethodsPass extends AbstractRecursivePass
{
/**
* {@inheritdoc}
*/
protected function processValue($value, $isRoot = false)
{
$value = parent::processValue($value, $isRoot);

if (!$value instanceof Definition || !$value->isAutowired() || $value->isAbstract() || !$value->getClass()) {
return $value;
}
if (!$reflectionClass = $this->container->getReflectionClass($value->getClass(), false)) {
return $value;
}

$alreadyCalledMethods = array();

foreach ($value->getMethodCalls() as list($method)) {
$alreadyCalledMethods[strtolower($method)] = true;
}

foreach ($reflectionClass->getMethods() as $reflectionMethod) {
$r = $reflectionMethod;

if ($r->isConstructor() || isset($alreadyCalledMethods[strtolower($r->name)])) {
continue;
}

while (true) {
if (false !== $doc = $r->getDocComment()) {
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
$value->addMethodCall($reflectionMethod->name);
break;
}
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
break;
}
}
try {
$r = $r->getPrototype();
} catch (\ReflectionException $e) {
break; // method has no prototype
}
}
}

return $value;
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
View file
Expand Up @@ -58,6 +58,7 @@ public function __construct()
new CheckDefinitionValidityPass(),
new RegisterServiceSubscribersPass(),
new ResolveNamedArgumentsPass(),
new AutowireRequiredMethodsPass(),
new ResolveBindingsPass(),
$autowirePass = new AutowirePass(false),
new ResolveServiceSubscribersPass(),
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveNamedArgumentsPass.php
View file
Expand Up @@ -61,11 +61,11 @@ protected function processValue($value, $isRoot = false)
}
}

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));
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));
}

if (null !== $argument && !$argument instanceof Reference && !$argument instanceof Definition) {
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)));
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)));
}

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

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));
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));
}

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

0 comments on commit ec11187

Please sign in to comment.