Skip to content

Commit 586053a

Browse files
greg0iregreg0ire
committed
Distinguish direct and indirect deprecations
This will split vendor deprection into 2 groups: - the group where both the caller and the callee are inside a vendor package, named "indirect"; - the group where the caller xor the callee is inside a vendor package, named "direct".
1 parent 34d016d commit 586053a

File tree

8 files changed

+171
-18
lines changed

8 files changed

+171
-18
lines changed

‎src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,14 @@ public static function register($mode = 0)
8080
'remainingCount' => 0,
8181
'legacyCount' => 0,
8282
'otherCount' => 0,
83-
'remaining vendorCount' => 0,
83+
'remaining direct vendorCount' => 0,
84+
'remaining indirect vendorCount' => 0,
8485
'unsilenced' => array(),
8586
'remaining' => array(),
8687
'legacy' => array(),
8788
'other' => array(),
88-
'remaining vendor' => array(),
89+
'remaining direct vendor' => array(),
90+
'remaining indirect vendor' => array(),
8991
);
9092
$deprecationHandler = function ($type, $msg, $file, $line, $context = array()) use (&$deprecations, $getConfiguration, $UtilPrefix) {
9193
$configuration = $getConfiguration();
@@ -96,7 +98,6 @@ public static function register($mode = 0)
9698
}
9799

98100
$deprecation = new Deprecation($msg, debug_backtrace(), $file);
99-
$trace = $deprecation->trace();
100101
$group = 'other';
101102

102103
if ($deprecation->originatesFromAnObject()) {
@@ -107,8 +108,8 @@ public static function register($mode = 0)
107108
$group = 'unsilenced';
108109
} elseif ($deprecation->isLegacy($UtilPrefix)) {
109110
$group = 'legacy';
110-
} elseif ($deprecation->originatesFromVendor()) {
111-
$group = 'remaining vendor';
111+
} elseif (!$deprecation->isInternal()) {
112+
$group = $deprecation->isIndirect() ? 'remaining indirect vendor' : 'remaining direct vendor';
112113
} else {
113114
$group = 'remaining';
114115
}
@@ -165,14 +166,14 @@ public static function register($mode = 0)
165166
return $b['count'] - $a['count'];
166167
};
167168

168-
$groups = array('unsilenced', 'remaining', 'remaining vendor', 'legacy', 'other');
169+
$groups = array('unsilenced', 'remaining', 'remaining direct vendor', 'remaining indirect vendor', 'legacy', 'other');
169170

