From d953266a21139a8ba58e255e9da5d305abcb8e26 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 17 Oct 2024 11:53:35 +1300 Subject: [PATCH] ENH Do not output core code deprecation messages by default --- src/Dev/Deprecation.php | 92 ++++++++++++++++++++++- tests/php/Dev/DeprecationTest.php | 117 ++++++++++++++++++++++++++---- 2 files changed, 191 insertions(+), 18 deletions(-) diff --git a/src/Dev/Deprecation.php b/src/Dev/Deprecation.php index b5e9cf935b2..feb90ec337f 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 $showNoticesCalledFromSupportedCode = false; + /** * Enable throwing deprecation warnings. By default, this excludes warnings for * deprecated code which is called by core Silverstripe modules. @@ -146,6 +151,12 @@ protected static function get_called_method_from_trace($backtrace, $level = 1) 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 @@ -163,8 +174,47 @@ protected static function get_called_method_from_trace($backtrace, $level = 1) 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 isCalledFromSupportedCode(array $backtrace): bool + { + $called = Deprecation::get_called_from_trace($backtrace, 1); + $file = $called['file'] ?? ''; + if (!$file) { + return false; + } + // This is a special case for silverstripe-framework when running in CI + // Needed because the silverstripe-module is run in the root folder + // rather than in the vendor folder + 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', + ]); + if (preg_match("#/vendor/($vendors)/#", $file)) { + return true; + } + return false; } public static function isEnabled(): bool @@ -245,6 +295,22 @@ public static function shouldShowForCli(): bool return Deprecation::$shouldShowForCli; } + /** + * If true, deprecation warnings will be shown for deprecated code which is called by core Silverstripe modules. + */ + public static function getShowNoticesCalledFromSupportedCode(): bool + { + return Deprecation::$showNoticesCalledFromSupportedCode; + } + + /** + * Set whether deprecation warnings will be shown for deprecated code which is called by core Silverstripe modules. + */ + public static function setShowNoticesCalledFromSupportedCode(bool $value): void + { + Deprecation::$showNoticesCalledFromSupportedCode = $value; + } + public static function outputNotices(): void { if (!Deprecation::isEnabled()) { @@ -258,9 +324,13 @@ public static function outputNotices(): void $arr = array_shift(Deprecation::$userErrorMessageBuffer); $message = $arr['message']; $calledWithNoticeSuppression = $arr['calledWithNoticeSuppression']; + $isCalledFromSupportedCode = $arr['isCalledFromSupportedCode']; if ($calledWithNoticeSuppression && !Deprecation::$showNoReplacementNotices) { continue; } + if ($isCalledFromSupportedCode && !Deprecation::$showNoticesCalledFromSupportedCode) { + continue; + } Deprecation::$isTriggeringError = true; user_error($message, E_USER_DEPRECATED); Deprecation::$isTriggeringError = false; @@ -294,6 +364,7 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC $data = [ 'key' => sha1($string), 'message' => $string, + 'isCalledFromSupportedCode' => false, 'calledWithNoticeSuppression' => Deprecation::$insideNoticeSuppression ]; } else { @@ -322,13 +393,13 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC $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, + 'isCalledFromSupportedCode' => Deprecation::isCalledFromSupportedCode($backtrace), 'calledWithNoticeSuppression' => Deprecation::$insideNoticeSuppression ]; } @@ -360,6 +431,23 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC } } + /** + * Shorthand method to create a suppressed notice for something with no immediate replacement. + * If $string is empty, then a standardised message will be used + */ + 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 e4b3a97eec4..9642356d605 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 @@ protected function setup(): void // https://github.com/laminas/laminas-di/pull/30#issuecomment-927585210 parent::setup(); $this->noticesWereEnabled = Deprecation::isEnabled(); + $this->showSupportedNoticesWasEnabled = Deprecation::getShowNoticesCalledFromSupportedCode(); $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,8 @@ protected function setup(): void // Fallback to default PHP error handler return false; }); + // This is required to clear out the notice from instantiating DeprecationTestObject in TableBuilder::buildTables(). + Deprecation::outputNotices(); } protected function tearDown(): void @@ -55,6 +60,7 @@ protected function tearDown(): void } else { Deprecation::disable(); } + Deprecation::setShowNoticesCalledFromSupportedCode($this->showSupportedNoticesWasEnabled); restore_error_handler(); $this->oldHandler = null; parent::tearDown(); @@ -66,6 +72,18 @@ private function myDeprecatedMethod(): string return 'abc'; } + private function myDeprecatedMethodNoReplacement(): string + { + Deprecation::noticeWithNoReplacment('1.2.3'); + return 'abc'; + } + + private function enableDeprecationNotices(bool $showNoReplacementNotices = false): void + { + Deprecation::enable($showNoReplacementNotices); + Deprecation::setShowNoticesCalledFromSupportedCode(true); + } + public function testNotice() { $message = implode(' ', [ @@ -75,7 +93,7 @@ public function testNotice() ]); $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 +101,29 @@ public function testNotice() 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 +133,7 @@ public function testCallUserFunc() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func([$this, 'myDeprecatedMethod']); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -107,7 +148,7 @@ public function testCallUserFuncArray() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func_array([$this, 'myDeprecatedMethod'], []); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -115,7 +156,7 @@ public function testCallUserFuncArray() public function testwithSuppressedNoticeDefault() { - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -132,7 +173,7 @@ public function testwithSuppressedNoticeTrue() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -149,7 +190,7 @@ public function testwithSuppressedNoticeTrueCallUserFunc() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return call_user_func([$this, 'myDeprecatedMethod']); }); @@ -166,7 +207,7 @@ public function testNoticewithSuppressedNoticeTrue() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Deprecation::withSuppressedNotice(function () { Deprecation::notice('123', 'My message.'); }); @@ -182,7 +223,7 @@ public function testClasswithSuppressedNotice() ]); $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 +240,7 @@ public function testClassWithInjectorwithSuppressedNotice() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Injector::inst()->get(DeprecationTestObject::class); Deprecation::outputNotices(); } @@ -217,6 +258,50 @@ public function testDisabled() Deprecation::outputNotices(); } + public function testshowNoticesCalledFromSupportedCode() + { + $this->expectNotToPerformAssertions(); + $this->enableDeprecationNotices(true); + // showNoticesCalledFromSupportedCode 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::setShowNoticesCalledFromSupportedCode(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 +316,7 @@ public function testConfigGetFirst() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'first_config'); Deprecation::outputNotices(); } @@ -244,7 +329,7 @@ public function testConfigGetSecond() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'second_config'); Deprecation::outputNotices(); } @@ -254,7 +339,7 @@ public function testConfigGetThird() $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 +352,7 @@ public function testConfigSet() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->set(DeprecationTestObject::class, 'first_config', 'abc'); Deprecation::outputNotices(); } @@ -280,7 +365,7 @@ public function testConfigMerge() ]); $this->expectDeprecation(); $this->expectDeprecationMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->merge(DeprecationTestObject::class, 'array_config', ['abc']); Deprecation::outputNotices(); } @@ -366,7 +451,7 @@ private function runConfigVsEnvTest(string $varName, $envVal, bool $configVal, b switch ($varName) { case 'SS_DEPRECATION_ENABLED': if ($configVal) { - Deprecation::enable(); + $this->enableDeprecationNotices(); } else { Deprecation::disable(); } @@ -542,7 +627,7 @@ public function testIsEnabled(string $envMode, ?bool $envEnabled, bool $staticEn private function setEnabledViaStatic(bool $enabled): void { if ($enabled) { - Deprecation::enable(); + $this->enableDeprecationNotices(); } else { Deprecation::disable(); }