Skip to content

Commit d0eb698

Browse files
nicolas-grekasnicolas-grekas
committed
[PhpUnitBridge] fix reporting deprecations when they come from DebugClassLoader
1 parent 85a2fcd commit d0eb698

File tree

8 files changed

+114
-99
lines changed

8 files changed

+114
-99
lines changed

‎.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
vendor/
22
composer.lock
33
phpunit.xml
4-
Tests/DeprecationErrorHandler/fake_vendor/symfony/error-handler/DebugClassLoader.php

‎DeprecationErrorHandler.php

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -132,44 +132,48 @@ public function handleError($type, $msg, $file, $line, $context = [])
132132
return \call_user_func(self::getPhpUnitErrorHandler(), $type, $msg, $file, $line, $context);
133133
}
134134

135-
$deprecation = new Deprecation($msg, debug_backtrace(), $file);
135+
$trace = debug_backtrace();
136+
137+
if (isset($trace[1]['function'], $trace[1]['args'][0]) && ('trigger_error' === $trace[1]['function'] || 'user_error' === $trace[1]['function'])) {
138+
$msg = $trace[1]['args'][0];
139+
}
140+
141+
$deprecation = new Deprecation($msg, $trace, $file);
136142
if ($deprecation->isMuted()) {
137143
return null;
138144
}
139-
$group = 'other';
140145

