diff --git a/enrol/externallib.php b/enrol/externallib.php index 0497bc3fcf3..4cb5e0c4b24 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -211,7 +211,7 @@ class core_enrol_external extends external_api { 'lastname' => new external_value(PARAM_NOTAGS, 'The family name of the user', VALUE_OPTIONAL), 'fullname' => new external_value(PARAM_NOTAGS, 'The fullname of the user'), 'email' => new external_value(PARAM_TEXT, 'Email address', VALUE_OPTIONAL), - 'address' => new external_value(PARAM_MULTILANG, 'Postal address', VALUE_OPTIONAL), + 'address' => new external_value(PARAM_TEXT, 'Postal address', VALUE_OPTIONAL), 'phone1' => new external_value(PARAM_NOTAGS, 'Phone 1', VALUE_OPTIONAL), 'phone2' => new external_value(PARAM_NOTAGS, 'Phone 2', VALUE_OPTIONAL), 'department' => new external_value(PARAM_TEXT, 'department', VALUE_OPTIONAL), diff --git a/lib/classes/deprecated.php b/lib/classes/deprecated.php new file mode 100644 index 00000000000..deb278abe94 --- /dev/null +++ b/lib/classes/deprecated.php @@ -0,0 +1,53 @@ +. + +namespace core; + +use Attribute; + +/** + * 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 + */ +#[Attribute] +class deprecated { + /** + * A deprecated item. + * + * This attribute can be applied to any function, class, method, constant, property, enum, etc. + * + * 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 $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 + */ + public function __construct( + public readonly mixed $descriptor, + 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, + ) { + } +} diff --git a/lib/classes/deprecation.php b/lib/classes/deprecation.php new file mode 100644 index 00000000000..b71fd502d9b --- /dev/null +++ b/lib/classes/deprecation.php @@ -0,0 +1,203 @@ +. + +namespace core; + +/** + * Deprecation utility. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class deprecation { + /** + * Get the attribute from a reference. + * + * The reference can be: + * - a string, in which case it will be checked for a function, class, method, property, constant, or enum. + * - an array + * - an instantiated object, in which case the object will be checked for a class, method, property, or constant. + * + * @param array|string|object $reference A reference to a potentially deprecated thing. + * @return null|deprecated + */ + public static function from(array|string|object $reference): ?deprecated { + if (is_string($reference)) { + if (str_contains($reference, '::')) { + // The reference is a string but it looks to be in the format `object::item`. + return self::from(explode('::', $reference)); + } + + if (class_exists($reference)) { + // The reference looks to be a class name. + return self::from([$reference]); + } + + 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 null; + } + + if (is_object($reference)) { + // The reference is an object. Normalise and check again. + return self::from([$reference]); + } + + if (is_array($reference) && count($reference)) { + if (is_object($reference[0])) { + $rc = new \ReflectionObject($reference[0]); + + if ($rc->isEnum() && $reference[0]->name) { + // Enums can be passed via ::from([enum::NAME]). + // In this case they will have a 'name', which must exist. + return self::from_reflected_object($rc, $reference[0]->name); + } + return self::from_reflected_object($rc, $reference[1] ?? null); + } + + if (is_string($reference[0]) && class_exists($reference[0])) { + $rc = new \ReflectionClass($reference[0]); + return self::from_reflected_object($rc, $reference[1] ?? null); + } + + // The reference is an array, but it's not an object or a class that currently exists. + return null; + } + } + + /** + * Check if a reference is deprecated. + * + * @param array|string|object $reference + * @return bool + */ + public static function is_deprecated(array|string|object $reference): bool { + return self::from($reference) !== null; + } + + /** + * Emit a deprecation notice if the reference is deprecated. + * + * @param array|string|object $reference + */ + public static function emit_deprecation_if_present(array|string|object $reference): void { + if ($attribute = self::from($reference)) { + self::emit_deprecation_notice($attribute); + } + } + + /** + * Fetch a 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 + */ + protected static function from_reflected_object( + \ReflectionClass $rc, + ?string $name, + ): ?deprecated { + if ($name === null) { + // No name specified. This may be a deprecated class. + if ($attributes = $rc->getAttributes(deprecated::class)) { + return $attributes[0]->newInstance(); + } + return null; + } + + 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(); + } + } + + 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(); + } + } + + 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 null; + } + + /** + * Get a string describing the deprecation. + * + * @param deprecated $attribute + * @return string + */ + public static function get_deprecation_string(deprecated $attribute): string { + $output = "Deprecation: {$attribute->descriptor} has been deprecated"; + if ($attribute->since) { + $output .= " since {$attribute->since}"; + } + + $output .= "."; + + if ($attribute->reason) { + $output .= " {$attribute->reason}."; + } + + if ($attribute->replacement) { + $output .= " Use {$attribute->replacement} instead."; + } + + if ($attribute->mdl) { + $output .= " See {$attribute->mdl} for more information."; + } + + return $output; + } + + /** + * Emit the relevant deprecation notice. + * + * @param deprecated $attribute + */ + public static function emit_deprecation_notice(deprecated $attribute): void { + if (!$attribute->emit) { + return; + } + + $message = self::get_deprecation_string($attribute); + + if ($attribute->final) { + throw new \coding_exception($message); + } + + debugging($message, DEBUG_DEVELOPER); + } +} diff --git a/lib/classes/hub/api.php b/lib/classes/hub/api.php index 1cf8bd95fa4..118bac8e741 100644 --- a/lib/classes/hub/api.php +++ b/lib/classes/hub/api.php @@ -171,9 +171,9 @@ class api { * * Parameter $options may have any of these fields: * [ - * 'ids' => new external_multiple_structure(new external_value(PARAM_INTEGER, 'id of a course in the hub course + * 'ids' => new external_multiple_structure(new external_value(PARAM_INT, 'id of a course in the hub course * directory'), 'ids of course', VALUE_OPTIONAL), - * 'sitecourseids' => new external_multiple_structure(new external_value(PARAM_INTEGER, 'id of a course in the + * 'sitecourseids' => new external_multiple_structure(new external_value(PARAM_INT, 'id of a course in the * site'), 'ids of course in the site', VALUE_OPTIONAL), * 'coverage' => new external_value(PARAM_TEXT, 'coverage', VALUE_OPTIONAL), * 'licenceshortname' => new external_value(PARAM_ALPHANUMEXT, 'licence short name', VALUE_OPTIONAL), @@ -185,7 +185,7 @@ class api { * ratingaverage', VALUE_OPTIONAL), * 'givememore' => new external_value(PARAM_INT, 'next range of result - range size being set by the hub * server ', VALUE_OPTIONAL), - * 'allsitecourses' => new external_value(PARAM_INTEGER, + * 'allsitecourses' => new external_value(PARAM_INT, * 'if 1 return all not visible and visible courses whose siteid is the site * matching token. Only courses of this site are returned. * givememore parameter is ignored if this param = 1. @@ -194,7 +194,7 @@ class api { * * Each course in the returned array of courses will have fields: * [ - * 'id' => new external_value(PARAM_INTEGER, 'id'), + * 'id' => new external_value(PARAM_INT, 'id'), * 'fullname' => new external_value(PARAM_TEXT, 'course name'), * 'shortname' => new external_value(PARAM_TEXT, 'course short name'), * 'description' => new external_value(PARAM_TEXT, 'course description'), @@ -211,7 +211,7 @@ class api { * 'audience' => new external_value(PARAM_ALPHA, 'audience'), * 'educationallevel' => new external_value(PARAM_ALPHA, 'educational level'), * 'creatornotes' => new external_value(PARAM_RAW, 'creator notes'), - * 'creatornotesformat' => new external_value(PARAM_INTEGER, 'notes format'), + * 'creatornotesformat' => new external_value(PARAM_INT, 'notes format'), * 'demourl' => new external_value(PARAM_URL, 'demo URL', VALUE_OPTIONAL), * 'courseurl' => new external_value(PARAM_URL, 'course URL', VALUE_OPTIONAL), * 'backupsize' => new external_value(PARAM_INT, 'course backup size in bytes', VALUE_OPTIONAL), @@ -305,7 +305,7 @@ class api { * 'audience' => new external_value(PARAM_ALPHA, 'audience'), * 'educationallevel' => new external_value(PARAM_ALPHA, 'educational level'), * 'creatornotes' => new external_value(PARAM_RAW, 'creator notes'), - * 'creatornotesformat' => new external_value(PARAM_INTEGER, 'notes format'), + * 'creatornotesformat' => new external_value(PARAM_INT, 'notes format'), * 'demourl' => new external_value(PARAM_URL, 'demo URL', VALUE_OPTIONAL), * 'courseurl' => new external_value(PARAM_URL, 'course URL', VALUE_OPTIONAL), * 'enrollable' => new external_value(PARAM_BOOL, 'is the course enrollable', VALUE_DEFAULT, 0), diff --git a/lib/classes/param.php b/lib/classes/param.php index e9a153ff18d..0c350a7078f 100644 --- a/lib/classes/param.php +++ b/lib/classes/param.php @@ -220,18 +220,36 @@ enum param: string { * It was one of the first types, that is why it is abused so much ;-) * @deprecated since 2.0 */ + #[deprecated( + 'param::CLEAN', + since: '2.0', + reason: 'Please use a more specific type of parameter', + emit: false, + )] case CLEAN = 'clean'; /** * PARAM_INTEGER - deprecated alias for PARAM_INT * @deprecated since 2.0 */ + #[deprecated( + 'param::INTEGER', + since: '2.0', + reason: 'Alias for INT', + final: true, + )] case INTEGER = 'integer'; /** * PARAM_NUMBER - deprecated alias of PARAM_FLOAT * @deprecated since 2.0 */ + #[deprecated( + 'param::NUMBER', + since: '2.0', + reason: 'Alias for FLOAT', + final: true, + )] case NUMBER = 'number'; /** @@ -239,6 +257,12 @@ enum param: string { * NOTE: originally alias for PARAM_ALPHANUMEXT * @deprecated since 2.0 */ + #[deprecated( + 'param::ACTION', + since: '2.0', + reason: 'Alias for PARAM_ALPHANUMEXT', + final: true, + )] case ACTION = 'action'; /** @@ -246,12 +270,24 @@ enum param: string { * NOTE: originally alias for PARAM_APLHA * @deprecated since 2.0 */ + #[deprecated( + 'param::FORMAT', + since: '2.0', + reason: 'Alias for PARAM_ALPHANUMEXT', + final: true, + )] case FORMAT = 'format'; /** * PARAM_MULTILANG - deprecated alias of PARAM_TEXT. * @deprecated since 2.0 */ + #[deprecated( + 'param::MULTILANG', + since: '2.0', + reason: 'Alias for PARAM_TEXT', + final: true, + )] case MULTILANG = 'multilang'; /** @@ -334,11 +370,14 @@ enum param: string { * $selectedgradeitem = param::INT->clean($selectedgradeitem); * * - * @param mixed $param the variable we are cleaning + * @param mixed $value the value to clean * @return mixed * @throws coding_exception */ public function clean(mixed $value): mixed { + // Check and emit a deprecation notice if required. + deprecation::emit_deprecation_if_present($this); + if (is_array($value)) { throw new coding_exception('clean() can not process arrays, please use clean_array() instead.'); } else if (is_object($value)) { @@ -1279,4 +1318,13 @@ enum param: string { return ''; } } + + /** + * Whether the parameter is deprecated. + * + * @return bool + */ + public function is_deprecated(): bool { + return deprecation::is_deprecated($this); + } } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 9c7a9c6826a..0669b3e9af2 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -73,6 +73,8 @@ define('HOURMINS', 60); // We currently include \core\param manually here to avoid broken upgrades. // This may change after the next LTS release as LTS releases require the previous LTS release. +require_once(__DIR__ . '/classes/deprecation.php'); +require_once(__DIR__ . '/classes/deprecated.php'); require_once(__DIR__ . '/classes/param.php'); /** diff --git a/lib/tests/deprecation_test.php b/lib/tests/deprecation_test.php new file mode 100644 index 00000000000..12a0dd52bd9 --- /dev/null +++ b/lib/tests/deprecation_test.php @@ -0,0 +1,241 @@ +. + +namespace core; + +/** + * Tests for \core\deprecated and \core\deprecation. + * + * @package core + * @category test + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \core\deprecated + * @covers \core\deprecation + */ +class deprecation_test extends \advanced_testcase { + /** + * @dataProvider emit_provider + */ + public function test_emit( + array $args, + bool $expectdebugging, + bool $expectexception, + ): void { + if ($expectexception) { + $this->expectException(\coding_exception::class); + } + + $attribute = new deprecated( + 'Test description', + ...$args, + ); + + deprecation::emit_deprecation_notice($attribute); + + if ($expectdebugging) { + $this->assertdebuggingcalledcount(1); + } + } + + public static function emit_provider(): array { + return [ + [ + [ + 'final' => false, + 'emit' => false, + ], + false, + false, + ], + [ + [ + 'final' => false, + 'emit' => true, + ], + true, + false, + ], + [ + [ + 'final' => true, + 'emit' => false, + ], + false, + false, + ], + [ + [ + 'final' => true, + 'emit' => true, + ], + false, + true, + ], + ]; + } + + /** + * @dataProvider get_deprecation_string_provider + */ + public function test_get_deprecation_string( + string $descriptor, + ?string $since, + ?string $reason, + ?string $replacement, + ?string $mdl, + string $expected, + ): void { + $attribute = new deprecated( + descriptor: $descriptor, + since: $since, + reason: $reason, + replacement: $replacement, + mdl: $mdl, + ); + + $this->assertEquals( + $expected, + deprecation::get_deprecation_string($attribute), + ); + + deprecation::emit_deprecation_notice($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, + 'Deprecation: Test description has been deprecated. Use Test replacement instead.', + ], + [ + 'Test description', + null, + 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.', + ], + [ + 'Test description', + '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.', + ], + ]; + } + + /** + * @dataProvider from_provider + */ + public function test_from($reference, bool $isdeprecated): void { + $attribute = deprecation::from($reference); + if ($isdeprecated) { + $this->assertInstanceOf(deprecated::class, $attribute); + $this->assertTrue(deprecation::is_deprecated($reference)); + $this->assertDebuggingNotCalled(); + + deprecation::emit_deprecation_if_present($reference); + $this->assertDebuggingCalled(deprecation::get_deprecation_string($attribute)); + } else { + $this->assertNull($attribute); + $this->assertFalse(deprecation::is_deprecated($reference)); + deprecation::emit_deprecation_if_present($reference); + $this->assertDebuggingNotCalled(); + } + } + + public function test_from_object(): void { + require_once(dirname(__FILE__) . '/fixtures/deprecated_fixtures.php'); + + $this->assertNull(deprecation::from(new \core\fixtures\not_deprecated_class())); + $this->assertInstanceOf(deprecated::class, deprecation::from([new \core\fixtures\deprecated_class()])); + } + + public static function from_provider(): array { + require_once(dirname(__FILE__) . '/fixtures/deprecated_fixtures.php'); + return [ + // Classes. + [\core\fixtures\deprecated_class::class, true], + [[\core\fixtures\deprecated_class::class], true], + [\core\fixtures\not_deprecated_class::class, false], + [[\core\fixtures\not_deprecated_class::class], false], + + // Class properties. + [\core\fixtures\deprecated_class::class . '::deprecatedproperty', true], + [[\core\fixtures\deprecated_class::class, 'deprecatedproperty'], true], + + [\core\fixtures\deprecated_class::class . '::notdeprecatedproperty', false], + [[\core\fixtures\deprecated_class::class, 'notdeprecatedproperty'], false], + + // Class constants. + [\core\fixtures\deprecated_class::class . '::DEPRECATED_CONST', true], + [[\core\fixtures\deprecated_class::class, 'DEPRECATED_CONST'], true], + + [\core\fixtures\deprecated_class::class . '::NOT_DEPRECATED_CONST', false], + [[\core\fixtures\deprecated_class::class, 'NOT_DEPRECATED_CONST'], false], + + // Class methods. + [\core\fixtures\deprecated_class::class . '::deprecated_method', true], + [[\core\fixtures\deprecated_class::class, 'deprecated_method'], true], + + [\core\fixtures\deprecated_class::class . '::not_deprecated_method', false], + [[\core\fixtures\deprecated_class::class, 'not_deprecated_method'], false], + + // Non-existent class. + ['non_existent_class', false], + [['non_existent_class'], false], + + // Not-deprecated class. + [\core\fixtures\not_deprecated_class::class, false], + + // Deprecated global function. + ['core\fixtures\deprecated_function', true], + ['core\fixtures\not_deprecated_function', false], + ]; + } +} diff --git a/lib/tests/fixtures/deprecated_fixtures.php b/lib/tests/fixtures/deprecated_fixtures.php new file mode 100644 index 00000000000..a5f8990f474 --- /dev/null +++ b/lib/tests/fixtures/deprecated_fixtures.php @@ -0,0 +1,52 @@ +. + +namespace core\fixtures; + +use core\deprecated; + +/** + * A file containing a variety of fixturs for deprecated attribute tests. + * + * @package core + * @copyright 2024 Andrew Lyons + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +#[deprecated('Deprecated class')] +class deprecated_class { + protected string $notdeprecatedproperty = 'Not deprecated property'; + + #[deprecated('Deprecated property')] + protected string $deprecatedproperty = 'Deprecated property'; + + const NOT_DEPRECATED_CONST = 'Not deprecated const'; + + #[deprecated('Deprecated const')] + const DEPRECATED_CONST = 'Deprecated const'; + + public function not_deprecated_method() {} + + #[deprecated('Deprecated method')] + public function deprecated_method() {} +} + +class not_deprecated_class {} + +function not_deprecated_function() {} + +#[deprecated('Deprecated function')] +function deprecated_function() {} diff --git a/lib/tests/param_test.php b/lib/tests/param_test.php index 3162ac85143..0d82cfd4e84 100644 --- a/lib/tests/param_test.php +++ b/lib/tests/param_test.php @@ -97,6 +97,39 @@ class param_test extends \advanced_testcase { // Some deprecated parameters. [param::INTEGER, true], [param::NUMBER, true], + [param::CLEAN, true], ]; } + + + /** + * Test that finally deprecated params throw an exception when cleaning. + * + * @dataProvider deprecated_param_provider + * @param param $params + */ + public function test_deprecated_params_except(param $param): void { + $this->expectException(\coding_exception::class); + $param->clean('foo'); + } + + /** + * Provider for deprecated parameters. + * + * @return array + */ + public static function deprecated_param_provider(): array { + return array_map( + fn (param $param): array => [$param], + array_filter( + param::cases(), + function (param $param): bool { + if ($attribute = deprecation::from($param)) { + return $attribute->emit && $attribute->final; + } + return false; + }, + ), + ); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 6f62d0ddae3..c64e2ba142f 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -26,6 +26,11 @@ information provided here is intended especially for developers. - set_attempts_available(): Used to set the number of attempts available for the task - get_attempts_available(): Used to get the number of attempts available for the task. * There is a new DML method $DB->get_fieldset. For some reason, this did not exist even though get_fieldset_select etc. did. +* Deprecated PARAM_ types with the exception of PARAM_CLEAN now emit a deprecation exception. These were all deprecated in Moodle 2.0. +* A new \core\deprecated attribute can be used to more clearly describe deprecated methods. +* A new \core\deprecation class can be used to inspect for deprecated attributes: + - `\core\deprecation::is_deprecated(example::class);` + - `\core\deprecation::emit_deprecation_if_present([self::class, 'some_method']);` === 4.3 === diff --git a/mod/workshop/classes/external.php b/mod/workshop/classes/external.php index 6605fc2ff04..2e514712d93 100644 --- a/mod/workshop/classes/external.php +++ b/mod/workshop/classes/external.php @@ -1508,7 +1508,7 @@ class mod_workshop_external extends external_api { $data->feedbackauthor_editor['text'] = $wsdata['value']; break; case 'feedbackauthorformat': - $data->feedbackauthor_editor['format'] = clean_param($wsdata['value'], PARAM_FORMAT); + $data->feedbackauthor_editor['format'] = clean_param($wsdata['value'], PARAM_ALPHANUMEXT); break; case 'feedbackauthorinlineattachmentsid': $data->feedbackauthor_editor['itemid'] = clean_param($wsdata['value'], PARAM_INT); diff --git a/question/type/gapselect/questionbase.php b/question/type/gapselect/questionbase.php index 10595d011c1..33bd490ac1f 100644 --- a/question/type/gapselect/questionbase.php +++ b/question/type/gapselect/questionbase.php @@ -222,7 +222,7 @@ abstract class qtype_gapselect_question_base extends question_graded_automatical public function get_expected_data() { $vars = array(); foreach ($this->places as $place => $notused) { - $vars[$this->field($place)] = PARAM_INTEGER; + $vars[$this->field($place)] = PARAM_INT; } return $vars; }