170171
$displayDeprecations = function ($deprecations) use ($colorize, $cmp, $groups, $configuration) {
171172
foreach ($groups as $group) {
172173
if ($deprecations[$group.'Count']) {
173174
echo "\n", $colorize(
174175
sprintf('%s deprecation notices (%d)', ucfirst($group), $deprecations[$group.'Count']),
175-
'legacy' !== $group && 'remaining vendor' !== $group
176+
'legacy' !== $group && 'remaining direct vendor' !== $group && 'remaining indirect vendor' !== $group
176177
), "\n";
177178

178179
if (!$configuration->verboseOutput()) {

‎src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Configuration.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*/
1717
class Configuration
1818
{
19-
private const GROUPS = array('total', /* TODO: 'indirect', 'direct', */ 'internal');
19+
private const GROUPS = array('total', 'indirect', 'direct', 'internal');
2020

2121
/**
2222
* @var int[]

‎src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler/Deprecation.php

Lines changed: 65 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class Deprecation
3939
/**
4040
* @var bool
4141
*/
42-
private $originatesFromVendor;
42+
private $internal;
4343

4444
/** @var string[] absolute paths to vendor directories */
4545
private static $vendors;
@@ -53,7 +53,7 @@ public function __construct(string $message, array $trace, string $file)
5353
// No-op
5454
}
5555
$line = $trace[$i];
56-
$this->originatesFromVendor = isset($line['file']) && $this->pathOriginatesFromVendor($file);
56+
$this->internal = !$this->pathOriginatesFromVendor($file);
5757
if (isset($line['object']) || isset($line['class'])) {
5858
if (isset($line['class']) && 0 === strpos(
5959
$line['class'],
@@ -67,7 +67,7 @@ public function __construct(string $message, array $trace, string $file)
6767
// \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest()
6868
// then we need to use the serialized information to determine
6969
// if the error has been triggered from vendor code.
70-
$this->originatesFromVendor = isset($parsedMsg['triggering_file'])
70+
$this->internal = isset($parsedMsg['triggering_file'])
7171
&& $this->pathOriginatesFromVendor($parsedMsg['triggering_file']);
7272

7373
return;
@@ -92,9 +92,9 @@ public function originatesFromAnObject(): bool
9292
return isset($this->originatingClass);
9393
}
9494

95-
public function originatesFromVendor(): bool
95+
public function isInternal(): bool
9696
{
97-
return $this->originatesFromVendor;
97+
return $this->internal;
9898
}
9999

100100
public function originatingClass(): string
@@ -132,9 +132,66 @@ public function isLegacy(string $utilPrefix): bool
132132
|| \in_array('legacy', $test::getGroups($class, $method), true);
133133
}
134134

135-
public function trace(): array
135+
/**
136+
* Tells whether both the calling package and the called package are vendor
137+
* packages.
138+
*/
139+
public function isIndirect(): bool
140+
{
141+
$erroringFile = $erroringPackage = null;
142+
foreach ($this->trace as $line) {
143+
if (!isset($line['file'])) {
144+
continue;
145+
}
146+
$file = $line['file'];
147+
if ('-' === $file || 'Standard input code' === $file || !realpath($file)) {
148+
continue;
149+
}
150+
if (!$this->pathOriginatesFromVendor($file)) {
151+
return false;
152+
}
153+
if (null !== $erroringFile && null !== $erroringPackage) {
154+
if ($this->getPackage($file) !== $erroringPackage) {
155+
return true;
156+
}
157+
continue;
158+
}
159+
$erroringFile = $file;
160+
$erroringPackage = $this->getPackage($file);
161+
}
162+
163+
return false;
164+
}
165+
166+
/**
167+
* inVendors() should always be called prior to calling this method.
168+
*/
169+
private function getPackage(string $path): string
136170
{
137-
return $this->trace;
171+
$path = realpath($path) ?: $path;
172+
foreach (self::getVendors() as $vendorRoot) {
173+
if (0 === strpos($path, $vendorRoot)) {
174+
$relativePath = substr($path, \strlen($vendorRoot) + 1);
175+
$vendor = strstr($relativePath, \DIRECTORY_SEPARATOR, true);
176+
if (false === $vendor) {
177+
throw new \RuntimeException(sprintf(
178+
'Could not find directory separator "%s" in path "%s"',
179+
\DIRECTORY_SEPARATOR,
180+
$relativePath
181+
));
182+
}
183+
184+
return $vendor.'/'.strstr(substr(
185+
$relativePath,
186+
\strlen($vendor) + 1
187+
), \DIRECTORY_SEPARATOR, true);
188+
}
189+
}
190+
191+
throw new \RuntimeException(sprintf(
192+
'No vendors found for path "%s"',
193+
$path
194+
));
138195
}
139196

140197
private static function getVendors(): array
@@ -144,7 +201,7 @@ private static function getVendors(): array
144201
foreach (get_declared_classes() as $class) {
145202
if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) {
146203
$r = new \ReflectionClass($class);
147-
$v = dirname(dirname($r->getFileName()));
204+
$v = \dirname(\dirname($r->getFileName()));
148205
if (file_exists($v.'/composer/installed.json')) {
149206
self::$vendors[] = $v;
150207
}

‎src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/DeprecationTest.php

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,43 @@ class DeprecationTest extends TestCase
1818
{
1919
public function testItCanDetermineTheClassWhereTheDeprecationHappened()
2020
{
21-
$deprecation = new Deprecation('💩', debug_backtrace(), __FILE__);
21+
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
2222
$this->assertTrue($deprecation->originatesFromAnObject());
23+
$this->assertSame(self::class, $deprecation->originatingClass());
24+
$this->assertSame(__FUNCTION__, $deprecation->originatingMethod());
25+
}
26+
27+
public function testItCanTellWhetherItIsInternal()
28+
{
29+
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
30+
$this->assertTrue($deprecation->isInternal());
31+
}
32+
33+
public function testLegacyTestMethodIsDetectedAsSuch()
34+
{
35+
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
36+
$this->assertTrue($deprecation->isLegacy('whatever'));
37+
}
38+
39+
public function testItCanBeConvertedToAString()
40+
{
41+
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
42+
$this->assertContains('💩', (string) $deprecation);
43+
$this->assertContains(__FUNCTION__, (string) $deprecation);
44+
}
45+
46+
public function testItRulesOutFilesOutsideVendorsAsIndirect()
47+
{
48+
$deprecation = new Deprecation('💩', $this->debugBacktrace(), __FILE__);
49+
$this->assertFalse($deprecation->isIndirect());
50+
}
51+
52+
/**
53+
* This method is here to simulate the extra level from the piece of code
54+
* triggering an error to the error handler
55+
*/
56+
public function debugBacktrace(): array
57+
{
58+
return debug_backtrace();
2359
}
2460
}

‎src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/fake_vendor/acme/lib/SomeService.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace acme\lib;
4+
5+
class SomeService
6+
{
7+
public function deprecatedApi()
8+
{
9+
@trigger_error(
10+
__FUNCTION__.' is deprecated! You should stop relying on it!',
11+
E_USER_DEPRECATED
12+
);
13+
}
14+
}

‎src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/fake_vendor/acme/outdated-lib/outdated_file.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
/* We have not caught up on the deprecations yet and still call the other lib
3+
in a deprecated way. */
4+
5+
include __DIR__.'/../lib/SomeService.php';
6+
$defraculator = new \acme\lib\SomeService();
7+
$defraculator->deprecatedApi();

‎src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/lagging_vendor.phpt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
Test DeprecationErrorHandler in weak vendors mode on vendor file
3+
--FILE--
4+
<?php
5+
6+
putenv('SYMFONY_DEPRECATIONS_HELPER=allow_outdated_vendors');
7+
putenv('ANSICON');
8+
putenv('ConEmuANSI');
9+
putenv('TERM');
10+
11+
$vendor = __DIR__;
12+
while (!file_exists($vendor.'/vendor')) {
13+
$vendor = dirname($vendor);
14+
}
15+
define('PHPUNIT_COMPOSER_INSTALL', $vendor.'/vendor/autoload.php');
16+
require PHPUNIT_COMPOSER_INSTALL;
17+
require_once __DIR__.'/../../bootstrap.php';
18+
eval(<<<'EOPHP'
19+
namespace PHPUnit\Util;
20+
21+
class Test
22+
{
23+
public static function getGroups()
24+
{
25+
return array();
26+
}
27+
}
28+
EOPHP
29+
);
30+
require __DIR__.'/fake_vendor/autoload.php';
31+
require __DIR__.'/fake_vendor/acme/outdated-lib/outdated_file.php';
32+
33+
?>
34+
--EXPECTF--
35+
Remaining indirect vendor deprecation notices (1)
36+
37+
1x: deprecatedApi is deprecated! You should stop relying on it!
38+
1x in SomeService::deprecatedApi from acme\lib

‎src/Symfony/Bridge/PhpUnit/Tests/DeprecationErrorHandler/weak_vendors_on_vendor.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Unsilenced deprecation notices (3)
2828
1x: unsilenced bar deprecation
2929
1x in FooTestCase::testNonLegacyBar
3030

31-
Remaining vendor deprecation notices (1)
31+
Remaining direct vendor deprecation notices (1)
3232

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

0 commit comments

Comments
 (0)