bug #22282 [DI] Prevent AutowirePass from triggering irrelevant deprecations (chalasr)

This PR was merged into the 2.8 branch.

Discussion
----------

[DI] Prevent AutowirePass from triggering irrelevant deprecations

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no (just a failing test case atm)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

For populating available types the AutowirePass iterates over `$container->getDefinitions()` trying to instantiate a reflection class for each definition.
Problem is that if one of these classes is deprecated, a notice is triggered due to the reflection, even if the service is actually never used.

~~Right now this only reproduces the issue with a failing test case~~, this bug (if we agree it's a bug) breaks the test suite of a bundle I maintain (see https://travis-ci.org/lexik/LexikJWTAuthenticationBundle/jobs/218275650#L262)

Solutions I can think about for now:
- ~~Skip deprecated definitions from type registering, meaning that if a service is deprecated a day, all autowired services that rely on it will suddenly be broken, also the bug would remain if the definition is not deprecated but relies on a deprecated class, not good I think~~
- Register an error handler ignoring deprecations during the type registering process (`AutowirePass#populateAvailableType()`), ~~works but makes my test suite say `THE ERROR HANDLER HAS CHANGED`. I'll push my try as a 2nd commit.~~

Thoughts?

Commits
-------

77927f4b33 [DI] Prevent AutowirePass from triggering irrelevant deprecations
This commit is contained in:
Fabien Potencier 2017-04-05 08:02:21 -07:00
commit 91b025affc
3 changed files with 49 additions and 1 deletions

View File

@ -303,9 +303,28 @@ class AutowirePass implements CompilerPassInterface
$class = $this->container->getParameterBag()->resolveValue($class);
if ($deprecated = $definition->isDeprecated()) {
$prevErrorHandler = set_error_handler(function ($level, $message, $file, $line) use (&$prevErrorHandler) {
return (E_USER_DEPRECATED === $level || !$prevErrorHandler) ? false : $prevErrorHandler($level, $message, $file, $line);
});
}
$e = null;
try {
$reflector = new \ReflectionClass($class);
} catch (\ReflectionException $e) {
} catch (\Exception $e) {
} catch (\Throwable $e) {
}
if ($deprecated) {
restore_error_handler();
}
if (null !== $e) {
if (!$e instanceof \ReflectionException) {
throw $e;
}
$reflector = false;
}

View File

@ -442,6 +442,17 @@ class AutowirePassTest extends TestCase
$this->assertTrue($container->hasDefinition('bar'));
}
public function testProcessDoesNotTriggerDeprecations()
{
$container = new ContainerBuilder();
$container->register('deprecated', 'Symfony\Component\DependencyInjection\Tests\Fixtures\DeprecatedClass')->setDeprecated(true);
$container->register('foo', __NAMESPACE__.'\Foo');
$container->register('bar', __NAMESPACE__.'\Bar')->setAutowired(true);
$pass = new AutowirePass();
$pass->process($container);
}
public function testEmptyStringIsKept()
{
$container = new ContainerBuilder();

View File

@ -0,0 +1,18 @@
<?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\Tests\Fixtures;
@trigger_error('deprecated', E_USER_DEPRECATED);
class DeprecatedClass
{
}