Skip to content

[PhpUnitBridge] fix reporting deprecations when they come from DebugClassLoader #40067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/Symfony/Bridge/PhpUnit/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
vendor/
composer.lock
phpunit.xml
Tests/DeprecationErrorHandler/fake_vendor/symfony/error-handler/DebugClassLoader.php
60 changes: 32 additions & 28 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,44 +132,48 @@ public function handleError($type, $msg, $file, $line, $context = [])
return \call_user_func(self::getPhpUnitErrorHandler(), $type, $msg, $file, $line, $context);
}

$deprecation = new Deprecation($msg, debug_backtrace(), $file);
$trace = debug_backtrace();

if (isset($trace[1]['function'], $trace[1]['args'][0]) && ('trigger_error' === $trace[1]['function'] || 'user_error' === $trace[1]['function'])) {
$msg = $trace[1]['args'][0];
}

$deprecation = new Deprecation($msg, $trace, $file);
if ($deprecation->isMuted()) {
return null;
}
$group = 'other';

if ($deprecation->originatesFromAnObject()) {
$class = $deprecation->originatingClass();
$method = $deprecation->originatingMethod();
$msg = $deprecation->getMessage();
$msg = $deprecation->getMessage();

if (error_reporting() & $type) {
$group = 'unsilenced';
} elseif ($deprecation->isLegacy()) {
$group = 'legacy';
} else {
$group = [
Deprecation::TYPE_SELF => 'remaining self',
Deprecation::TYPE_DIRECT => 'remaining direct',
Deprecation::TYPE_INDIRECT => 'remaining indirect',
Deprecation::TYPE_UNDETERMINED => 'other',
][$deprecation->getType()];
}
if (error_reporting() & $type) {
$group = 'unsilenced';
} elseif ($deprecation->isLegacy()) {
$group = 'legacy';
} else {
$group = [
Deprecation::TYPE_SELF => 'remaining self',
Deprecation::TYPE_DIRECT => 'remaining direct',
Deprecation::TYPE_INDIRECT => 'remaining indirect',
Deprecation::TYPE_UNDETERMINED => 'other',
][$deprecation->getType()];
}

if ($this->getConfiguration()->shouldDisplayStackTrace($msg)) {
echo "\n".ucfirst($group).' '.$deprecation->toString();
if ($this->getConfiguration()->shouldDisplayStackTrace($msg)) {
echo "\n".ucfirst($group).' '.$deprecation->toString();

exit(1);
}
if ('legacy' !== $group) {
$ref = &$this->deprecations[$group][$msg]['count'];
++$ref;
exit(1);
}

if ('legacy' !== $group) {
$ref = &$this->deprecations[$group][$msg]['count'];
++$ref;

if ($deprecation->originatesFromAnObject()) {
$class = $deprecation->originatingClass();
$method = $deprecation->originatingMethod();
$ref = &$this->deprecations[$group][$msg][$class.'::'.$method];
++$ref;
}
} else {
$ref = &$this->deprecations[$group][$msg]['count'];
++$ref;
}

++$this->deprecations[$group.'Count'];
Expand Down
123 changes: 71 additions & 52 deletions src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,48 +55,64 @@ class Deprecation
public function __construct($message, array $trace, $file)
{
$this->trace = $trace;

if ('trigger_error' === (isset($trace[1]['function']) ? $trace[1]['function'] : null)
&& (DebugClassLoader::class === ($class = (isset($trace[2]['class']) ? $trace[2]['class'] : null)) || LegacyDebugClassLoader::class === $class)
&& 'checkClass' === (isset($trace[2]['function']) ? $trace[2]['function'] : null)
&& null !== ($extraFile = (isset($trace[2]['args'][1]) ? $trace[2]['args'][1] : null))
&& '' !== $extraFile
&& false !== $extraFile = realpath($extraFile)
) {
$this->getOriginalFilesStack();
array_splice($this->originalFilesStack, 2, 1, $extraFile);
}

$this->message = $message;

$i = \count($trace);
while (1 < $i && $this->lineShouldBeSkipped($trace[--$i])) {
// No-op
}

$line = $trace[$i];
$this->triggeringFile = $file;
if (isset($line['object']) || isset($line['class'])) {
if (isset($line['class']) && 0 === strpos($line['class'], SymfonyTestsListenerFor::class)) {
set_error_handler(function () {});
$parsedMsg = unserialize($this->message);
restore_error_handler();
$this->message = $parsedMsg['deprecation'];
$this->originClass = $parsedMsg['class'];
$this->originMethod = $parsedMsg['method'];
if (isset($parsedMsg['files_stack'])) {
$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.
if (isset($parsedMsg['triggering_file'])) {
$this->triggeringFile = $parsedMsg['triggering_file'];

for ($j = 1; $j < $i; ++$j) {
if (!isset($trace[$j]['function'], $trace[1 + $j]['class'], $trace[1 + $j]['args'][0])) {
continue;
}

if ('trigger_error' === $trace[$j]['function'] && !isset($trace[$j]['class'])) {
if (\in_array($trace[1 + $j]['class'], [DebugClassLoader::class, LegacyDebugClassLoader::class], true)) {
$class = $trace[1 + $j]['args'][0];
$this->triggeringFile = isset($trace[1 + $j]['args'][1]) ? realpath($trace[1 + $j]['args'][1]) : (new \ReflectionClass($class))->getFileName();
$this->getOriginalFilesStack();
array_splice($this->originalFilesStack, 0, $j, [$this->triggeringFile]);
}

break;
}
}

if (isset($line['object']) || isset($line['class'])) {
if (!isset($line['class'], $trace[$i - 2]['function']) || 0 !== strpos($line['class'], SymfonyTestsListenerFor::class)) {
$this->originClass = isset($line['object']) ? \get_class($line['object']) : $line['class'];
$this->originMethod = $line['function'];

return;
}

if ('trigger_error' !== $trace[$i - 2]['function'] || isset($trace[$i - 2]['class'])) {
$this->originClass = \get_class($line['args'][0]);
$this->originMethod = $line['args'][0]->getName();

return;
}
$this->originClass = isset($line['object']) ? \get_class($line['object']) : $line['class'];
$this->originMethod = $line['function'];

set_error_handler(function () {});
$parsedMsg = unserialize($this->message);
restore_error_handler();
$this->message = $parsedMsg['deprecation'];
$this->originClass = $parsedMsg['class'];
$this->originMethod = $parsedMsg['method'];
if (isset($parsedMsg['files_stack'])) {
$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.
if (isset($parsedMsg['triggering_file'])) {
$this->triggeringFile = $parsedMsg['triggering_file'];
}
}
}

Expand Down Expand Up @@ -130,7 +146,9 @@ public function originatingClass()
throw new \LogicException('Check with originatesFromAnObject() before calling this method.');
}

return $this->originClass;
$class = $this->originClass;

return false !== strpos($class, "@anonymous\0") ? (get_parent_class($class) ?: key(class_implements($class)) ?: 'class').'@anonymous' : $class;
}

/**
Expand Down Expand Up @@ -158,8 +176,7 @@ public function getMessage()
*/
public function isLegacy()
{
$class = $this->originatingClass();
if ((new \ReflectionClass($class))->isInternal()) {
if (!$this->originClass || (new \ReflectionClass($this->originClass))->isInternal()) {
return false;
}

Expand All @@ -168,8 +185,8 @@ public function isLegacy()
return 0 === strpos($method, 'testLegacy')
|| 0 === strpos($method, 'provideLegacy')
|| 0 === strpos($method, 'getLegacy')
|| strpos($class, '\Legacy')
|| \in_array('legacy', Test::getGroups($class, $method), true);
|| strpos($this->originClass, '\Legacy')
|| \in_array('legacy', Test::getGroups($this->originClass, $method), true);
}

/**
Expand All @@ -195,11 +212,10 @@ public function isMuted()
*/
public function getType()
{
$triggeringFilePathType = $this->getPathType($this->triggeringFile);
if (self::PATH_TYPE_SELF === $triggeringFilePathType) {
if (self::PATH_TYPE_SELF === $pathType = $this->getPathType($this->triggeringFile)) {
return self::TYPE_SELF;
}
if (self::PATH_TYPE_UNDETERMINED === $triggeringFilePathType) {
if (self::PATH_TYPE_UNDETERMINED === $pathType) {
return self::TYPE_UNDETERMINED;
}
$erroringFile = $erroringPackage = null;
Expand All @@ -208,10 +224,10 @@ public function getType()
if ('-' === $file || 'Standard input code' === $file || !realpath($file)) {
continue;
}
if (self::PATH_TYPE_SELF === $this->getPathType($file)) {
if (self::PATH_TYPE_SELF === $pathType = $this->getPathType($file)) {
return self::TYPE_DIRECT;
}
if (self::PATH_TYPE_UNDETERMINED === $this->getPathType($file)) {
if (self::PATH_TYPE_UNDETERMINED === $pathType) {
return self::TYPE_UNDETERMINED;
}
if (null !== $erroringFile && null !== $erroringPackage) {
Expand All @@ -233,7 +249,7 @@ private function getOriginalFilesStack()
if (null === $this->originalFilesStack) {
$this->originalFilesStack = [];
foreach ($this->trace as $frame) {
if (!isset($frame['file']) || \in_array($frame['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
if (!isset($frame['file'], $frame['function']) || (!isset($frame['class']) && \in_array($frame['function'], ['require', 'require_once', 'include', 'include_once'], true))) {
continue;
}

Expand All @@ -259,13 +275,10 @@ private function getPackage($path)
$relativePath = substr($path, \strlen($vendorRoot) + 1);
$vendor = strstr($relativePath, \DIRECTORY_SEPARATOR, true);
if (false === $vendor) {
throw new \RuntimeException(sprintf('Could not find directory separator "%s" in path "%s".', \DIRECTORY_SEPARATOR, $relativePath));
return 'symfony';
}

return rtrim($vendor.'/'.strstr(substr(
$relativePath,
\strlen($vendor) + 1
), \DIRECTORY_SEPARATOR, true), '/');
return rtrim($vendor.'/'.strstr(substr($relativePath, \strlen($vendor) + 1), \DIRECTORY_SEPARATOR, true), '/');
}
}

Expand All @@ -279,6 +292,13 @@ private static function getVendors()
{
if (null === self::$vendors) {
self::$vendors = $paths = [];
self::$vendors[] = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Legacy';
if (class_exists(DebugClassLoader::class, false)) {
self::$vendors[] = \dirname((new \ReflectionClass(DebugClassLoader::class))->getFileName());
}
if (class_exists(LegacyDebugClassLoader::class, false)) {
self::$vendors[] = \dirname((new \ReflectionClass(LegacyDebugClassLoader::class))->getFileName());
}
foreach (get_declared_classes() as $class) {
if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) {
$r = new \ReflectionClass($class);
Expand Down Expand Up @@ -354,10 +374,9 @@ public function toString()
$reflection->setAccessible(true);
$reflection->setValue($exception, $this->trace);

return 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().':'.
"\n".$this->message.
"\nStack trace:".
"\n".str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString()).
"\n";
return ($this->originatesFromAnObject() ? 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().":\n" : '')
.$this->message."\n"
."Stack trace:\n"
.str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString())."\n";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ public function testGetTypeDetectsSelf(string $expectedType, string $message, st
public function providerGetTypeUsesRightTrace()
{
$vendorDir = self::getVendorDir();
$fakeTrace = [
['function' => 'trigger_error'],
['class' => SymfonyTestsListenerTrait::class, 'function' => 'endTest'],
['class' => SymfonyTestsListenerForV5::class, 'function' => 'endTest'],
];

return [
'no_file_in_stack' => [Deprecation::TYPE_DIRECT, '', [['function' => 'myfunc1'], ['function' => 'myfunc2']]],
Expand All @@ -205,7 +210,7 @@ public function providerGetTypeUsesRightTrace()
$vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php',
],
]),
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
$fakeTrace,
],
'serialized_stack_files_from_various_packages' => [
Deprecation::TYPE_INDIRECT,
Expand All @@ -218,7 +223,7 @@ public function providerGetTypeUsesRightTrace()
$vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php',
],
]),
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
$fakeTrace,
],
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,6 @@ EOPHP
);
require __DIR__.'/fake_vendor/autoload.php';

// We need the real DebugClassLoader FQCN but in a vendor path.
if (!file_exists($errorHandlerRootDir = __DIR__.'/../../../../Component/ErrorHandler')) {
if (!file_exists($errorHandlerRootDir = __DIR__.'/../../vendor/symfony/error-handler')) {
die('Could not find the ErrorHandler component root directory.');
}
}

file_put_contents($fakeDebugClassLoadPath = __DIR__.'/fake_vendor/symfony/error-handler/DebugClassLoader.php', file_get_contents($errorHandlerRootDir.'/DebugClassLoader.php'));
require $fakeDebugClassLoadPath;

\Symfony\Component\ErrorHandler\DebugClassLoader::enable();
new \App\Services\BarService();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,11 @@ Unsilenced deprecation notices (3)
1x: unsilenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Remaining direct deprecation notices (1)
Remaining direct deprecation notices (2)

1x: root deprecation

1x: silenced bar deprecation
1x in FooTestCase::testNonLegacyBar

Legacy deprecation notices (2)

Other deprecation notices (1)

1x: root deprecation
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/PhpUnit/bin/simple-phpunit.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/

// Please update when phpunit needs to be reinstalled with fresh deps:
// Cache-Id: 2020-01-31 10:00 UTC
// Cache-Id: 2021-02-04 11:00 UTC

error_reporting(-1);

Expand Down