8000 Distinguish direct and indirect deprecations · symfony/symfony@308d9b0 · GitHub
[go: up one dir, main page]

Skip to content

Commit 308d9b0

Browse files
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 ac486a4 commit 308d9b0

File tree

8 files changed

+170
-17
lines changed

8 files changed

+170
-17
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()) {
@@ -108,8 +109,8 @@ public static function register($mode = 0)
108109
$group = 'unsilenced';
109110
} elseif ($deprecation->isLegacy($UtilPrefix)) {
110111
$group = 'legacy';
111-
} elseif ($deprecation->originatesFromVendor()) {
112-
$group = 'remaining vendor';
112+
} elseif (!$deprecation->isInternal()) {
113+
$group = $deprecation->isIndirect() ? 'remaining indirect vendor' : 'remaining direct vendor';
113114
} else {
114115
$group = 'remaining';
115116
}
@@ -166,14 +167,14 @@ public static function register($mode = 0)
166167
return $b['count'] - $a['count'];
167168
};
168169

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

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

179180
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 = ['total', /* TODO: 'indirect', 'direct', */ 'internal'];
19+
private const GROUPS = ['total', 'indirect', 'direct', 'internal'];
2020

2121
/**
2222
* @var int[]

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

Lines changed: 64 additions & 7 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
F438 // \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
return;
7373
}
@@ -91,9 +91,9 @@ public function originatesFromAnObject(): bool
9191
return isset($this->originatingClass);
9292
}
9393

94-
public function originatesFromVendor(): bool
94+
public function isInternal(): bool
9595
{
96-
return $this->originatesFromVendor;
96+
return $this->internal;
9797
}
9898

9999
public function originatingClass(): string
@@ -129,9 +129,66 @@ public function isLegacy(string $utilPrefix): bool
129129
|| \in_array('legacy', $test::getGroups($class, $method), true);
130130
}
131131

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

137194
private static function getVendors(): array

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
}
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+
}
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();
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)
0