8000 bug #22282 [DI] Prevent AutowirePass from triggering irrelevant depre… · symfony/symfony@91b025a · GitHub
[go: up one dir, main page]
Skip to content

Commit 91b025a

Browse files
fabpotfabpot
committed
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 ------- 77927f4 [DI] Prevent AutowirePass from triggering irrelevant deprecations
2 parents f92ada8 + 77927f4 commit 91b025a

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,28 @@ private function getReflectionClass($id, Definition $definition)
303303

304304
$class = $this->container->getParameterBag()->resolveValue($class);
305305

306+
if ($deprecated = $definition->isDeprecated()) {
307+
$prevErrorHandler = set_error_handler(function ($level, $message, $file, $line) use (&$prevErrorHandler) {
308+
return (E_USER_DEPRECATED === $level || !$prevErrorHandler) ? false : $prevErrorHandler($level, $message, $file, $line);
309+
});
310+
}
311+
312+
$e = null;
313+
306314
try {
307315
$reflector = new \ReflectionClass($class);
308-
} catch (\ReflectionException $e) {
316+
} catch (\Exception $e) {
317+
} catch (\Throwable $e) {
318+
}
319+
320+
if ($deprecated) {
321+
restore_error_handler();
322+
}
323+
324+
if (null !== $e) {
325+
if (!$e instanceof \ReflectionException) {
326+
throw $e;
327+
}
309328
$reflector = false;
310329
}
311330

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,17 @@ public function testIgnoreServiceWithClassNotExisting()
442442
$this->assertTrue($container->hasDefinition('bar'));
443443
}
444444

445+
public function testProcessDoesNotTriggerDeprecations()
446+
{
447+
$container = new ContainerBuilder();
448+
$container->register('deprecated', 'Symfony\Component\DependencyInjection\Tests\Fixtures\DeprecatedClass')->setDeprecated(true);
449+
$container->register('foo', __NAMESPACE__.'\Foo');
450+
$container->register('bar', __NAMESPACE__.'\Bar')->setAutowired(true);
451+
452+
$pass = new AutowirePass();
453+
$pass->process($container);
454+
}
455+
445456
public function testEmptyStringIsKept()
446457
{
447458
$container = new ContainerBuilder();

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/DeprecatedClass.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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\Tests\Fixtures;
13+
14+
@trigger_error('deprecated', E_USER_DEPRECATED);
15+
16+
class DeprecatedClass
17+
{
18+
}

0 commit comments

Comments
 (0)
0