diff --git a/lib/classes/deprecated.php b/lib/classes/deprecated.php index deb278abe94..452f7844f2e 100644 --- a/lib/classes/deprecated.php +++ b/lib/classes/deprecated.php @@ -34,20 +34,25 @@ class deprecated { * * Note: The mere presence of the attribute does not do anything. It must be checked by some part of the code. * - * @param mixed $descriptor A brief descriptor of the thing that was deprecated. + * @param null|string $replacement Any replacement for the deprecated thing * @param null|string $since When it was deprecated * @param null|string $reason Why it was deprecated - * @param null|string $replacement Any replacement for the deprecated thing * @param null|string $mdl Link to the Moodle Tracker issue for more information + * @param bool $final Whether this is a final deprecation + * @param bool $emit Whether to emit a deprecation warning */ public function __construct( - public readonly mixed $descriptor, + public readonly ?string $replacement, public readonly ?string $since = null, public readonly ?string $reason = null, - public readonly ?string $replacement = null, public readonly ?string $mdl = null, public readonly bool $final = false, public readonly bool $emit = true, ) { + if ($replacement === null && $reason === null && $mdl === null) { + throw new \coding_exception( + 'A deprecated item which is not deprecated must provide a reason, or an issue number.', + ); + } } } diff --git a/lib/classes/deprecated_with_reference.php b/lib/classes/deprecated_with_reference.php new file mode 100644 index 00000000000..82b1621d7dc --- /dev/null +++ b/lib/classes/deprecated_with_reference.php @@ -0,0 +1,58 @@ +. + +namespace core; + +/** + * Attribute to describe a deprecated item. + * + * @package core + * @copyright 2023 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class deprecated_with_reference extends deprecated { + /** + * A deprecated item which also includes a reference to the owning feature. + * + * This attribute is not expected to be used more generally. It is an internal feature. + * + * @param string $owner The code which owns the usage + * @param null|string $replacement Any replacement for the deprecated thing + * @param null|string $since When it was deprecated + * @param null|string $reason Why it was deprecated + * @param null|string $mdl Link to the Moodle Tracker issue for more information + * @param bool $final Whether this is a final deprecation + * @param bool $emit Whether to emit a deprecation warning + */ + public function __construct( + public readonly string $owner, + ?string $replacement, + ?string $since, + ?string $reason, + ?string $mdl, + bool $final, + bool $emit, + ) { + parent::__construct( + replacement: $replacement, + since: $since, + reason: $reason, + mdl: $mdl, + final: $final, + emit: $emit, + ); + } +} diff --git a/lib/classes/deprecation.php b/lib/classes/deprecation.php index b71fd502d9b..eb8fa7e416f 100644 --- a/lib/classes/deprecation.php +++ b/lib/classes/deprecation.php @@ -49,10 +49,7 @@ class deprecation { if (function_exists($reference)) { // The reference looks to be a global function. - $ref = new \ReflectionFunction($reference); - if ($attributes = $ref->getAttributes(deprecated::class)) { - return $attributes[0]->newInstance(); - } + return self::get_attribute(new \ReflectionFunction($reference), $reference); } return null; @@ -85,6 +82,32 @@ class deprecation { } } + /** + * Get a deprecation attribute from a reflector. + * + * @param \Reflector $ref The reflector + * @param string $owner A descriptor of the owner of the thing that is deprecated + * @return null|deprecated_with_reference + */ + protected static function get_attribute( + \Reflector $ref, + string $owner, + ): ?deprecated_with_reference { + if ($attributes = $ref->getAttributes(deprecated::class)) { + $attribute = $attributes[0]->newInstance(); + return new deprecated_with_reference( + owner: $owner, + replacement: $attribute->replacement, + since: $attribute->since, + reason: $attribute->reason, + mdl: $attribute->mdl, + final: $attribute->final, + emit: $attribute->emit, + ); + } + return null; + } + /** * Check if a reference is deprecated. * @@ -107,47 +130,44 @@ class deprecation { } /** - * Fetch a deprecation attribute from a reflected object. + * Fetch a referenced deprecation attribute from a reflected object. * * @param \ReflectionClass $rc The reflected object * @param null|string $name The name of the thing to check for deprecation - * @return null|deprecated + * @return null|deprecated_with_reference */ protected static function from_reflected_object( \ReflectionClass $rc, ?string $name, - ): ?deprecated { + ): ?deprecated_with_reference { if ($name === null) { // No name specified. This may be a deprecated class. - if ($attributes = $rc->getAttributes(deprecated::class)) { - return $attributes[0]->newInstance(); - } - return null; + return self::get_attribute($rc, $rc->name); } if ($rc->hasConstant($name)) { // This class has a constant with the specified name. // Note: This also applies to enums. - $ref = $rc->getReflectionConstant($name); - if ($attributes = $ref->getAttributes(deprecated::class)) { - return $attributes[0]->newInstance(); - } + return self::get_attribute( + $rc->getReflectionConstant($name), + "{$rc->name}::{$name}", + ); } if ($rc->hasMethod($name)) { // This class has a method with the specified name. - $ref = $rc->getMethod($name); - if ($attributes = $ref->getAttributes(deprecated::class)) { - return $attributes[0]->newInstance(); - } + return self::get_attribute( + $rc->getMethod($name), + "{$rc->name}::{$name}", + ); } if ($rc->hasProperty($name)) { // This class has a property with the specified name. - $ref = $rc->getProperty($name); - if ($attributes = $ref->getAttributes(deprecated::class)) { - return $attributes[0]->newInstance(); - } + return self::get_attribute( + $rc->getProperty($name), + "{$rc->name}::{$name}", + ); } return null; @@ -157,10 +177,19 @@ class deprecation { * Get a string describing the deprecation. * * @param deprecated $attribute + * @param string $owner * @return string */ - public static function get_deprecation_string(deprecated $attribute): string { - $output = "Deprecation: {$attribute->descriptor} has been deprecated"; + public static function get_deprecation_string( + deprecated $attribute, + ): string { + $output = "Deprecation:"; + + if ($attribute instanceof deprecated_with_reference) { + $output .= " {$attribute->owner}"; + } + $output .= " has been deprecated"; + if ($attribute->since) { $output .= " since {$attribute->since}"; } @@ -187,7 +216,9 @@ class deprecation { * * @param deprecated $attribute */ - public static function emit_deprecation_notice(deprecated $attribute): void { + protected static function emit_deprecation_notice( + deprecated $attribute, + ): void { if (!$attribute->emit) { return; } diff --git a/lib/classes/param.php b/lib/classes/param.php index 359ffc37b29..e6cba152b07 100644 --- a/lib/classes/param.php +++ b/lib/classes/param.php @@ -222,9 +222,9 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::CLEAN', + replacement: 'a more specific type of parameter', since: '2.0', - reason: 'Please use a more specific type of parameter', + reason: 'The CLEAN param type is too generic to perform satisfactory validation', emit: false, )] case CLEAN = 'clean'; @@ -234,7 +234,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::INTEGER', + replacement: 'param::INT', since: '2.0', reason: 'Alias for INT', final: true, @@ -246,7 +246,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::NUMBER', + replacement: 'param::FLOAT', since: '2.0', reason: 'Alias for FLOAT', final: true, @@ -259,7 +259,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::ACTION', + replacement: 'param::ALPHANUMEXT', since: '2.0', reason: 'Alias for PARAM_ALPHANUMEXT', final: true, @@ -272,7 +272,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::FORMAT', + replacement: 'param::ALPHANUMEXT', since: '2.0', reason: 'Alias for PARAM_ALPHANUMEXT', final: true, @@ -284,7 +284,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::MULTILANG', + replacement: 'param::TEXT', since: '2.0', reason: 'Alias for PARAM_TEXT', final: true, @@ -303,7 +303,7 @@ enum param: string { * @deprecated since 2.0 */ #[deprecated( - 'param::CLEANFILE', + replacement: 'param::FILE', since: '2.0', reason: 'Alias for PARAM_FILE', )] diff --git a/lib/tests/deprecation_test.php b/lib/tests/deprecation_test.php index 12a0dd52bd9..efa1fd8e48d 100644 --- a/lib/tests/deprecation_test.php +++ b/lib/tests/deprecation_test.php @@ -24,6 +24,7 @@ namespace core; * @copyright 2024 Andrew Lyons * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * @covers \core\deprecated + * @covers \core\deprecated_with_reference * @covers \core\deprecation */ class deprecation_test extends \advanced_testcase { @@ -40,11 +41,13 @@ class deprecation_test extends \advanced_testcase { } $attribute = new deprecated( - 'Test description', ...$args, + replacement: 'Test replacement', ); - deprecation::emit_deprecation_notice($attribute); + $rc = new \ReflectionClass(deprecation::class); + $method = $rc->getMethod('emit_deprecation_notice'); + $method->invoke(null, $attribute); if ($expectdebugging) { $this->assertdebuggingcalledcount(1); @@ -92,19 +95,20 @@ class deprecation_test extends \advanced_testcase { * @dataProvider get_deprecation_string_provider */ public function test_get_deprecation_string( - string $descriptor, + ?string $replacement, ?string $since, ?string $reason, - ?string $replacement, ?string $mdl, string $expected, ): void { - $attribute = new deprecated( - descriptor: $descriptor, + $attribute = new deprecated_with_reference( + owner: 'Test description', + replacement: $replacement, since: $since, reason: $reason, - replacement: $replacement, mdl: $mdl, + final: false, + emit: true, ); $this->assertEquals( @@ -112,63 +116,81 @@ class deprecation_test extends \advanced_testcase { deprecation::get_deprecation_string($attribute), ); - deprecation::emit_deprecation_notice($attribute); + $rc = new \ReflectionClass(deprecation::class); + $method = $rc->getMethod('emit_deprecation_notice'); + $method->invoke(null, $attribute); + $this->assertDebuggingCalled($expected); } public static function get_deprecation_string_provider(): array { return [ [ - 'Test description', - null, - null, - null, - null, - 'Deprecation: Test description has been deprecated.', - ], - [ - 'Test description', - '4.1', - null, - null, - null, - 'Deprecation: Test description has been deprecated since 4.1.', - ], - [ - 'Test description', - null, - 'Test reason', - null, - null, - 'Deprecation: Test description has been deprecated. Test reason.', - ], - [ - 'Test description', - null, - null, 'Test replacement', null, + null, + null, 'Deprecation: Test description has been deprecated. Use Test replacement instead.', ], [ - 'Test description', + 'Test replacement', + '4.1', null, null, + 'Deprecation: Test description has been deprecated since 4.1. Use Test replacement instead.', + ], + [ + 'Test replacement', + null, + 'Test reason', + null, + 'Deprecation: Test description has been deprecated. Test reason. Use Test replacement instead.', + ], + [ + 'Test replacement', + null, + null, + null, + 'Deprecation: Test description has been deprecated. Use Test replacement instead.', + ], + [ + 'Test replacement', + null, null, 'https://docs.moodle.org/311/en/Deprecated', - 'Deprecation: Test description has been deprecated. See https://docs.moodle.org/311/en/Deprecated for more information.', + 'Deprecation: Test description has been deprecated. Use Test replacement instead. See https://docs.moodle.org/311/en/Deprecated for more information.', ], [ - 'Test description', + 'Test replacement', '4.1', 'Test reason', - 'Test replacement', 'https://docs.moodle.org/311/en/Deprecated', 'Deprecation: Test description has been deprecated since 4.1. Test reason. Use Test replacement instead. See https://docs.moodle.org/311/en/Deprecated for more information.', ], + [ + null, + null, + 'Test reason', + null, + 'Deprecation: Test description has been deprecated. Test reason.', + ], + [ + null, + null, + null, + 'MDL-80677', + 'Deprecation: Test description has been deprecated. See MDL-80677 for more information.', + ], ]; } + public function test_deprecated_without_replacement(): void { + $this->expectException(\coding_exception::class); + new deprecated( + replacement: null, + ); + } + /** * @dataProvider from_provider */ @@ -230,6 +252,9 @@ class deprecation_test extends \advanced_testcase { ['non_existent_class', false], [['non_existent_class'], false], + // Non-existent feature in an existent class. + [[\core\fixtures\not_deprecated_class::class, 'no_such_method'], false], + // Not-deprecated class. [\core\fixtures\not_deprecated_class::class, false], diff --git a/lib/tests/fixtures/deprecated_fixtures.php b/lib/tests/fixtures/deprecated_fixtures.php index a5f8990f474..b8bf75a2cb0 100644 --- a/lib/tests/fixtures/deprecated_fixtures.php +++ b/lib/tests/fixtures/deprecated_fixtures.php @@ -26,27 +26,32 @@ use core\deprecated; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -#[deprecated('Deprecated class')] +#[deprecated('not_deprecated_class')] class deprecated_class { protected string $notdeprecatedproperty = 'Not deprecated property'; - #[deprecated('Deprecated property')] + #[deprecated('$this->notdeprecatedproperty')] protected string $deprecatedproperty = 'Deprecated property'; const NOT_DEPRECATED_CONST = 'Not deprecated const'; - #[deprecated('Deprecated const')] + #[deprecated('not_deprecated_class::NEW_CONSTANT')] const DEPRECATED_CONST = 'Deprecated const'; - public function not_deprecated_method() {} + public function not_deprecated_method() { + } - #[deprecated('Deprecated method')] - public function deprecated_method() {} + #[deprecated(replacement: null, mdl: 'MDL-80677')] + public function deprecated_method() { + } } -class not_deprecated_class {} +class not_deprecated_class { +} -function not_deprecated_function() {} +function not_deprecated_function() { +} -#[deprecated('Deprecated function')] -function deprecated_function() {} +#[deprecated('not_deprecated_class::not_deprecated_method()')] +function deprecated_function() { +}