[PhpUnitBridge]allow outdated vendors mode #24867
Introduce weak_lagging_vendors mode
In this mode, failures coming from vendors that call other vendors in a deprecated way are not taken into account when deciding to make the build fail. They also appear in a separate group.
- Loading branch information
commit 9fdbe2bbc9f50414d2c8089f08721801215b7f06
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ class DeprecationErrorHandler | |
| { | ||
| const MODE_WEAK = 'weak'; | ||
| const MODE_WEAK_VENDORS = 'weak_vendors'; | ||
| const MODE_WEAK_LAGGING_VENDORS = 'weak_lagging_vendors'; | ||
| const MODE_DISABLED = 'disabled'; | ||
|
|
||
| private static $isRegistered = false; | ||
|
|
@@ -30,6 +31,8 @@ class DeprecationErrorHandler | |
| * The following reporting modes are supported: | ||
| * - use "weak" to hide the deprecation report but keep a global count; | ||
| * - use "weak_vendors" to act as "weak" but only for vendors; | ||
| * - use "weak_lagging_vendors" to act as "weak" but only for vendors that | ||
| * failed to keep up with their upstream dependencies deprecations; | ||
| * - use "/some-regexp/" to stop the test suite whenever a deprecation | ||
| * message matches the given regular expression; | ||
| * - use a number to define the upper bound of allowed deprecations, | ||
|
|
@@ -54,7 +57,11 @@ public static function register($mode = 0) | |
| if (false === $mode) { | ||
| $mode = getenv('SYMFONY_DEPRECATIONS_HELPER'); | ||
| } | ||
| if (!in_array($mode, array( | ||
| DeprecationErrorHandler::MODE_WEAK, | ||
| DeprecationErrorHandler::MODE_WEAK_VENDORS, | ||
| DeprecationErrorHandler::MODE_WEAK_LAGGING_VENDORS, | ||
| ), true) && (!isset($mode[0]) || '/' !== $mode[0])) { | ||
| $mode = preg_match('/^[1-9][0-9]*$/', $mode) ? (int) $mode : 0; | ||
| } | ||
|
|
||
|
|
@@ -66,11 +73,13 @@ public static function register($mode = 0) | |
| 'remainingCount' => 0, | ||
| 'legacyCount' => 0, | ||
| 'otherCount' => 0, | ||
| 'lagging vendorCount' => 0, | ||
| 'remaining vendorCount' => 0, | ||
| 'unsilenced' => array(), | ||
| 'remaining' => array(), | ||
| 'legacy' => array(), | ||
| 'other' => array(), | ||
| 'lagging vendor' => array(), | ||
| 'remaining vendor' => array(), | ||
| ); | ||
| $deprecationHandler = function ($type, $msg, $file, $line, $context = array()) use (&$deprecations, $getMode, $UtilPrefix) { | ||
|
|
@@ -84,8 +93,9 @@ public static function register($mode = 0) | |
| $trace = debug_backtrace(true); | ||
| $group = 'other'; | ||
| $isVendor = false; | ||
| $isWeak = DeprecationErrorHandler::MODE_WEAK === $mode || | ||
| (DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode && $isVendor = self::inVendors($file)) || | ||
| (DeprecationErrorHandler::MODE_WEAK_LAGGING_VENDORS === $mode && $isLaggingVendor = self::isLaggingVendor($trace)); | ||
| $i = count($trace); | ||
| while (1 < $i && (!isset($trace[--$i]['class']) || ('ReflectionMethod' === $trace[$i]['class'] || 0 === strpos($trace[$i]['class'], 'PHPUnit_') || 0 === strpos($trace[$i]['class'], 'PHPUnit\\')))) { | ||
| // No-op | ||
|
|
@@ -101,7 +111,9 @@ public static function register($mode = 0) | |
| // \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. | ||
| $isWeak = DeprecationErrorHandler::MODE_WEAK === $mode || | ||
| (DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode && $isVendor = isset($parsedMsg['triggering_file']) && self::inVendors($parsedMsg['triggering_file'])) || | ||
| (DeprecationErrorHandler::MODE_WEAK_LAGGING_VENDORS === $mode); // not enough information to make the right call, so let's be lenient | ||
| } else { | ||
| $class = isset($trace[$i]['object']) ? get_class($trace[$i]['object']) : $trace[$i]['class']; | ||
| $method = $trace[$i]['function']; | ||
|
|
@@ -118,6 +130,8 @@ public static function register($mode = 0) | |
| || in_array('legacy', $Test::getGroups($class, $method), true) | ||
| ) { | ||
| $group = 'legacy'; | ||
| } elseif (DeprecationErrorHandler::MODE_WEAK_LAGGING_VENDORS === $mode && $isLaggingVendor) { | ||
| $group = 'lagging vendor'; | ||
| } elseif (DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode && $isVendor) { | ||
| $group = 'remaining vendor'; | ||
| } else { | ||
|
|
@@ -192,13 +206,16 @@ public static function register($mode = 0) | |
| if (DeprecationErrorHandler::MODE_WEAK_VENDORS === $mode) { | ||
| $groups[] = 'remaining vendor'; | ||
| } | ||
| if (DeprecationErrorHandler::MODE_WEAK_LAGGING_VENDORS === $mode) { | ||
| $groups[] = 'lagging vendor'; | ||
| } | ||
| array_push($groups, 'legacy', 'other'); | ||
|
|
||
| foreach ($groups as $group) { | ||
| if ($deprecations[$group.'Count']) { | ||
| echo "\n", $colorize( | ||
| sprintf('%s deprecation notices (%d)', ucfirst($group), $deprecations[$group.'Count']), | ||
| !in_array($group, array('legacy', 'remaining vendor', 'lagging vendor'), true) | ||
| ), "\n"; | ||
|
|
||
| uasort($deprecations[$group], $cmp); | ||
|
|
@@ -227,11 +244,58 @@ public static function register($mode = 0) | |
| } | ||
| } | ||
|
|
||
| private static function isLaggingVendor(array $trace): bool | ||
| { | ||
| $erroringFile = $erroringPackage = null; | ||
| foreach ($trace as $line) { | ||
| if (!isset($line['file'])) { | ||
| continue; | ||
| } | ||
| $file = $line['file']; | ||
| if ('-' === $file) { | ||
| continue; | ||
| } | ||
| if (!self::inVendors($file)) { | ||
| return false; | ||
| } | ||
| if (null !== $erroringFile && null !== $erroringPackage) { | ||
| if (self::getPackage($file) !== $erroringPackage) { | ||
| return true; | ||
| } | ||
| } else { | ||
| $erroringFile = $file; | ||
| $erroringPackage = self::getPackage($file); | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * inVendors() should always be called prior to calling this method. | ||
| */ | ||
| private static function getPackage(string $path): string | ||
| { | ||
| $path = realpath($path) ?: $path; | ||
| foreach (self::getVendors() as $vendorRoot) { | ||
| if (0 === strpos($path, $vendorRoot)) { | ||
| $relativePath = substr($path, strlen($vendorRoot) + 1); | ||
| $vendor = strstr($relativePath, DIRECTORY_SEPARATOR, true); | ||
|
|
||
| return $vendor.'/'.strstr(substr($relativePath, strlen($vendor) + 1), DIRECTORY_SEPARATOR, true); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. you should throw a meaningful exception when reaching this place explaining that the provided path is not a vendor path, instead of letting PHP throw a fatal error due to There was a problem hiding this comment. @stof we're inside the error handler, that's why I refrained from doing that, I'm not sure how it will behave, but I will try. There was a problem hiding this comment. Wait, it's an error handler, so an exception should probably be fine. There was a problem hiding this comment. Reaching this place would indicate a bad usage of the private method inside the class anyway (as you are not supposed to pass a non-vendor path). So I think it would be fine (and would make it easier for contributors to see their mistakes) There was a problem hiding this comment. I tried putting an exception unconditionally on the first line, and it worked as expected, so it's fine indeed 👍 |
||
|
|
||
| throw new \RuntimeException('No vendors found'); | ||
| } | ||
|
|
||
| private static function getVendors(): array | ||
| { | ||
| /** @var string[] absolute paths to vendor directories */ | ||
| static $vendors; | ||
|
|
||
| if (null === $vendors) { | ||
| $vendors = array(); | ||
| foreach (get_declared_classes() as $class) { | ||
|
There was a problem hiding this comment. you need to initialize |
||
| if ('C' === $class[0] && 0 === strpos($class, 'ComposerAutoloaderInit')) { | ||
| $r = new \ReflectionClass($class); | ||
|
|
@@ -242,11 +306,17 @@ private static function inVendors(string $path): bool | |
| } | ||
| } | ||
| } | ||
|
|
||
| return $vendors; | ||
| } | ||
|
|
||
| private static function inVendors(string $path): bool | ||
| { | ||
| $realPath = realpath($path); | ||
| if (false === $realPath && '-' !== $path && 'Standard input code' !== $path) { | ||
| return true; | ||
| } | ||
| foreach (self::getVendors() as $vendor) { | ||
| if (0 === strpos($realPath, $vendor) && false !== strpbrk(substr($realPath, strlen($vendor), 1), '/'.DIRECTORY_SEPARATOR)) { | ||
| return true; | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <?php | ||
| /* We have not caught up on the deprecations yet and still call the other lib | ||
| in a deprecated way. */ | ||
|
|
||
| include __DIR__.'/../lib/SomeService.php'; | ||
| $defraculator = new \acme\lib\SomeService(); | ||
| $defraculator->deprecatedApi(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <?php | ||
|
|
||
| namespace acme\lib; | ||
|
|
||
| class SomeService | ||
| { | ||
| public function deprecatedApi() | ||
| { | ||
| @trigger_error( | ||
| __FUNCTION__.' is deprecated! You should stop relying on it!', | ||
| E_USER_DEPRECATED | ||
| ); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| --TEST-- | ||
| Test DeprecationErrorHandler in weak vendors mode when calling deprecated api | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| putenv('SYMFONY_DEPRECATIONS_HELPER=weak_lagging_vendors'); | ||
| putenv('ANSICON'); | ||
| putenv('ConEmuANSI'); | ||
| putenv('TERM'); | ||
|
|
||
| $vendor = __DIR__; | ||
| while (!file_exists($vendor.'/vendor')) { | ||
| $vendor = dirname($vendor); | ||
| } | ||
| define('PHPUNIT_COMPOSER_INSTALL', $vendor.'/vendor/autoload.php'); | ||
| require PHPUNIT_COMPOSER_INSTALL; | ||
| require_once __DIR__.'/../../bootstrap.php'; | ||
| eval(<<<'EOPHP' | ||
| namespace PHPUnit\Util; | ||
|
|
||
| class Test | ||
| { | ||
| public static function getGroups() | ||
| { | ||
| return array(); | ||
| } | ||
| } | ||
| EOPHP | ||
| ); | ||
| require __DIR__.'/fake_vendor/autoload.php'; | ||
| require __DIR__.'/fake_vendor/acme/lib/SomeService.php'; | ||
| $defraculator = new \Acme\Lib\SomeService(); | ||
| $defraculator->deprecatedApi(); | ||
|
|
||
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Remaining deprecation notices (1) | ||
|
|
||
| deprecatedApi is deprecated! You should stop relying on it!: 1x | ||
| 1x in SomeService::deprecatedApi from acme\lib | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| --TEST-- | ||
| Test DeprecationErrorHandler in weak vendors mode on vendor file | ||
| --FILE-- | ||
| <?php | ||
|
|
||
| putenv('SYMFONY_DEPRECATIONS_HELPER=weak_lagging_vendors'); | ||
| putenv('ANSICON'); | ||
| putenv('ConEmuANSI'); | ||
| putenv('TERM'); | ||
|
|
||
| $vendor = __DIR__; | ||
| while (!file_exists($vendor.'/vendor')) { | ||
| $vendor = dirname($vendor); | ||
| } | ||
| define('PHPUNIT_COMPOSER_INSTALL', $vendor.'/vendor/autoload.php'); | ||
| require PHPUNIT_COMPOSER_INSTALL; | ||
| require_once __DIR__.'/../../bootstrap.php'; | ||
| eval(<<<'EOPHP' | ||
| namespace PHPUnit\Util; | ||
|
|
||
| class Test | ||
| { | ||
| public static function getGroups() | ||
| { | ||
| return array(); | ||
| } | ||
| } | ||
| EOPHP | ||
| ); | ||
| require __DIR__.'/fake_vendor/autoload.php'; | ||
| require __DIR__.'/fake_vendor/acme/lagging-lib/lagging_file.php'; | ||
|
|
||
| ?> | ||
| --EXPECTF-- | ||
| Lagging vendor deprecation notices (1) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I am wrong, but shouldn't it be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both statements are true ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes 😄