diff --git a/composer.json b/composer.json index f4ac18e476d..a88e48e38c9 100644 --- a/composer.json +++ b/composer.json @@ -39,6 +39,7 @@ "sensiolabs/ansi-to-html": "^1.2", "silverstripe/config": "^3", "silverstripe/assets": "^3", + "silverstripe/supported-modules": "^1.1", "silverstripe/vendor-plugin": "^2", "sminnee/callbacklist": "^0.1.1", "symfony/cache": "^7.0", diff --git a/src/Control/Controller.php b/src/Control/Controller.php index 564457b0eb3..b66ae94424d 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -19,7 +19,6 @@ */ class Controller extends RequestHandler implements TemplateGlobalProvider { - /** * An array of arguments extracted from the URL. * diff --git a/src/Dev/Deprecation.php b/src/Dev/Deprecation.php index f728bef43cf..e8d1111a431 100644 --- a/src/Dev/Deprecation.php +++ b/src/Dev/Deprecation.php @@ -3,11 +3,14 @@ namespace SilverStripe\Dev; use BadMethodCallException; +use RuntimeException; use SilverStripe\Control\Director; use SilverStripe\Core\Environment; use SilverStripe\Core\Injector\InjectionCreator; use SilverStripe\Core\Injector\InjectorLoader; use SilverStripe\Core\Manifest\Module; +use SilverStripe\Core\Path; +use SilverStripe\SupportedModules\MetaData; /** * Handles raising an notice when accessing a deprecated method, class, configuration, or behaviour. @@ -77,6 +80,18 @@ class Deprecation */ private static bool $showNoReplacementNotices = false; + /** + * @internal + */ + private static bool $showNoticesCalledFromSupportedCode = false; + + /** + * Cache of supported module directories, read from silverstripe/supported-modules repositories.json + * + * @internal + */ + private static array $supportedModuleDirectories = []; + /** * Enable throwing deprecation warnings. By default, this excludes warnings for * deprecated code which is called by core Silverstripe modules. @@ -146,6 +161,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 +184,51 @@ 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; + } + return Deprecation::fileIsInSupportedModule($file); + } + + /** + * Check whether a file (path to file) is in a supported module + */ + public static function fileIsInSupportedModule(string $file): bool + { + // Cache the supported modules list + if (count(Deprecation::$supportedModuleDirectories) === 0) { + // Do not make a network request when fetching metadata which could slow down a website + // While there is a small risk of the list being out of date, there is minimal downside to this + $metaData = MetaData::getAllRepositoryMetaData(fromRemote: false); + $dirs = array_map(fn($module) => "/vendor/{$module['packagist']}/", $metaData['supportedModules']); + // This is a special case for silverstripe-framework when running in CI + // Needed because module is run in the root folder rather than in the vendor folder + $dirs[] = '/silverstripe-framework/'; + Deprecation::$supportedModuleDirectories = $dirs; + } + foreach (Deprecation::$supportedModuleDirectories as $dir) { + if (str_contains($file, $dir)) { + return true; + } + } + return false; } public static function isEnabled(): bool @@ -245,6 +309,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 +338,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 +378,10 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC $data = [ 'key' => sha1($string), 'message' => $string, + // Setting to `false` as here as any SCOPE_CONFIG notices from supported modules have + // already been filtered out if needed if they came from a supported module in + // SilverStripe\Config\Transformer\YamlTransformer::checkForDeprecatedConfig() + 'isCalledFromSupportedCode' => false, 'calledWithNoticeSuppression' => Deprecation::$insideNoticeSuppression ]; } else { @@ -322,13 +410,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 +448,23 @@ public static function notice($atVersion, $string = '', $scope = Deprecation::SC } } + /** + * Shorthand method to create a suppressed notice for something with no immediate replacement. + * If $message is empty, then a standardised message will be used + */ + public static function noticeWithNoReplacment( + string $atVersion, + string $message = '', + int $scope = Deprecation::SCOPE_METHOD + ): void { + if ($message === '') { + $message = 'Will be removed without equivalent functionality to replace it.'; + } + Deprecation::withSuppressedNotice( + fn() => Deprecation::notice($atVersion, $message, $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 f0530acbe6c..03f0154fd48 100644 --- a/tests/php/Dev/DeprecationTest.php +++ b/tests/php/Dev/DeprecationTest.php @@ -26,6 +26,8 @@ class DeprecationTest extends SapphireTest private bool $noticesWereEnabled = false; + private bool $showSupportedNoticesWasEnabled = false; + protected function setup(): void { // Use custom error handler for two reasons: @@ -34,6 +36,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')) { @@ -49,6 +52,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 @@ -58,6 +63,7 @@ protected function tearDown(): void } else { Deprecation::disable(); } + Deprecation::setShowNoticesCalledFromSupportedCode($this->showSupportedNoticesWasEnabled); restore_error_handler(); $this->oldHandler = null; parent::tearDown(); @@ -69,6 +75,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(' ', [ @@ -78,7 +96,7 @@ public function testNotice() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = $this->myDeprecatedMethod(); $this->assertSame('abc', $ret); // call outputNotices() directly because the regular shutdown function that emits @@ -86,6 +104,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(' ', [ @@ -95,7 +136,7 @@ public function testCallUserFunc() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func([$this, 'myDeprecatedMethod']); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -110,7 +151,7 @@ public function testCallUserFuncArray() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = call_user_func_array([$this, 'myDeprecatedMethod'], []); $this->assertSame('abc', $ret); Deprecation::outputNotices(); @@ -118,7 +159,7 @@ public function testCallUserFuncArray() public function testwithSuppressedNoticeDefault() { - Deprecation::enable(); + $this->enableDeprecationNotices(); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -135,7 +176,7 @@ public function testwithSuppressedNoticeTrue() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return $this->myDeprecatedMethod(); }); @@ -152,7 +193,7 @@ public function testwithSuppressedNoticeTrueCallUserFunc() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); $ret = Deprecation::withSuppressedNotice(function () { return call_user_func([$this, 'myDeprecatedMethod']); }); @@ -169,7 +210,7 @@ public function testNoticewithSuppressedNoticeTrue() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Deprecation::withSuppressedNotice(function () { Deprecation::notice('123', 'My message.'); }); @@ -185,7 +226,7 @@ public function testClasswithSuppressedNotice() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($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; @@ -202,7 +243,7 @@ public function testClassWithInjectorwithSuppressedNotice() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(true); + $this->enableDeprecationNotices(true); Injector::inst()->get(DeprecationTestObject::class); Deprecation::outputNotices(); } @@ -218,6 +259,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. @@ -232,7 +317,7 @@ public function testConfigGetFirst() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'first_config'); Deprecation::outputNotices(); } @@ -245,7 +330,7 @@ public function testConfigGetSecond() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'second_config'); Deprecation::outputNotices(); } @@ -255,7 +340,7 @@ public function testConfigGetThird() $message = 'Config SilverStripe\Dev\Tests\DeprecationTest\DeprecationTestObject.third_config is deprecated.'; $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::inst()->get(DeprecationTestObject::class, 'third_config'); Deprecation::outputNotices(); } @@ -268,7 +353,7 @@ public function testConfigSet() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->set(DeprecationTestObject::class, 'first_config', 'abc'); Deprecation::outputNotices(); } @@ -281,7 +366,7 @@ public function testConfigMerge() ]); $this->expectException(DeprecationTestException::class); $this->expectExceptionMessage($message); - Deprecation::enable(); + $this->enableDeprecationNotices(); Config::modify()->merge(DeprecationTestObject::class, 'array_config', ['abc']); Deprecation::outputNotices(); } @@ -361,7 +446,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(); } @@ -533,7 +618,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(); }