From 9cc00719c76437ed29032abcad427a55de13d685 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 16 Oct 2024 19:15:20 +1300 Subject: [PATCH] ENH Do not output core code deprecation messages by default --- src/Dev/Deprecation.php | 106 ++++++++++++++++++++++++-- tests/php/Dev/DeprecationTest.php | 119 ++++++++++++++++++++++++++---- 2 files changed, 202 insertions(+), 23 deletions(-) diff --git a/src/Dev/Deprecation.php b/src/Dev/Deprecation.php index b5e9cf935..be818363b 100644 --- a/src/Dev/Deprecation.php +++ b/src/Dev/Deprecation.php @@ -77,6 +77,11 @@ class Deprecation */ private static bool $showNoReplacementNotices = false; + /** + * @internal + */ + private static bool $showCalledFromSupportedCodeNotices = false; + /** * Enable throwing deprecation warnings. By default, this excludes warnings for * deprecated code which is called by core Silverstripe modules. @@ -146,12 +151,18 @@ class Deprecation if (!$level) { $level = 1; } + $called = Deprecation::get_called_from_trace($backtrace, $level); + return ($called['class'] ?? '') . ($called['type'] ?? '') . ($called['function'] ?? ''); + } + + private static function get_called_from_trace(array $backtrace, int $level): array + { $newLevel = $level; - // handle closures inside withSuppressedNotice() - if (Deprecation::$insideNoticeSuppression - && substr($backtrace[$newLevel]['function'], -strlen('{closure}')) === '{closure}' - ) { - $newLevel = $newLevel + 2; + if (Deprecation::$insideNoticeSuppression) { + // handle closures inside withSuppressedNotice() + if (substr($backtrace[$newLevel]['function'], -strlen('{closure}')) === '{closure}') { + $newLevel = $newLevel + 2; + } } // handle call_user_func if ($level === 4 && strpos($backtrace[2]['function'] ?? '', 'call_user_func') !== false) { @@ -163,8 +174,53 @@ class Deprecation if ($level == 4 && ($backtrace[$newLevel]['class'] ?? '') === InjectionCreator::class) { $newLevel = $newLevel + 4; } + // handle noticeWithNoReplacment() + foreach ($backtrace as $trace) { + if (($trace['class'] ?? '') === Deprecation::class + && ($trace['function'] ?? '') === 'noticeWithNoReplacment' + ) { + $newLevel = $newLevel + 1; + break; + } + } $called = $backtrace[$newLevel] ?? []; - return ($called['class'] ?? '') . ($called['type'] ?? '') . ($called['function'] ?? ''); + return $called; + } + + private static function calledFromSupportedCode(array $backtrace): bool + { + $called = Deprecation::get_called_from_trace($backtrace, 1); + $file = $called['file'] ?? ''; + if ($file) { + $isSupportedVendorFile = Deprecation::isSupportedVendorFile($file); + if (!$isSupportedVendorFile) { + return false; + } + } + return true; + } + + /** + * Whether the given file is supported code + */ + private static function isSupportedVendorFile(string $file): bool + { + // This is a special case for silverstripe-framework when running in CI + if (str_contains($file, '/silverstripe-framework/')) { + return true; + } + // Doing a fairly simple check to see if a file is in a supported vendor folder, rather than whether + // the module itself is actually supported + $vendors = implode('|', [ + 'bringyourownideas', + 'colymba', + 'cwp', + 'dnadesign', + 'silverstripe', + 'symbiote', + 'tractorcow', + ]); + return (bool) preg_match("#/vendor/($vendors)/#", $file); } public static function isEnabled(): bool @@ -245,6 +301,19 @@ class Deprecation return Deprecation::$shouldShowForCli; } + /** + * If true, deprecation warnings will be shown for deprecated code which is called by core Silverstripe modules. + */ + public static function getShowCalledFromSupportedCodeNotices(): bool + { + return Deprecation::$showCalledFromSupportedCodeNotices; + } + + public static function setShowCalledFromSupportedCodeNotices(bool $value): void + { + Deprecation::$showCalledFromSupportedCodeNotices = $value; + } + public static function outputNotices(): void { if (!Deprecation::isEnabled()) { @@ -258,9 +327,13 @@ class Deprecation $arr = array_shift(Deprecation::$userErrorMessageBuffer); $message = $arr['message']; $calledWithNoticeSuppression = $arr['calledWithNoticeSuppression']; + $calledFromSupportedCode = $arr['calledFromSupportedCode']; if ($calledWithNoticeSuppression && !Deprecation::$showNoReplacementNotices) { continue; } + if ($calledFromSupportedCode && !Deprecation::$showCalledFromSupportedCodeNotices) { + continue; + } Deprecation::$isTriggeringError = true; user_error($message, E_USER_DEPRECATED); Deprecation::$isTriggeringError = false; @@ -294,6 +367,7 @@ class Deprecation $data = [ 'key' => sha1($string), 'message' => $string, + 'calledFromSupportedCode' => false, 'calledWithNoticeSuppression' => Deprecation::$insideNoticeSuppression ]; } else { @@ -322,13 +396,13 @@ class Deprecation $level = Deprecation::$insideNoticeSuppression ? 4 : 2; $string .= " Called from " . Deprecation::get_called_method_from_trace($backtrace, $level) . '.'; - if ($caller) { $string = $caller . ' is deprecated.' . ($string ? ' ' . $string : ''); } $data = [ 'key' => sha1($string), 'message' => $string, + 'calledFromSupportedCode' => Deprecation::calledFromSupportedCode($backtrace), 'calledWithNoticeSuppression' => Deprecation::$insideNoticeSuppression ]; } @@ -360,6 +434,24 @@ class Deprecation } } + /** + * Shorthand method to create a suppressed notice for something with no immediate replacement. + * If $string is empty, then a standardised message will be used, which is: + * Will be removed without equivalent functionality to replace it. + */ + public static function noticeWithNoReplacment( + string $atVersion, + string $string = '', + int $scope = Deprecation::SCOPE_METHOD + ): void { + if ($string === '') { + $string = 'Will be removed without equivalent functionality to replace it.'; + } + Deprecation::withSuppressedNotice( + fn() => Deprecation::notice($atVersion, $string, $scope) + ); + } + private static function varAsBoolean($val): bool { if (is_string($val)) { diff --git a/tests/php/Dev/DeprecationTest.php b/tests/php/Dev/DeprecationTest.php index e4b3a97ee..fce4faadf 100644 --- a/tests/php/Dev/DeprecationTest.php +++ b/tests/php/Dev/DeprecationTest.php @@ -23,6 +23,8 @@ class DeprecationTest extends SapphireTest private bool $noticesWereEnabled = false; + private bool $showSupportedNoticesWasEnabled = false; + protected function setup(): void { // Use custom error handler for two reasons: @@ -31,6 +33,7 @@ class DeprecationTest extends SapphireTest // https://github.com/laminas/laminas-di/pull/30#issuecomment-927585210 parent::setup(); $this->noticesWereEnabled = Deprecation::isEnabled(); + $this->showSupportedNoticesWasEnabled = Deprecation::getShowCalledFromSupportedCodeNotices(); $this->oldHandler = set_error_handler(function (int $errno, string $errstr, string $errfile, int $errline) { if ($errno === E_USER_DEPRECATED) { if (str_contains($errstr, 'SilverStripe\\Dev\\Tests\\DeprecationTest')) { @@ -46,6 +49,10 @@ class DeprecationTest extends SapphireTest // Fallback to default PHP error handler return false; }); + // This is required to clear out the message + // 'SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject is deprecated. Some class message. Called from + // SilverStripe\ORM\Connect\TableBuilder->buildTables." + Deprecation::outputNotices(); } protected function tearDown(): void @@ -55,6 +62,7 @@ class DeprecationTest extends SapphireTest } else { Deprecation::disable(); } + Deprecation::setShowCalledFromSupportedCodeNotices($this->showSupportedNoticesWasEnabled); restore_error_handler(); $this->oldHandler = null; parent::tearDown(); @@ -66,6 +74,18 @@ class DeprecationTest extends SapphireTest return 'abc'; } + private function myDeprecatedMethodNoReplacement(): string + { + Deprecation::noticeWithNoReplacment('1.2.3'); + return 'abc'; + } + + private function enableDeprecationNotices(bool $showNoReplacementNotices = false): void + { + Deprecation::enable($showNoReplacementNotices); + Deprecation::setShowCalledFromSupportedCodeNotices(true); + } + public function testNotice() { $message = implode(' ', [ @@ -75,7 +95,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = $this->myDeprecatedMethod(); $this->assertSame('abc', $ret); // call outputNotices() directly because the regular shutdown function that emits @@ -83,6 +103,29 @@ class DeprecationTest extends SapphireTest Deprecation::outputNotices(); } + public function testNoticeNoReplacement() + { + $message = implode(' ', [ + 'SilverStripe\Dev\Tests\DeprecationTest->myDeprecatedMethodNoReplacement is deprecated.', + 'Will be removed without equivalent functionality to replace it.', + 'Called from SilverStripe\Dev\Tests\DeprecationTest->testNoticeNoReplacement.' + ]); + $this->expectDeprecation(); + $this->expectDeprecationMessage($message); + $this->enableDeprecationNotices(true); + $ret = $this->myDeprecatedMethodNoReplacement(); + $this->assertSame('abc', $ret); + Deprecation::outputNotices(); + } + + public function testNoticeNoReplacementNoSupressed() + { + $this->enableDeprecationNotices(); + $ret = $this->myDeprecatedMethodNoReplacement(); + $this->assertSame('abc', $ret); + Deprecation::outputNotices(); + } + public function testCallUserFunc() { $message = implode(' ', [ @@ -92,7 +135,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func([$this, 'myDeprecatedMethod']); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -107,7 +150,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func_array([$this, 'myDeprecatedMethod'], []); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -115,7 +158,7 @@ class DeprecationTest extends SapphireTest public function testwithSuppressedNoticeDefault() { - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -132,7 +175,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -149,7 +192,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return call_user_func([$this, 'myDeprecatedMethod']); }); @@ -166,7 +209,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Deprecation::withSuppressedNotice(function () { Deprecation::notice('123', 'My message.'); }); @@ -182,7 +225,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); // using this syntax because my IDE was complaining about DeprecationTestObject not existing // when trying to use `new DeprecationTestObject();` $class = DeprecationTestObject::class; @@ -199,7 +242,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Injector::inst()->get(DeprecationTestObject::class); Deprecation::outputNotices(); } @@ -217,6 +260,50 @@ class DeprecationTest extends SapphireTest Deprecation::outputNotices(); } + public function testShowCalledFromSupportedCodeNotices() + { + $this->expectNotToPerformAssertions(); + $this->enableDeprecationNotices(true); + // showCalledFromSupportedCodeNotices is set to true by default for these unit tests + // as it is testing code within vendor/silverstripe + // This test is to ensure that the method works as expected when we disable this + // and we should expect no exceptions to be thrown + // + // Note specifically NOT testing the following because it's counted as being called + // from phpunit itself, which is not considered supported code + // Deprecation::withSuppressedNotice(function () { + // Deprecation::notice('123', 'My message.'); + // }); + Deprecation::setShowCalledFromSupportedCodeNotices(false); + // notice() + $this->myDeprecatedMethod(); + // noticeNoReplacement() + $this->myDeprecatedMethodNoReplacement(); + // callUserFunc() + call_user_func([$this, 'myDeprecatedMethod']); + // callUserFuncArray() + call_user_func_array([$this, 'myDeprecatedMethod'], []); + // withSuppressedNotice() + Deprecation::withSuppressedNotice( + fn() => $this->myDeprecatedMethod() + ); + // withSuppressedNoticeTrue() + Deprecation::withSuppressedNotice(function () { + $this->myDeprecatedMethod(); + }); + // withSuppressedNoticeTrueCallUserFunc() + Deprecation::withSuppressedNotice(function () { + call_user_func([$this, 'myDeprecatedMethod']); + }); + // classWithSuppressedNotice() + $class = DeprecationTestObject::class; + new $class(); + // classWithInjectorwithSuppressedNotice() + Injector::inst()->get(DeprecationTestObject::class); + // Output notices - there should be none + Deprecation::outputNotices(); + } + // The following tests would be better to put in the silverstripe/config module, however this is not // possible to do in a clean way as the config for the DeprecationTestObject will not load if it // is inside the silverstripe/config directory, as there is no _config.php file or _config folder. @@ -231,7 +318,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'first_config'); Deprecation::outputNotices(); } @@ -244,7 +331,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'second_config'); Deprecation::outputNotices(); } @@ -254,7 +341,7 @@ class DeprecationTest extends SapphireTest $message = 'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.third_config is deprecated.'; $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'third_config'); Deprecation::outputNotices(); } @@ -267,7 +354,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->set(DeprecationTestObject::class, 'first_config', 'abc'); Deprecation::outputNotices(); } @@ -280,7 +367,7 @@ class DeprecationTest extends SapphireTest ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->merge(DeprecationTestObject::class, 'array_config', ['abc']); Deprecation::outputNotices(); } @@ -366,7 +453,7 @@ class DeprecationTest extends SapphireTest switch ($varName) { case 'SS_DEPRECATION_ENABLED': if ($configVal) { - Deprecation::enable(); + $this->enableDeprecationNotices(); } else { Deprecation::disable(); } @@ -542,7 +629,7 @@ class DeprecationTest extends SapphireTest private function setEnabledViaStatic(bool $enabled): void { if ($enabled) { - Deprecation::enable(); + $this->enableDeprecationNotices(); } else { Deprecation::disable(); }