diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php index a1aee18b8c2d6..5443293d38502 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php @@ -108,7 +108,19 @@ public static function collectDeprecations($outputFile) return $ErrorHandler::handleError($type, $msg, $file, $line, $context); } - $deprecations[] = [error_reporting(), $msg, $file]; + $trace = debug_backtrace(); + $filesStack = []; + foreach ($trace as $line) { + if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) { + continue; + } + + if (isset($line['file'])) { + $filesStack[] = $line['file']; + } + } + + $deprecations[] = [error_reporting(), $msg, $file, $filesStack]; }); register_shutdown_function(function () use ($outputFile, &$deprecations) { diff --git a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php index ba9e753c7b1d1..1eca72a5f5d8a 100644 --- a/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php +++ b/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php @@ -46,6 +46,8 @@ class Deprecation /** @var string[] absolute paths to vendor directories */ private static $vendors; + private $originalFilesStack; + /** * @param string $message * @param string $file @@ -66,12 +68,13 @@ public function __construct($message, array $trace, $file) $this->message = $parsedMsg['deprecation']; $this->originClass = $parsedMsg['class']; $this->originMethod = $parsedMsg['method']; + $this->originalFilesStack = $parsedMsg['files_stack']; // If the deprecation has been triggered via // \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest() // then we need to use the serialized information to determine // if the error has been triggered from vendor code. - $this->self = isset($parsedMsg['triggering_file']) - && $this->pathOriginatesFromVendor($parsedMsg['triggering_file']); + $this->self = !isset($parsedMsg['triggering_file']) + || !$this->pathOriginatesFromVendor($parsedMsg['triggering_file']); return; } @@ -159,6 +162,24 @@ public function isLegacy($utilPrefix) || \in_array('legacy', $test::getGroups($class, $method), true); } + private function getOriginalFilesStack(): array + { + if (null === $this->originalFilesStack) { + $this->originalFilesStack = []; + foreach ($this->trace as $line) { + if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) { + continue; + } + if (!isset($line['file'])) { + continue; + } + $this->originalFilesStack[] = $line['file']; + } + } + + return $this->originalFilesStack; + } + /** * Tells whether both the calling package and the called package are vendor * packages. @@ -168,14 +189,8 @@ public function isLegacy($utilPrefix) public function isIndirect() { $erroringFile = $erroringPackage = null; - foreach ($this->trace as $line) { - if (\in_array($line['function'], ['require', 'require_once', 'include', 'include_once'], true)) { - continue; - } - if (!isset($line['file'])) { - continue; - } - $file = $line['file']; + + foreach ($this->getOriginalFilesStack() as $file) { if ('-' === $file || 'Standard input code' === $file || !realpath($file)) { continue; } @@ -281,19 +296,4 @@ public function toString() "\n".str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString()). "\n"; } - - private function getPackageFromLine(array $line) - { - if (!isset($line['file'])) { - return 'internal function'; - } - if (!$this->pathOriginatesFromVendor($line['file'])) { - return 'source code'; - } - try { - return $this->getPackage($line['file']); - } catch (\RuntimeException $e) { - return 'unknown'; - } - } } diff --git a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php index cf2723796106e..521a71f0e049d 100644 --- a/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php +++ b/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php @@ -32,10 +32,10 @@ class SymfonyTestsListenerTrait private static $globallyEnabled = false; private $state = -1; private $skippedFile = false; - private $wasSkipped = array(); - private $isSkipped = array(); - private $expectedDeprecations = array(); - private $gatheredDeprecations = array(); + private $wasSkipped = []; + private $isSkipped = []; + private $expectedDeprecations = []; + private $gatheredDeprecations = []; private $previousErrorHandler; private $testsWithWarnings; private $reportUselessTests; @@ -45,7 +45,7 @@ class SymfonyTestsListenerTrait /** * @param array $mockedNamespaces List of namespaces, indexed by mocked features (time-sensitive or dns-sensitive) */ - public function __construct(array $mockedNamespaces = array()) + public function __construct(array $mockedNamespaces = []) { if (class_exists('PHPUnit_Util_Blacklist')) { \PHPUnit_Util_Blacklist::$blacklistedClassNames['\Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait'] = 2; @@ -57,7 +57,7 @@ public function __construct(array $mockedNamespaces = array()) foreach ($mockedNamespaces as $type => $namespaces) { if (!\is_array($namespaces)) { - $namespaces = array($namespaces); + $namespaces = [$namespaces]; } if ('time-sensitive' === $type) { foreach ($namespaces as $ns) { @@ -114,7 +114,7 @@ public function startTestSuite($suite) $Test = 'PHPUnit\Util\Test'; } $suiteName = $suite->getName(); - $this->testsWithWarnings = array(); + $this->testsWithWarnings = []; foreach ($suite->tests() as $test) { if (!($test instanceof \PHPUnit\Framework\TestCase || $test instanceof TestCase)) { @@ -145,11 +145,11 @@ public function startTestSuite($suite) if (!$this->wasSkipped = require $this->skippedFile) { echo "All tests already ran successfully.\n"; - $suite->setTests(array()); + $suite->setTests([]); } } } - $testSuites = array($suite); + $testSuites = [$suite]; for ($i = 0; isset($testSuites[$i]); ++$i) { foreach ($testSuites[$i]->tests() as $test) { if ($test instanceof \PHPUnit_Framework_TestSuite || $test instanceof TestSuite) { @@ -168,7 +168,7 @@ public function startTestSuite($suite) } } } elseif (2 === $this->state) { - $skipped = array(); + $skipped = []; foreach ($suite->tests() as $test) { if (!($test instanceof \PHPUnit\Framework\TestCase || $test instanceof TestCase) || isset($this->wasSkipped[$suiteName]['*']) @@ -240,7 +240,7 @@ public function startTest($test) $test->getTestResultObject()->beStrictAboutTestsThatDoNotTestAnything(false); $this->expectedDeprecations = $annotations['method']['expectedDeprecation']; - $this->previousErrorHandler = set_error_handler(array($this, 'handleError')); + $this->previousErrorHandler = set_error_handler([$this, 'handleError']); } } } @@ -281,25 +281,27 @@ public function endTest($test, $time) $deprecations = file_get_contents($this->runsInSeparateProcess); unlink($this->runsInSeparateProcess); putenv('SYMFONY_DEPRECATIONS_SERIALIZE'); - foreach ($deprecations ? unserialize($deprecations) : array() as $deprecation) { - $error = serialize(array('deprecation' => $deprecation[1], 'class' => $className, 'method' => $test->getName(false), 'triggering_file' => isset($deprecation[2]) ? $deprecation[2] : null)); - if ($deprecation[0]) { - @trigger_error($error, E_USER_DEPRECATED); - } else { - @trigger_error($error, E_USER_DEPRECATED); - } + foreach ($deprecations ? unserialize($deprecations) : [] as $deprecation) { + $error = serialize([ + 'deprecation' => $deprecation[1], + 'class' => $className, + 'method' => $test->getName(false), + 'triggering_file' => isset($deprecation[2]) ? $deprecation[2] : null, + 'files_stack' => $deprecation[3], + ]); + @trigger_error($error, E_USER_DEPRECATED); } $this->runsInSeparateProcess = false; } if ($this->expectedDeprecations) { - if (!\in_array($test->getStatus(), array($BaseTestRunner::STATUS_SKIPPED, $BaseTestRunner::STATUS_INCOMPLETE), true)) { + if (!\in_array($test->getStatus(), [$BaseTestRunner::STATUS_SKIPPED, $BaseTestRunner::STATUS_INCOMPLETE], true)) { $test->addToAssertionCount(\count($this->expectedDeprecations)); } restore_error_handler(); - if (!$errored && !\in_array($test->getStatus(), array($BaseTestRunner::STATUS_SKIPPED, $BaseTestRunner::STATUS_INCOMPLETE, $BaseTestRunner::STATUS_FAILURE, $BaseTestRunner::STATUS_ERROR), true)) { + if (!$errored && !\in_array($test->getStatus(), [$BaseTestRunner::STATUS_SKIPPED, $BaseTestRunner::STATUS_INCOMPLETE, $BaseTestRunner::STATUS_FAILURE, $BaseTestRunner::STATUS_ERROR], true)) { try { $prefix = "@expectedDeprecation:\n"; $test->assertStringMatchesFormat($prefix.'%A '.implode("\n%A ", $this->expectedDeprecations)."\n%A", $prefix.' '.implode("\n ", $this->gatheredDeprecations)."\n"); @@ -310,7 +312,7 @@ public function endTest($test, $time) } } - $this->expectedDeprecations = $this->gatheredDeprecations = array(); + $this->expectedDeprecations = $this->gatheredDeprecations = []; $this->previousErrorHandler = null; } if (!$this->runsInSeparateProcess && -2 < $this->state && ($test instanceof \PHPUnit\Framework\TestCase || $test instanceof TestCase)) { @@ -318,12 +320,12 @@ public function endTest($test, $time) ClockMock::withClockMock(false); } if (\in_array('dns-sensitive', $groups, true)) { - DnsMock::withMockedHosts(array()); + DnsMock::withMockedHosts([]); } } } - public function handleError($type, $msg, $file, $line, $context = array()) + public function handleError($type, $msg, $file, $line, $context = []) { if (E_USER_DEPRECATED !== $type && E_DEPRECATED !== $type) { $h = $this->previousErrorHandler; diff --git a/src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php b/src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php index 92bad71e08498..34bf795396af6 100644 --- a/src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php +++ b/src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php @@ -11,11 +11,31 @@ namespace Symfony\Bridge\PhpUnit\Tests\DeprecationErrorHandler; +use Composer\Autoload\ClassLoader; use PHPUnit\Framework\TestCase; use Symfony\Bridge\PhpUnit\DeprecationErrorHandler\Deprecation; +use Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerForV5; class DeprecationTest extends TestCase { + public static function setUpBeforeClass(): void + { + $vendorDir = self::getVendorDir(); + + mkdir($vendorDir.'/myfakevendor/myfakepackage1', 0777, true); + mkdir($vendorDir.'/myfakevendor/myfakepackage2'); + touch($vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php'); + touch($vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php'); + touch($vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php'); + } + + private static function getVendorDir(): string + { + $reflection = new \ReflectionClass(ClassLoader::class); + + return \dirname($reflection->getFileName(), 2); + } + public function testItCanDetermineTheClassWhereTheDeprecationHappened() { $deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__); @@ -49,12 +69,133 @@ public function testItRulesOutFilesOutsideVendorsAsIndirect() $this->assertFalse($deprecation->isIndirect()); } + public function providerIsSelf(): array + { + return [ + 'not_from_vendors_file' => [true, '', 'MyClass1', ''], + 'nonexistent_file' => [false, '', 'MyClass1', 'dummy_vendor_path'], + 'serialized_trace_without_triggering_file' => [ + true, + serialize(['class' => '', 'method' => '', 'deprecation' => '', 'files_stack' => []]), + SymfonyTestsListenerForV5::class, + '', + ], + 'serialized_trace_with_not_from_vendors_triggering_file' => [ + true, + serialize([ + 'class' => '', + 'method' => '', + 'deprecation' => '', + 'triggering_file' => '', + 'files_stack' => [], + ]), + SymfonyTestsListenerForV5::class, + '', + ], + 'serialized_trace_with_nonexistent_triggering_file' => [ + false, + serialize([ + 'class' => '', + 'method' => '', + 'deprecation' => '', + 'triggering_file' => 'dummy_vendor_path', + 'files_stack' => [], + ]), + SymfonyTestsListenerForV5::class, + '', + ], + ]; + } + + /** + * @dataProvider providerIsSelf + */ + public function testIsSelf(bool $expectedIsSelf, string $message, string $traceClass, string $file): void + { + $trace = [ + ['class' => 'MyClass1', 'function' => 'myMethod'], + ['class' => $traceClass, 'function' => 'myMethod'], + ]; + $deprecation = new Deprecation($message, $trace, $file); + $this->assertEquals($expectedIsSelf, $deprecation->isSelf()); + } + + public function providerIsIndirectUsesRightTrace(): array + { + $vendorDir = self::getVendorDir(); + + return [ + 'no_file_in_stack' => [false, '', [['function' => 'myfunc1'], ['function' => 'myfunc2']]], + 'files_in_stack_from_various_packages' => [ + true, + '', + [ + ['function' => 'myfunc1', 'file' => $vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php'], + ['function' => 'myfunc2', 'file' => $vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php'], + ], + ], + 'serialized_stack_files_from_same_package' => [ + false, + serialize([ + 'deprecation' => 'My deprecation message', + 'class' => 'MyClass', + 'method' => 'myMethod', + 'files_stack' => [ + $vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php', + $vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php', + ], + ]), + [['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']], + ], + 'serialized_stack_files_from_various_packages' => [ + true, + serialize([ + 'deprecation' => 'My deprecation message', + 'class' => 'MyClass', + 'method' => 'myMethod', + 'files_stack' => [ + $vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile1.php', + $vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php', + ], + ]), + [['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']], + ], + ]; + } + + /** + * @dataProvider providerIsIndirectUsesRightTrace + */ + public function testIsIndirectUsesRightTrace(bool $expectedIsIndirect, string $message, array $trace): void + { + $deprecation = new Deprecation($message, $trace, ''); + $this->assertEquals($expectedIsIndirect, $deprecation->isIndirect()); + } + /** * This method is here to simulate the extra level from the piece of code - * triggering an error to the error handler + * triggering an error to the error handler. */ public function debugBacktrace(): array { return debug_backtrace(); } + + private static function removeDir($dir): void + { + $files = glob($dir.'/*'); + foreach ($files as $file) { + if (is_file($file)) { + unlink($file); + } else { + self::removeDir($file); + } + } + rmdir($dir); + } + + public static function tearDownAfterClass(): void + { + self::removeDir(self::getVendorDir().'/myfakevendor'); + } }