141-
if ($deprecation->originatesFromAnObject()) {
142-
$class = $deprecation->originatingClass();
143-
$method = $deprecation->originatingMethod();
144-
$msg = $deprecation->getMessage();
146+
$msg = $deprecation->getMessage();
145147

146-
if (error_reporting() & $type) {
147-
$group = 'unsilenced';
148-
} elseif ($deprecation->isLegacy()) {
149-
$group = 'legacy';
150-
} else {
151-
$group = [
152-
Deprecation::TYPE_SELF => 'remaining self',
153-
Deprecation::TYPE_DIRECT => 'remaining direct',
154-
Deprecation::TYPE_INDIRECT => 'remaining indirect',
155-
Deprecation::TYPE_UNDETERMINED => 'other',
156-
][$deprecation->getType()];
157-
}
148+
if (error_reporting() & $type) {
149+
$group = 'unsilenced';
150+
} elseif ($deprecation->isLegacy()) {
151+
$group = 'legacy';
152+
} else {
153+
$group = [
154+
Deprecation::TYPE_SELF => 'remaining self',
155+
Deprecation::TYPE_DIRECT => 'remaining direct',
156+
Deprecation::TYPE_INDIRECT => 'remaining indirect',
157+
Deprecation::TYPE_UNDETERMINED => 'other',
158+
][$deprecation->getType()];
159+
}
158160

159-
if ($this->getConfiguration()->shouldDisplayStackTrace($msg)) {
160-
echo "\n".ucfirst($group).' '.$deprecation->toString();
161+
if ($this->getConfiguration()->shouldDisplayStackTrace($msg)) {
162+
echo "\n".ucfirst($group).' '.$deprecation->toString();
161163

162-
exit(1);
163-
}
164-
if ('legacy' !== $group) {
165-
$ref = &$this->deprecations[$group][$msg]['count'];
166-
++$ref;
164+
exit(1);
165+
}
166+
167+
if ('legacy' !== $group) {
168+
$ref = &$this->deprecations[$group][$msg]['count'];
169+
++$ref;
170+
171+
if ($deprecation->originatesFromAnObject()) {
172+
$class = $deprecation->originatingClass();
173+
$method = $deprecation->originatingMethod();
167174
$ref = &$this->deprecations[$group][$msg][$class.'::'.$method];
168175
++$ref;
169176
}
170-
} else {
171-
$ref = &$this->deprecations[$group][$msg]['count'];
172-
++$ref;
173177
}
174178

175179
++$this->deprecations[$group.'Count'];

‎DeprecationErrorHandler/Deprecation.php

Lines changed: 71 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -55,48 +55,64 @@ class Deprecation
5555
public function __construct($message, array $trace, $file)
5656
{
5757
$this->trace = $trace;
58-
59-
if ('trigger_error' === (isset($trace[1]['function']) ? $trace[1]['function'] : null)
60-
&& (DebugClassLoader::class === ($class = (isset($trace[2]['class']) ? $trace[2]['class'] : null)) || LegacyDebugClassLoader::class === $class)
61-
&& 'checkClass' === (isset($trace[2]['function']) ? $trace[2]['function'] : null)
62-
&& null !== ($extraFile = (isset($trace[2]['args'][1]) ? $trace[2]['args'][1] : null))
63-
&& '' !== $extraFile
64-
&& false !== $extraFile = realpath($extraFile)
65-
) {
66-
$this->getOriginalFilesStack();
67-
array_splice($this->originalFilesStack, 2, 1, $extraFile);
68-
}
69-
7058
$this->message = $message;
59+
7160
$i = \count($trace);
7261
while (1 < $i && $this->lineShouldBeSkipped($trace[--$i])) {
7362
// No-op
7463
}
64+
7565
$line = $trace[$i];
7666
$this->triggeringFile = $file;
77-
if (isset($line['object']) || isset($line['class'])) {
78-
if (isset($line['class']) && 0 === strpos($line['class'], SymfonyTestsListenerFor::class)) {
79-
set_error_handler(function () {});
80-
$parsedMsg = unserialize($this->message);
81-
restore_error_handler();
82-
$this->message = $parsedMsg['deprecation'];
83-
$this->originClass = $parsedMsg['class'];
84-
$this->originMethod = $parsedMsg['method'];
85-
if (isset($parsedMsg['files_stack'])) {
86-
$this->originalFilesStack = $parsedMsg['files_stack'];
87-
}
88-
// If the deprecation has been triggered via
89-
// \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest()
90-
// then we need to use the serialized information to determine
91-
// if the error has been triggered from vendor code.
92-
if (isset($parsedMsg['triggering_file'])) {
93-
$this->triggeringFile = $parsedMsg['triggering_file'];
67+
68+
for ($j = 1; $j < $i; ++$j) {
69+
if (!isset($trace[$j]['function'], $trace[1 + $j]['class'], $trace[1 + $j]['args'][0])) {
70+
continue;
71+
}
72+
73+
if ('trigger_error' === $trace[$j]['function'] && !isset($trace[$j]['class'])) {
74+
if (\in_array($trace[1 + $j]['class'], [DebugClassLoader::class, LegacyDebugClassLoader::class], true)) {
75+
$class = $trace[1 + $j]['args'][0];
76+
$this->triggeringFile = isset($trace[1 + $j]['args'][1]) ? realpath($trace[1 + $j]['args'][1]) : (new \ReflectionClass($class))->getFileName();
77+
$this->getOriginalFilesStack();
78+
array_splice($this->originalFilesStack, 0, $j, [$this->triggeringFile]);
9479
}
9580

81+
break;
82+
}
83+
}
84+
85+
if (isset($line['object']) || isset($line['class'])) {
86+
if (!isset($line['class'], $trace[$i - 2]['function']) || 0 !== strpos($line['class'], SymfonyTestsListenerFor::class)) {
87+
$this->originClass = isset($line['object']) ? \get_class($line['object']) : $line['class'];
88+
$this->originMethod = $line['function'];
89+
90+
return;
91+
}
92+
93+
if ('trigger_error' !== $trace[$i - 2]['function'] || isset($trace[$i - 2]['class'])) {
94+
$this->originClass = \get_class($line['args'][0]);
95+
$this->originMethod = $line['args'][0]->getName();
96+
9697
return;
9798
}
98-
$this->originClass = isset($line['object']) ? \get_class($line['object']) : $line['class'];
99-
$this->originMethod = $line['function'];
99+
100+
set_error_handler(function () {});
101+
$parsedMsg = unserialize($this->message);
102+
restore_error_handler();
103+
$this->message = $parsedMsg['deprecation'];
104+
$this->originClass = $parsedMsg['class'];
105+
$this->originMethod = $parsedMsg['method'];
106+
if (isset($parsedMsg['files_stack'])) {
107+
$this->originalFilesStack = $parsedMsg['files_stack'];
108+
}
109+
// If the deprecation has been triggered via
110+
// \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest()
111+
// then we need to use the serialized information to determine
112+
// if the error has been triggered from vendor code.
113+
if (isset($parsedMsg['triggering_file'])) {
114+
$this->triggeringFile = $parsedMsg['triggering_file'];
115+
}
100116
}
101117
}
102118

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

133-
return $this->originClass;
149+
$class = $this->originClass;
150+
151+
return false !== strpos($class, "@anonymous\0") ? (get_parent_class($class) ?: key(class_implements($class)) ?: 'class').'@anonymous' : $class;
134152
}
135153

136154
/**
@@ -158,8 +176,7 @@ public function getMessage()
158176
*/
159177
public function isLegacy()
160178
{
161-
$class = $this->originatingClass();
162-
if ((new \ReflectionClass($class))->isInternal()) {
179+
if (!$this->originClass || (new \ReflectionClass($this->originClass))->isInternal()) {
163180
return false;
164181
}
165182

@@ -168,8 +185,8 @@ public function isLegacy()
168185
return 0 === strpos($method, 'testLegacy')
169186
|| 0 === strpos($method, 'provideLegacy')
170187
|| 0 === strpos($method, 'getLegacy')
171-
|| strpos($class, '\Legacy')
172-
|| \in_array('legacy', Test::getGroups($class, $method), true);
188+
|| strpos($this->originClass, '\Legacy')
189+
|| \in_array('legacy', Test::getGroups($this->originClass, $method), true);
173190
}
174191

175192
/**
@@ -195,11 +212,10 @@ public function isMuted()
195212
*/
196213
public function getType()
197214
{
198-
$triggeringFilePathType = $this->getPathType($this->triggeringFile);
199-
if (self::PATH_TYPE_SELF === $triggeringFilePathType) {
215+
if (self::PATH_TYPE_SELF === $pathType = $this->getPathType($this->triggeringFile)) {
200216
return self::TYPE_SELF;
201217
}
202-
if (self::PATH_TYPE_UNDETERMINED === $triggeringFilePathType) {
218+
if (self::PATH_TYPE_UNDETERMINED === $pathType) {
203219
return self::TYPE_UNDETERMINED;
204220
}
205221
$erroringFile = $erroringPackage = null;
@@ -208,10 +224,10 @@ public function getType()
208224
if ('-' === $file || 'Standard input code' === $file || !realpath($file)) {
209225
continue;
210226
}
211-
if (self::PATH_TYPE_SELF === $this->getPathType($file)) {
227+
if (self::PATH_TYPE_SELF === $pathType = $this->getPathType($file)) {
212228
return self::TYPE_DIRECT;
213229
}
214-
if (self::PATH_TYPE_UNDETERMINED === $this->getPathType($file)) {
230+
if (self::PATH_TYPE_UNDETERMINED === $pathType) {
215231
return self::TYPE_UNDETERMINED;
216232
}
217233
if (null !== $erroringFile && null !== $erroringPackage) {
@@ -233,7 +249,7 @@ private function getOriginalFilesStack()
233249
if (null === $this->originalFilesStack) {
234250
$this->originalFilesStack = [];
235251
foreach ($this->trace as $frame) {
236-
if (!isset($frame['file']) || \in_array($frame['function'], ['require', 'require_once', 'include', 'include_once'], true)) {
252+
if (!isset($frame['file'], $frame['function']) || (!isset($frame['class']) && \in_array($frame['function'], ['require', 'require_once', 'include', 'include_once'], true))) {
237253
continue;
238254
}
239255

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

265-
return rtrim($vendor.'/'.strstr(substr(
266-
$relativePath,
267-
\strlen($vendor) + 1
268-
), \DIRECTORY_SEPARATOR, true), '/');
281+
return rtrim($vendor.'/'.strstr(substr($relativePath, \strlen($vendor) + 1), \DIRECTORY_SEPARATOR, true), '/');
269282
}
270283
}
271284

@@ -279,6 +292,13 @@ private static function getVendors()
279292
{
280293
if (null === self::$vendors) {
281294
self::$vendors = $paths = [];
295+
self::$vendors[] = \dirname(__DIR__).\DIRECTORY_SEPARATOR.'Legacy';
296+
if (class_exists(DebugClassLoader::class, false)) {
297+
self::$vendors[] = \dirname((new \ReflectionClass(DebugClassLoader::class))->getFileName());
298+
}
299+
if (class_exists(LegacyDebugClassLoader::class, false)) {
300+
self::$vendors[] = \dirname((new \ReflectionClass(LegacyDebugClassLoader::class))->getFileName());
301+
}
282302
foreach (get_declared_classes() as $class) {
283303
if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) {
284304
$r = new \ReflectionClass($class);
@@ -354,10 +374,9 @@ public function toString()
354374
$reflection->setAccessible(true);
355375
$reflection->setValue($exception, $this->trace);
356376

357-
return 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().':'.
358-
"\n".$this->message.
359-
"\nStack trace:".
360-
"\n".str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString()).
361-
"\n";
377+
return ($this->originatesFromAnObject() ? 'deprecation triggered by '.$this->originatingClass().'::'.$this->originatingMethod().":\n" : '')
378+
.$this->message."\n"
379+
."Stack trace:\n"
380+
.str_replace(' '.getcwd().\DIRECTORY_SEPARATOR, ' ', $exception->getTraceAsString())."\n";
362381
}
363382
}

‎Tests/DeprecationErrorHandler/DeprecationTest.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ public function testGetTypeDetectsSelf(string $expectedType, string $message, st
183183
public function providerGetTypeUsesRightTrace()
184184
{
185185
$vendorDir = self::getVendorDir();
186+
$fakeTrace = [
187+
['function' => 'trigger_error'],
188+
['class' => SymfonyTestsListenerTrait::class, 'function' => 'endTest'],
189+
['class' => SymfonyTestsListenerForV5::class, 'function' => 'endTest'],
190+
];
186191

187192
return [
188193
'no_file_in_stack' => [Deprecation::TYPE_DIRECT, '', [['function' => 'myfunc1'], ['function' => 'myfunc2']]],
@@ -205,7 +210,7 @@ public function providerGetTypeUsesRightTrace()
205210
$vendorDir.'/myfakevendor/myfakepackage1/MyFakeFile2.php',
206211
],
207212
]),
208-
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
213+
$fakeTrace,
209214
],
210215
'serialized_stack_files_from_various_packages' => [
211216
Deprecation::TYPE_INDIRECT,
@@ -218,7 +223,7 @@ public function providerGetTypeUsesRightTrace()
218223
$vendorDir.'/myfakevendor/myfakepackage2/MyFakeFile.php',
219224
],
220225
]),
221-
[['function' => 'myfunc1'], ['class' => SymfonyTestsListenerForV5::class, 'method' => 'mymethod']],
226+
$fakeTrace,
222227
],
223228
];
224229
}

‎Tests/DeprecationErrorHandler/debug_class_loader_autoload.phpt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,6 @@ EOPHP
3030
);
3131
require __DIR__.'/fake_vendor/autoload.php';
3232

33-
// We need the real DebugClassLoader FQCN but in a vendor path.
34-
if (!file_exists($errorHandlerRootDir = __DIR__.'/../../../../Component/ErrorHandler')) {
35-
if (!file_exists($errorHandlerRootDir = __DIR__.'/../../vendor/symfony/error-handler')) {
36-
die('Could not find the ErrorHandler component root directory.');
37-
}
38-
}
39-
40-
file_put_contents($fakeDebugClassLoadPath = __DIR__.'/fake_vendor/symfony/error-handler/DebugClassLoader.php', file_get_contents($errorHandlerRootDir.'/DebugClassLoader.php'));
41-
require $fakeDebugClassLoadPath;
42-
4333
\Symfony\Component\ErrorHandler\DebugClassLoader::enable();
4434
new \App\Services\BarService();
4535

‎Tests/DeprecationErrorHandler/fake_vendor/symfony/error-handler/.gitkeep

Whitespace-only changes.

‎Tests/DeprecationErrorHandler/weak_vendors_on_vendor.phpt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,11 @@ Unsilenced deprecation notices (3)
2929
1x: unsilenced bar deprecation
3030
1x in FooTestCase::testNonLegacyBar
3131

32-
Remaining direct deprecation notices (1)
32+
Remaining direct deprecation notices (2)
33+
34+
1x: root deprecation
3335

3436
1x: silenced bar deprecation
3537
1x in FooTestCase::testNonLegacyBar
3638

3739
Legacy deprecation notices (2)
38-
39-
Other deprecation notices (1)
40-
41-
1x: root deprecation

‎bin/simple-phpunit.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*/
1111

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

1515
error_reporting(-1);
1616

0 commit comments

Comments
 (0)