diff --git a/admin/tool/analytics/classes/output/helper.php b/admin/tool/analytics/classes/output/helper.php index 42025621144..c4628af2866 100644 --- a/admin/tool/analytics/classes/output/helper.php +++ b/admin/tool/analytics/classes/output/helper.php @@ -45,7 +45,7 @@ class helper { // Form field is PARAM_ALPHANUMEXT and we are sending fully qualified class names // as option names, but replacing the backslash for a string that is really unlikely // to ever be part of a class name. - return str_replace('\\', '__', $class); + return str_replace('\\', '__', $class ?? ''); } /** diff --git a/admin/tool/behat/cli/run.php b/admin/tool/behat/cli/run.php index afaddd8b88d..454299e7612 100644 --- a/admin/tool/behat/cli/run.php +++ b/admin/tool/behat/cli/run.php @@ -437,7 +437,7 @@ function print_combined_run_output($processes, $stoponfail = false) { $process->stop(0); } - $strlentoprint = strlen($update); + $strlentoprint = strlen($update ?? ''); // If not enough dots printed on line then just print. if ($strlentoprint < $remainingprintlen) { diff --git a/admin/tool/brickfield/classes/manager.php b/admin/tool/brickfield/classes/manager.php index 013a4b3f129..97fff856690 100644 --- a/admin/tool/brickfield/classes/manager.php +++ b/admin/tool/brickfield/classes/manager.php @@ -205,7 +205,7 @@ class manager { * @return string */ public static function get_contenthash(?string $content = null): string { - return sha1($content); + return sha1($content ?? ''); } /** diff --git a/admin/tool/dataprivacy/classes/data_registry.php b/admin/tool/dataprivacy/classes/data_registry.php index 75670f4953c..fcfe0d25e2c 100644 --- a/admin/tool/dataprivacy/classes/data_registry.php +++ b/admin/tool/dataprivacy/classes/data_registry.php @@ -47,7 +47,7 @@ class data_registry { * @return string[] */ public static function var_names_from_context($classname, $pluginname = '') { - $pluginname = trim($pluginname); + $pluginname = trim($pluginname ?? ''); if (!empty($pluginname)) { $categoryvar = $classname . '_' . $pluginname . '_category'; $purposevar = $classname . '_' . $pluginname . '_purpose'; diff --git a/admin/tool/dataprivacy/classes/external/purpose_exporter.php b/admin/tool/dataprivacy/classes/external/purpose_exporter.php index 8e2423553a0..bcbca407c4d 100644 --- a/admin/tool/dataprivacy/classes/external/purpose_exporter.php +++ b/admin/tool/dataprivacy/classes/external/purpose_exporter.php @@ -111,7 +111,7 @@ class purpose_exporter extends persistent_exporter { $values['formattedlawfulbases'] = $formattedbases; $formattedsensitivereasons = []; - $sensitivereasons = explode(',', $this->persistent->get('sensitivedatareasons')); + $sensitivereasons = explode(',', $this->persistent->get('sensitivedatareasons') ?? ''); if (!empty($sensitivereasons)) { foreach ($sensitivereasons as $reason) { if (empty(trim($reason))) { diff --git a/admin/tool/log/classes/helper/reader.php b/admin/tool/log/classes/helper/reader.php index 2e947748da0..ae0a376c816 100644 --- a/admin/tool/log/classes/helper/reader.php +++ b/admin/tool/log/classes/helper/reader.php @@ -74,10 +74,10 @@ trait reader { * @return mixed Decoded value */ public static function decode_other(?string $other) { - if ($other === 'N;' || preg_match('~^.:~', $other)) { + if ($other === 'N;' || preg_match('~^.:~', $other ?? '')) { return unserialize($other); } else { - return json_decode($other, true); + return json_decode($other ?? '', true); } } diff --git a/admin/tool/moodlenet/classes/local/remote_resource.php b/admin/tool/moodlenet/classes/local/remote_resource.php index e9526184b8c..a08a3bb7015 100644 --- a/admin/tool/moodlenet/classes/local/remote_resource.php +++ b/admin/tool/moodlenet/classes/local/remote_resource.php @@ -60,8 +60,8 @@ class remote_resource { public function __construct(\curl $curl, url $url, \stdClass $metadata) { $this->curl = $curl; $this->url = $url; - $this->filename = pathinfo($this->url->get_path(), PATHINFO_FILENAME); - $this->extension = pathinfo($this->url->get_path(), PATHINFO_EXTENSION); + $this->filename = pathinfo($this->url->get_path() ?? '', PATHINFO_FILENAME); + $this->extension = pathinfo($this->url->get_path() ?? '', PATHINFO_EXTENSION); $this->metadata = $metadata; } diff --git a/analytics/classes/model.php b/analytics/classes/model.php index 5fc63d4b166..155cb3d66c0 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -1828,7 +1828,7 @@ class model { */ public function get_name() { - if (trim($this->model->name) === '') { + if (trim($this->model->name ?? '') === '') { return $this->get_target()->get_name(); } else { diff --git a/backup/moodle2/backup_custom_fields.php b/backup/moodle2/backup_custom_fields.php index 9de5ce68067..2372003b170 100644 --- a/backup/moodle2/backup_custom_fields.php +++ b/backup/moodle2/backup_custom_fields.php @@ -197,7 +197,7 @@ class encrypted_final_element extends backup_final_element { $iv = self::generate_encryption_random_key(openssl_cipher_iv_length(backup::CIPHER)); // Everything is ready, let's encrypt and prepend the 1-shot iv. - $value = $iv . openssl_encrypt($value, backup::CIPHER, $this->key, OPENSSL_RAW_DATA, $iv); + $value = $iv . openssl_encrypt($value ?? '', backup::CIPHER, $this->key, OPENSSL_RAW_DATA, $iv); // Calculate the hmac of the value (iv + encrypted) and prepend it. $hmac = hash_hmac('sha256', $value, $this->key, true); diff --git a/backup/util/helper/restore_decode_processor.class.php b/backup/util/helper/restore_decode_processor.class.php index 0c7bf1e2cd7..80aa087b3f1 100644 --- a/backup/util/helper/restore_decode_processor.class.php +++ b/backup/util/helper/restore_decode_processor.class.php @@ -170,7 +170,7 @@ class restore_decode_processor { */ protected function precheck_content($content) { // Look for $@ in content (all interlinks contain that) - return (strpos($content, '$@') === false) ? false : $content; + return (strpos($content ?? '', '$@') === false) ? false : $content; } } diff --git a/backup/util/helper/restore_structure_parser_processor.class.php b/backup/util/helper/restore_structure_parser_processor.class.php index 9fc1f6e4d27..67a7a2d9213 100644 --- a/backup/util/helper/restore_structure_parser_processor.class.php +++ b/backup/util/helper/restore_structure_parser_processor.class.php @@ -55,8 +55,10 @@ class restore_structure_parser_processor extends grouped_parser_processor { return ''; } else if (is_numeric($cdata)) { return $cdata; - } else if (strlen($cdata) < 32) { // Impossible to have one link in 32cc - return $cdata; // (http://10.0.0.1/file.php/1/1.jpg, http://10.0.0.1/mod/url/view.php?id=) + } else if (strlen($cdata ?? '') < 32) { + // Impossible to have one link in 32cc. + // (http://10.0.0.1/file.php/1/1.jpg, http://10.0.0.1/mod/url/view.php?id=). + return $cdata; } if (strpos($cdata, '$@FILEPHP@$') !== false) { diff --git a/backup/util/xml/output/xml_output.class.php b/backup/util/xml/output/xml_output.class.php index 159fbdb10b9..059c2f95a08 100644 --- a/backup/util/xml/output/xml_output.class.php +++ b/backup/util/xml/output/xml_output.class.php @@ -107,7 +107,7 @@ abstract class xml_output { if (!$this->running) { throw new xml_output_exception('xml_output_not_started'); } - $lenc = strlen($content); // Get length in bytes + $lenc = strlen($content ?? ''); // Get length in bytes. if ($lenc == 0) { // 0 length contents, nothing to do return; } diff --git a/backup/util/xml/xml_writer.class.php b/backup/util/xml/xml_writer.class.php index cc1f3c26b5d..4673e4da3d4 100644 --- a/backup/util/xml/xml_writer.class.php +++ b/backup/util/xml/xml_writer.class.php @@ -260,7 +260,7 @@ class xml_writer { * ignores the rest of characters. Also normalize linefeeds and return chars. */ protected function xml_safe_utf8($content) { - $content = preg_replace('/[\x-\x8\xb-\xc\xe-\x1f\x7f]/is','', $content); // clean CTRL chars + $content = preg_replace('/[\x-\x8\xb-\xc\xe-\x1f\x7f]/is', '', $content ?? ''); // clean CTRL chars. $content = preg_replace("/\r\n|\r/", "\n", $content); // Normalize line&return=>line return $content; } diff --git a/badges/classes/backpack_api_mapping.php b/badges/classes/backpack_api_mapping.php index aa9155c4b1d..4c10f672a77 100644 --- a/badges/classes/backpack_api_mapping.php +++ b/badges/classes/backpack_api_mapping.php @@ -170,8 +170,8 @@ class backpack_api_mapping { $url = str_replace('[SCHEME]', $urlscheme, $url); $url = str_replace('[HOST]', $urlhost, $url); $url = str_replace('[URL]', $apiurl, $url); - $url = str_replace('[PARAM1]', $param1, $url); - $url = str_replace('[PARAM2]', $param2, $url); + $url = str_replace('[PARAM1]', $param1 ?? '', $url); + $url = str_replace('[PARAM2]', $param2 ?? '', $url); return $url; } diff --git a/cache/classes/helper.php b/cache/classes/helper.php index cf9364e30ea..e2f8b2300df 100644 --- a/cache/classes/helper.php +++ b/cache/classes/helper.php @@ -634,7 +634,7 @@ class cache_helper { */ public static function hash_key($key, cache_definition $definition) { if ($definition->uses_simple_keys()) { - if (debugging() && preg_match('#[^a-zA-Z0-9_]#', $key)) { + if (debugging() && preg_match('#[^a-zA-Z0-9_]#', $key ?? '')) { throw new coding_exception('Cache definition '.$definition->get_id().' requires simple keys. Invalid key provided.', $key); } // We put the key first so that we can be sure the start of the key changes. diff --git a/course/classes/external/course_summary_exporter.php b/course/classes/external/course_summary_exporter.php index 0916abda094..e50f11bdf6b 100644 --- a/course/classes/external/course_summary_exporter.php +++ b/course/classes/external/course_summary_exporter.php @@ -64,7 +64,7 @@ class course_summary_exporter extends \core\external\exporter { if ($progress === 0 || $progress > 0) { $hasprogress = true; } - $progress = floor($progress); + $progress = floor($progress ?? 0); $coursecategory = \core_course_category::get($this->data->category, MUST_EXIST, true); return array( 'fullnamedisplay' => get_course_display_name_for_list($this->data), diff --git a/course/format/classes/base.php b/course/format/classes/base.php index 66b1e5f09f5..26000882a46 100644 --- a/course/format/classes/base.php +++ b/course/format/classes/base.php @@ -608,7 +608,7 @@ abstract class base { $course = $this->get_course(); try { $sectionpreferences = (array) json_decode( - get_user_preferences('coursesectionspreferences_' . $course->id, null, $USER->id) + get_user_preferences("coursesectionspreferences_{$course->id}", '', $USER->id) ); if (empty($sectionpreferences)) { $sectionpreferences = []; diff --git a/course/format/classes/output/local/content/frontpagesection.php b/course/format/classes/output/local/content/frontpagesection.php index 50a4b47e243..ac34c59ac6c 100644 --- a/course/format/classes/output/local/content/frontpagesection.php +++ b/course/format/classes/output/local/content/frontpagesection.php @@ -84,7 +84,7 @@ class frontpagesection implements named_templatable, renderable { $sectionoutput = new $this->sectionclass($format, $section); $sectionoutput->hide_controls(); - if (trim($section->name) == '') { + if (trim($section->name ?? '') == '') { $sectionoutput->hide_title(); } diff --git a/course/user.php b/course/user.php index c6abc6f5f20..05f61885341 100644 --- a/course/user.php +++ b/course/user.php @@ -148,7 +148,7 @@ switch ($mode) { $coursenode->collapse = true; $coursenode->make_inactive(); - if (!preg_match('/^user\d{0,}$/', $activenode->key)) { // No user name found. + if (!preg_match('/^user\d{0,}$/', $activenode->key ?? '')) { // No user name found. $userurl = new moodle_url('/user/view.php', array('id' => $user->id, 'course' => $course->id)); // Add the user name. $usernode = $activenode->add(fullname($user), $userurl, navigation_node::TYPE_SETTING); diff --git a/customfield/classes/field.php b/customfield/classes/field.php index 1f7b7e1ec2c..7bd440abce1 100644 --- a/customfield/classes/field.php +++ b/customfield/classes/field.php @@ -92,6 +92,6 @@ class field extends persistent { * @return array */ protected function get_configdata() : array { - return json_decode($this->raw_get('configdata'), true) ?? array(); + return json_decode($this->raw_get('configdata') ?? '', true) ?? array(); } } diff --git a/enrol/lti/classes/data_connector.php b/enrol/lti/classes/data_connector.php index b061da986ad..430e14dd3ea 100644 --- a/enrol/lti/classes/data_connector.php +++ b/enrol/lti/classes/data_connector.php @@ -88,16 +88,18 @@ class data_connector extends DataConnector { global $DB; $id = $consumer->getRecordId(); + $key = $consumer->getKey(); + $result = false; if (!empty($id)) { $result = $DB->get_record($this->consumertable, ['id' => $id]); - } else { - $key256 = DataConnector::getConsumerKey($consumer->getKey()); + } else if (!empty($key)) { + $key256 = DataConnector::getConsumerKey($key); $result = $DB->get_record($this->consumertable, ['consumerkey256' => $key256]); } if ($result) { - if (empty($key256) || empty($result->consumerkey) || ($consumer->getKey() === $result->consumerkey)) { + if (empty($key256) || empty($result->consumerkey) || ($key === $result->consumerkey)) { $this->build_tool_consumer_object($result, $consumer); return true; } @@ -997,7 +999,7 @@ class data_connector extends DataConnector { $consumer->consumerName = $record->consumername; $consumer->consumerVersion = $record->consumerversion; $consumer->consumerGuid = $record->consumerguid; - $consumer->profile = json_decode($record->profile); + $consumer->profile = json_decode($record->profile ?? ''); $consumer->toolProxy = $record->toolproxy; $settings = unserialize($record->settings); if (!is_array($settings)) { diff --git a/enrol/lti/classes/local/ltiadvantage/lib/http_client.php b/enrol/lti/classes/local/ltiadvantage/lib/http_client.php index fe9fac862d8..1911ea2e1d0 100644 --- a/enrol/lti/classes/local/ltiadvantage/lib/http_client.php +++ b/enrol/lti/classes/local/ltiadvantage/lib/http_client.php @@ -78,8 +78,8 @@ class http_client implements IHttpClient { if (!$this->curlclient->get_errno() && !$this->curlclient->error) { // No errors, so format the response. $headersize = $info['header_size']; - $resheaders = substr($res, 0, $headersize); - $resbody = substr($res, $headersize); + $resheaders = substr($res ?? '', 0, $headersize); + $resbody = substr($res ?? '', $headersize); $headerlines = array_filter(explode("\r\n", $resheaders)); $parsedresponseheaders = [ 'httpstatus' => array_shift($headerlines) diff --git a/grade/report/singleview/classes/local/screen/user.php b/grade/report/singleview/classes/local/screen/user.php index 810bc5416a7..efeffe92bb1 100644 --- a/grade/report/singleview/classes/local/screen/user.php +++ b/grade/report/singleview/classes/local/screen/user.php @@ -405,16 +405,15 @@ class user extends tablelike implements selectable_items { $gradeitem = grade_item::fetch([ 'courseid' => $this->courseid, - 'id' => $matches[1] + 'id' => $matches[1], ]); $isscale = ($gradeitem->gradetype == GRADE_TYPE_SCALE); - $empties = (trim($value) === '' or ($isscale and $value == -1)); + $empties = (trim($value ?? '') === '' || ($isscale && $value == -1)); - if ($filter == 'all' or $empties) { - $data->$varname = ($isscale and empty($insertvalue)) ? - -1 : $insertvalue; + if ($filter == 'all' || $empties) { + $data->$varname = ($isscale && empty($insertvalue)) ? -1 : $insertvalue; } } } diff --git a/h5p/ajax.php b/h5p/ajax.php index 04a3ae1d9b1..f02f098e5e1 100644 --- a/h5p/ajax.php +++ b/h5p/ajax.php @@ -58,7 +58,7 @@ switch ($action) { $minor = optional_param('minorVersion', 0, PARAM_INT); // Normalise Moodle language using underscore, as opposed to H5P which uses dash. - $language = optional_param('default-language', null, PARAM_RAW); + $language = optional_param('default-language', '', PARAM_RAW); $language = clean_param(str_replace('-', '_', $language), PARAM_LANG); if (!empty($name)) { diff --git a/h5p/classes/api.php b/h5p/classes/api.php index eda8766641a..d2e9974dad2 100644 --- a/h5p/classes/api.php +++ b/h5p/classes/api.php @@ -163,7 +163,7 @@ class api { unset($library->major_version); $library->minorVersion = (int) $library->minorversion; unset($library->minorversion); - $library->metadataSettings = json_decode($library->metadatasettings); + $library->metadataSettings = json_decode($library->metadatasettings ?? ''); // If we already add this library means that it is an old version,as the previous query was sorted by version. if (isset($added[$library->name])) { diff --git a/h5p/classes/editor_framework.php b/h5p/classes/editor_framework.php index 0ab8b956338..45296c840fb 100644 --- a/h5p/classes/editor_framework.php +++ b/h5p/classes/editor_framework.php @@ -274,7 +274,7 @@ class editor_framework implements H5peditorStorage { if ($details) { $library->title = $details->title; $library->runnable = $details->runnable; - $library->metadataSettings = json_decode($details->metadatasettings); + $library->metadataSettings = json_decode($details->metadatasettings ?? ''); $library->example = $details->example; $library->tutorial = $details->tutorial; $librariesin[] = $library; diff --git a/lib/accesslib.php b/lib/accesslib.php index f5b72000891..f340b1f8641 100644 --- a/lib/accesslib.php +++ b/lib/accesslib.php @@ -4519,7 +4519,7 @@ function role_get_name(stdClass $role, $context = null, $rolenamedisplay = ROLEN } if ($rolenamedisplay == ROLENAME_ALIAS) { - if ($coursecontext and trim($role->coursealias) !== '') { + if ($coursecontext && $role->coursealias && trim($role->coursealias) !== '') { return format_string($role->coursealias, true, array('context'=>$coursecontext)); } else { return $original; @@ -4527,7 +4527,7 @@ function role_get_name(stdClass $role, $context = null, $rolenamedisplay = ROLEN } if ($rolenamedisplay == ROLENAME_BOTH) { - if ($coursecontext and trim($role->coursealias) !== '') { + if ($coursecontext && $role->coursealias && trim($role->coursealias) !== '') { return format_string($role->coursealias, true, array('context'=>$coursecontext)) . " ($original)"; } else { return $original; diff --git a/lib/behat/extension/Moodle/BehatExtension/Locator/FilesystemSkipPassedListLocator.php b/lib/behat/extension/Moodle/BehatExtension/Locator/FilesystemSkipPassedListLocator.php index 1a68338b77c..933d8a25703 100644 --- a/lib/behat/extension/Moodle/BehatExtension/Locator/FilesystemSkipPassedListLocator.php +++ b/lib/behat/extension/Moodle/BehatExtension/Locator/FilesystemSkipPassedListLocator.php @@ -64,7 +64,7 @@ final class FilesystemSkipPassedListLocator implements SpecificationLocator { * @return SpecificationIterator */ public function locateSpecifications(Suite $suite, $locator) { - if (!is_file($locator) || 'passed' !== pathinfo($locator, PATHINFO_EXTENSION)) { + if (!$locator || !is_file($locator) || 'passed' !== pathinfo($locator, PATHINFO_EXTENSION)) { return new NoSpecificationsIterator($suite); } diff --git a/lib/classes/component.php b/lib/classes/component.php index a7e8a6ce9a9..fb6fc9012b0 100644 --- a/lib/classes/component.php +++ b/lib/classes/component.php @@ -970,10 +970,9 @@ $cache = '.var_export($cache, true).'; } // Modules MUST NOT have any underscores, // component normalisation would break very badly otherwise! - return (bool)preg_match('/^[a-z][a-z0-9]*$/', $pluginname); - + return !is_null($pluginname) && (bool) preg_match('/^[a-z][a-z0-9]*$/', $pluginname); } else { - return (bool)preg_match('/^[a-z](?:[a-z0-9_](?!__))*[a-z0-9]+$/', $pluginname); + return !is_null($pluginname) && (bool) preg_match('/^[a-z](?:[a-z0-9_](?!__))*[a-z0-9]+$/', $pluginname); } } diff --git a/lib/classes/date.php b/lib/classes/date.php index 14e178ddddc..747a098faaa 100644 --- a/lib/classes/date.php +++ b/lib/classes/date.php @@ -319,7 +319,7 @@ class core_date { if (!defined('PHPUNIT_TEST')) { throw new coding_exception('core_date::phpunit_override_default_php_timezone() must be used only from unit tests'); } - $result = timezone_open($tz); // This triggers error if $tz invalid. + $result = timezone_open($tz ?? ''); // This triggers error if $tz invalid. if ($result !== false) { self::$defaultphptimezone = $tz; } else { diff --git a/lib/classes/output/mustache_filesystem_loader.php b/lib/classes/output/mustache_filesystem_loader.php index f90623ce47a..e79b479fed5 100644 --- a/lib/classes/output/mustache_filesystem_loader.php +++ b/lib/classes/output/mustache_filesystem_loader.php @@ -53,4 +53,16 @@ class mustache_filesystem_loader extends \Mustache_Loader_FilesystemLoader { // Call the Moodle template finder. return mustache_template_finder::get_template_filepath($name); } + + /** + * Only check if baseDir is a directory and requested templates are files if + * baseDir is using the filesystem stream wrapper. + * + * Always check path for mustache_filesystem_loader. + * + * @return bool Whether to check `is_dir` and `file_exists` + */ + protected function shouldCheckPath() { + return true; + } } diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php index a61b4f89dec..247f371949c 100644 --- a/lib/dml/pgsql_native_moodle_database.php +++ b/lib/dml/pgsql_native_moodle_database.php @@ -594,7 +594,7 @@ class pgsql_native_moodle_database extends moodle_database { } else if (preg_match('/int(\d)/i', $rawcolumn->type, $matches)) { $info->type = 'int'; - if (strpos($rawcolumn->adsrc, 'nextval') === 0) { + if (strpos($rawcolumn->adsrc ?? '', 'nextval') === 0) { $info->primary_key = true; $info->meta_type = 'R'; $info->unique = true; diff --git a/lib/evalmath/evalmath.class.php b/lib/evalmath/evalmath.class.php index 26e204bb73d..7e1a8790b30 100644 --- a/lib/evalmath/evalmath.class.php +++ b/lib/evalmath/evalmath.class.php @@ -260,7 +260,7 @@ class EvalMath { if (is_null($o2)) return $this->trigger(get_string('unexpectedclosingbracket', 'mathslib')); else $output[] = $o2; } - if (preg_match('/^('.self::$namepat.')\($/', $stack->last(2), $matches)) { // did we just close a function? + if (preg_match('/^('.self::$namepat.')\($/', $stack->last(2) ?? '', $matches)) { // did we just close a function? $fnn = $matches[1]; // get the function name $arg_count = $stack->pop(); // see how many arguments there were (cleverly stored on the stack, thank you) $fn = $stack->pop(); diff --git a/lib/evalmath/readme_moodle.txt b/lib/evalmath/readme_moodle.txt index c34b31d4241..b40f83b79f0 100644 --- a/lib/evalmath/readme_moodle.txt +++ b/lib/evalmath/readme_moodle.txt @@ -13,6 +13,7 @@ Our changes: * added round, ceil and floor functions. * EvalMath::EvalMath() changed to EvalMath::__construct() and there is a new EvalMath::EvalMath function to maintain backwards compatibility +* Ensure a string is passed to preg_match in EvalMath::nfx. To see all changes diff against version 1.1, available from: http://www.phpclasses.org/browse/package/2695.html @@ -27,4 +28,4 @@ Changes by Stefan Erlachner, Thomas Niedermaier (MDL-64414): * add function or: e.g. if (or(condition_1, condition_2, ... condition_n)) * add function and: -e.g. if (and(condition_1, condition_2, ... condition_n)) \ No newline at end of file +e.g. if (and(condition_1, condition_2, ... condition_n)) diff --git a/lib/filebrowser/file_info.php b/lib/filebrowser/file_info.php index b7dcc46b47c..2127d1f4f60 100644 --- a/lib/filebrowser/file_info.php +++ b/lib/filebrowser/file_info.php @@ -100,8 +100,8 @@ abstract class file_info { */ protected function build_search_files_sql($extensions, $prefix = null) { global $DB; - if (strlen($prefix)) { - $prefix = $prefix.'.'; + if ($prefix && strlen($prefix)) { + $prefix = $prefix . '.'; } else { $prefix = ''; } diff --git a/lib/filelib.php b/lib/filelib.php index 2fda3478737..514fb8aa3a0 100644 --- a/lib/filelib.php +++ b/lib/filelib.php @@ -518,9 +518,9 @@ function file_rewrite_pluginfile_urls($text, $file, $contextid, $component, $fil } if (!empty($options['reverse'])) { - return str_replace($baseurl, '@@PLUGINFILE@@/', $text); + return str_replace($baseurl, '@@PLUGINFILE@@/', $text ?? ''); } else { - return str_replace('@@PLUGINFILE@@/', $baseurl, $text); + return str_replace('@@PLUGINFILE@@/', $baseurl, $text ?? ''); } } @@ -785,7 +785,7 @@ function file_get_drafarea_files($draftitemid, $filepath = '/') { } // find the file this draft file was created from and count all references in local // system pointing to that file - $source = @unserialize($file->get_source()); + $source = @unserialize($file->get_source() ?? ''); if (isset($source->original)) { $item->refcount = $fs->search_references_count($source->original); } @@ -905,7 +905,7 @@ function file_get_submitted_draft_itemid($elname) { * @return stored_file */ function file_restore_source_field_from_draft_file($storedfile) { - $source = @unserialize($storedfile->get_source()); + $source = @unserialize($storedfile->get_source() ?? ''); if (!empty($source)) { if (is_object($source)) { $restoredsource = $source->source; @@ -1199,7 +1199,7 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea // Let's check if we can update this file or we need to delete and create. if ($newfile->is_directory()) { // Directories are always ok to just update. - } else if (($source = @unserialize($newfile->get_source())) && isset($source->original)) { + } else if (($source = @unserialize($newfile->get_source() ?? '')) && isset($source->original)) { // File has the 'original' - we need to update the file (it may even have not been changed at all). $original = file_storage::unpack_reference($source->original); if ($original['filename'] !== $oldfile->get_filename() || $original['filepath'] !== $oldfile->get_filepath()) { @@ -1235,7 +1235,7 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea // Field files.source for draftarea files contains serialised object with source and original information. // We only store the source part of it for non-draft file area. $newsource = $newfile->get_source(); - if ($source = @unserialize($newfile->get_source())) { + if ($source = @unserialize($newfile->get_source() ?? '')) { $newsource = $source->source; } if ($oldfile->get_source() !== $newsource) { @@ -1269,7 +1269,7 @@ function file_save_draft_area_files($draftitemid, $contextid, $component, $filea // the size and subdirectory tests are extra safety only, the UI should prevent it foreach ($newhashes as $file) { $file_record = array('contextid'=>$contextid, 'component'=>$component, 'filearea'=>$filearea, 'itemid'=>$itemid, 'timemodified'=>time()); - if ($source = @unserialize($file->get_source())) { + if ($source = @unserialize($file->get_source() ?? '')) { // Field files.source for draftarea files contains serialised object with source and original information. // We only store the source part of it for non-draft file area. $file_record['source'] = $source->source; @@ -1478,7 +1478,7 @@ function format_postdata_for_curlcall($postdata) { $currentdata = urlencode($k); format_array_postdata_for_curlcall($v, $currentdata, $data); } else { - $data[] = urlencode($k).'='.urlencode($v); + $data[] = urlencode($k).'='.urlencode($v ?? ''); } } $convertedpostdata = implode('&', $data); @@ -1771,7 +1771,7 @@ function mimeinfo($element, $filename) { $mimeinfo = & get_mimetypes_array(); static $iconpostfixes = array(256=>'-256', 128=>'-128', 96=>'-96', 80=>'-80', 72=>'-72', 64=>'-64', 48=>'-48', 32=>'-32', 24=>'-24', 16=>''); - $filetype = strtolower(pathinfo($filename, PATHINFO_EXTENSION)); + $filetype = strtolower(pathinfo($filename ?? '', PATHINFO_EXTENSION)); if (empty($filetype)) { $filetype = 'xxx'; // file without extension } @@ -2028,8 +2028,8 @@ function get_mimetype_description($obj, $capitalise=false) { } // MIME types may include + symbol but this is not permitted in string ids. - $safemimetype = str_replace('+', '_', $mimetype); - $safemimetypestr = str_replace('+', '_', $mimetypestr); + $safemimetype = str_replace('+', '_', $mimetype ?? ''); + $safemimetypestr = str_replace('+', '_', $mimetypestr ?? ''); $customdescription = mimeinfo('customdescription', $filename); if ($customdescription) { // Call format_string on the custom description so that multilang @@ -2919,7 +2919,7 @@ function file_overwrite_existing_draftfile(stored_file $newfile, stored_file $ex $fs = get_file_storage(); // Remember original file source field. - $source = @unserialize($existingfile->get_source()); + $source = @unserialize($existingfile->get_source() ?? ''); // Remember the original sortorder. $sortorder = $existingfile->get_sortorder(); if ($newfile->is_external_file()) { @@ -2943,7 +2943,7 @@ function file_overwrite_existing_draftfile(stored_file $newfile, stored_file $ex $newfile = $fs->create_file_from_storedfile($newfilerecord, $newfile); // Preserve original file location (stored in source field) for handling references. if (isset($source->original)) { - if (!($newfilesource = @unserialize($newfile->get_source()))) { + if (!($newfilesource = @unserialize($newfile->get_source() ?? ''))) { $newfilesource = new stdClass(); } $newfilesource->original = $source->original; diff --git a/lib/filestorage/file_storage.php b/lib/filestorage/file_storage.php index 71361d665d3..e3a852271ff 100644 --- a/lib/filestorage/file_storage.php +++ b/lib/filestorage/file_storage.php @@ -1800,7 +1800,7 @@ class file_storage { // the latter of which can go to 100, we need to make sure that quality here is // in a safe range or PHP WILL CRASH AND DIE. You have been warned. $quality = $quality > 9 ? (int)(max(1.0, (float)$quality / 100.0) * 9.0) : $quality; - imagepng($img, NULL, $quality, NULL); + imagepng($img, null, $quality, PNG_NO_FILTER); break; default: @@ -2462,6 +2462,6 @@ class file_storage { * @return string The file's content hash */ public static function hash_from_string($content) { - return sha1($content); + return sha1($content ?? ''); } } diff --git a/lib/filestorage/file_system_filedir.php b/lib/filestorage/file_system_filedir.php index 8f9eb56e089..19418514937 100644 --- a/lib/filestorage/file_system_filedir.php +++ b/lib/filestorage/file_system_filedir.php @@ -426,7 +426,7 @@ class file_system_filedir extends file_system { $contenthash = file_storage::hash_from_string($content); // Binary length. - $filesize = strlen($content); + $filesize = strlen($content ?? ''); $hashpath = $this->get_fulldir_from_hash($contenthash); $hashfile = $this->get_local_path_from_hash($contenthash, false); diff --git a/lib/filestorage/tgz_extractor.php b/lib/filestorage/tgz_extractor.php index 0fb40ba38a0..4d1bc370fb2 100644 --- a/lib/filestorage/tgz_extractor.php +++ b/lib/filestorage/tgz_extractor.php @@ -493,7 +493,7 @@ class tgz_extractor { unlink($this->currentfile); } else { // For index file, get number of files and delete temp file. - $contents = file_get_contents($this->currentfile, null, null, null, 128); + $contents = file_get_contents($this->currentfile, false, null, 0, 128); $matches = array(); if (preg_match('~^' . preg_quote(tgz_packer::ARCHIVE_INDEX_COUNT_PREFIX) . '([0-9]+)~', $contents, $matches)) { diff --git a/lib/form/classes/filetypes_util.php b/lib/form/classes/filetypes_util.php index 6dc4c74ea8d..f11791bd1b7 100644 --- a/lib/form/classes/filetypes_util.php +++ b/lib/form/classes/filetypes_util.php @@ -62,7 +62,7 @@ class filetypes_util { */ public function normalize_file_types($types) { - if ($types === '') { + if ($types === '' || $types === null) { return []; } diff --git a/lib/form/passwordunmask.php b/lib/form/passwordunmask.php index e90ee4df89f..f82d12f4131 100644 --- a/lib/form/passwordunmask.php +++ b/lib/form/passwordunmask.php @@ -86,7 +86,7 @@ class MoodleQuickForm_passwordunmask extends MoodleQuickForm_password { */ public function export_for_template(renderer_base $output) { $context = parent::export_for_template($output); - $context['valuechars'] = array_fill(0, strlen($context['value']), 'x'); + $context['valuechars'] = array_fill(0, strlen($context['value'] ?? ''), 'x'); return $context; } diff --git a/lib/formslib.php b/lib/formslib.php index 7995f206d6d..4bb23d26df1 100644 --- a/lib/formslib.php +++ b/lib/formslib.php @@ -3261,7 +3261,7 @@ class MoodleQuickForm_Renderer extends HTML_QuickForm_Renderer_Tableless{ $html = str_replace('{type}', $element->getType(), $html); $html = str_replace('{name}', $element->getName(), $html); $html = str_replace('{groupname}', '', $html); - $html = str_replace('{class}', $element->getAttribute('class'), $html); + $html = str_replace('{class}', $element->getAttribute('class') ?? '', $html); $emptylabel = ''; if ($element->getLabel() == '') { $emptylabel = 'femptylabel'; diff --git a/lib/html2text/lib.php b/lib/html2text/lib.php index 18972d4c700..6c4588e22a6 100644 --- a/lib/html2text/lib.php +++ b/lib/html2text/lib.php @@ -52,7 +52,7 @@ class core_html2text extends \Html2Text\Html2Text { */ function __construct($html = '', $options = array()) { // Call the parent constructor. - parent::__construct($html, $options); + parent::__construct($html ?? '', $options); // MDL-27736: Trailing spaces before newline or tab. $this->entSearch[] = '/[ ]+([\n\t])/'; diff --git a/lib/mathslib.php b/lib/mathslib.php index 8753695cc16..6cee99a3e32 100644 --- a/lib/mathslib.php +++ b/lib/mathslib.php @@ -123,7 +123,7 @@ class calc_formula { * @return string localised formula */ public static function localize($formula) { - $formula = str_replace('.', '$', $formula); // temp placeholder + $formula = str_replace('.', '$', $formula ?? ''); // Temp placeholder. $formula = str_replace(',', get_string('listsep', 'langconfig'), $formula); $formula = str_replace('$', get_string('decsep', 'langconfig'), $formula); return $formula; diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 2209a3efaf6..eb5f5dcd730 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -5836,7 +5836,7 @@ class navigation_json { } if ($child->forcetitle || $child->title !== $child->text) { - $attributes['title'] = htmlentities($child->title, ENT_QUOTES, 'UTF-8'); + $attributes['title'] = htmlentities($child->title ?? '', ENT_QUOTES, 'UTF-8'); } if (array_key_exists($child->key.':'.$child->type, $this->expandable)) { $attributes['expandable'] = $child->key; diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php index 850d4b2bb75..7ad7e852f2b 100644 --- a/lib/outputcomponents.php +++ b/lib/outputcomponents.php @@ -2352,7 +2352,7 @@ class html_writer { if (!is_null($for)) { $attributes = array_merge($attributes, array('for' => $for)); } - $text = trim($text); + $text = trim($text ?? ''); $label = self::tag('label', $text, $attributes); // TODO MDL-12192 $colonize disabled for now yet diff --git a/lib/outputlib.php b/lib/outputlib.php index 6dc24b30054..1bbd202dfd2 100644 --- a/lib/outputlib.php +++ b/lib/outputlib.php @@ -1854,7 +1854,7 @@ class theme_config { // Now resolve all theme settings or do any other postprocessing. // This needs to be done before calling core parser, since the parser strips [[settings]] tags. $csspostprocess = $this->csspostprocess; - if (function_exists($csspostprocess)) { + if ($csspostprocess && function_exists($csspostprocess)) { $css = $csspostprocess($css, $this); } diff --git a/lib/outputrequirementslib.php b/lib/outputrequirementslib.php index 07428ae29b8..109814a3f7e 100644 --- a/lib/outputrequirementslib.php +++ b/lib/outputrequirementslib.php @@ -720,7 +720,7 @@ class page_requirements_manager { if ($url instanceof moodle_url) { return $url; - } else if (strpos($url, '/') === 0) { + } else if (null !== $url && strpos($url, '/') === 0) { // Fix the admin links if needed. if ($CFG->admin !== 'admin') { if (strpos($url, "/admin/") === 0) { diff --git a/lib/setuplib.php b/lib/setuplib.php index 10e002a89bb..6e7c6f0f366 100644 --- a/lib/setuplib.php +++ b/lib/setuplib.php @@ -634,6 +634,7 @@ function get_docs_url($path = null) { $path = ''; } + $path = $path ?? ''; // Absolute URLs are used unmodified. if (substr($path, 0, 7) === 'http://' || substr($path, 0, 8) === 'https://') { return $path; diff --git a/lib/tablelib.php b/lib/tablelib.php index f81bb10717d..b9b593a7ffa 100644 --- a/lib/tablelib.php +++ b/lib/tablelib.php @@ -573,7 +573,7 @@ class flexible_table { global $SESSION; if (isset($SESSION->flextable[$uniqueid])) { $prefs = $SESSION->flextable[$uniqueid]; - } else if (!$prefs = json_decode(get_user_preferences('flextable_' . $uniqueid), true)) { + } else if (!$prefs = json_decode(get_user_preferences("flextable_{$uniqueid}", ''), true)) { return ''; } @@ -1463,7 +1463,7 @@ class flexible_table { // Load any existing user preferences. if ($this->persistent) { - $this->prefs = json_decode(get_user_preferences('flextable_' . $this->uniqueid), true); + $this->prefs = json_decode(get_user_preferences("flextable_{$this->uniqueid}", ''), true); $oldprefs = $this->prefs; } else if (isset($SESSION->flextable[$this->uniqueid])) { $this->prefs = $SESSION->flextable[$this->uniqueid]; @@ -2346,4 +2346,3 @@ class table_dataformat_export_format extends table_default_export_format_parent exit(); } } - diff --git a/lib/tests/behat/behat_accessibility.php b/lib/tests/behat/behat_accessibility.php index 7aa8d426f3f..022a7aed7f1 100644 --- a/lib/tests/behat/behat_accessibility.php +++ b/lib/tests/behat/behat_accessibility.php @@ -131,7 +131,7 @@ return (() => { EOF; for ($i = 0; $i < self::get_extended_timeout() * 10; $i++) { - $results = json_decode($this->evaluate_script($getresults)); + $results = json_decode($this->evaluate_script($getresults) ?? ''); if ($results) { break; } diff --git a/lib/tests/component_test.php b/lib/tests/component_test.php index 631fd27643b..2d0cc23632c 100644 --- a/lib/tests/component_test.php +++ b/lib/tests/component_test.php @@ -22,13 +22,6 @@ * @copyright 2013 Petr Skoda {@link http://skodak.org} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ - -defined('MOODLE_INTERNAL') || die(); - - -/** - * Class core_component_testcase. - */ class component_test extends advanced_testcase { /** @@ -217,33 +210,58 @@ class component_test extends advanced_testcase { } } - public function test_is_valid_plugin_name() { - $this->assertTrue(core_component::is_valid_plugin_name('mod', 'example1')); - $this->assertTrue(core_component::is_valid_plugin_name('mod', 'feedback360')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'feedback_360')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', '2feedback')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', '1example')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example.xx')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', '.example')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', '_example')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example_')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example_x1')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example-x1')); - $this->assertFalse(core_component::is_valid_plugin_name('mod', 'role')); + /** + * Test that the get_plugin_list_with_file() function returns the correct list of plugins. + * + * @covers \core_component::is_valid_plugin_name + * @dataProvider is_valid_plugin_name_provider + * @param array $arguments + * @param bool $expected + */ + public function test_is_valid_plugin_name(array $arguments, bool $expected): void { + $this->assertEquals($expected, core_component::is_valid_plugin_name(...$arguments)); + } - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example1')); - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example_x1')); - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example_x1_xxx')); - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'feedback360')); - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'feed_back360')); - $this->assertTrue(core_component::is_valid_plugin_name('tool', 'role')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', '1example')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example.xx')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example-xx')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', '.example')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', '_example')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example_')); - $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example__x1')); + /** + * Data provider for the is_valid_plugin_name function. + * + * @return array + */ + public function is_valid_plugin_name_provider(): array { + return [ + [['mod', 'example1'], true], + [['mod', 'feedback360'], true], + [['mod', 'feedback_360'], false], + [['mod', '2feedback'], false], + [['mod', '1example'], false], + [['mod', 'example.xx'], false], + [['mod', '.example'], false], + [['mod', '_example'], false], + [['mod', 'example_'], false], + [['mod', 'example_x1'], false], + [['mod', 'example-x1'], false], + [['mod', 'role'], false], + + [['tool', 'example1'], true], + [['tool', 'example_x1'], true], + [['tool', 'example_x1_xxx'], true], + [['tool', 'feedback360'], true], + [['tool', 'feed_back360'], true], + [['tool', 'role'], true], + [['tool', '1example'], false], + [['tool', 'example.xx'], false], + [['tool', 'example-xx'], false], + [['tool', '.example'], false], + [['tool', '_example'], false], + [['tool', 'example_'], false], + [['tool', 'example__x1'], false], + + // Some invalid cases. + [['mod', null], false], + [['mod', ''], false], + [['tool', null], false], + [['tool', ''], false], + ]; } public function test_normalize_componentname() { diff --git a/lib/tests/weblib_test.php b/lib/tests/weblib_test.php index 6b93afa3f1b..8e4f5606764 100644 --- a/lib/tests/weblib_test.php +++ b/lib/tests/weblib_test.php @@ -23,9 +23,7 @@ * @author T.J.Hunt@open.ac.uk * @license http://www.gnu.org/copyleft/gpl.html GNU Public License */ - class weblib_test extends advanced_testcase { - /** * @covers ::format_string */ @@ -985,4 +983,32 @@ EXPECTED; public function test_get_html_lang_attribute_value(string $langcode, string $expected): void { $this->assertEquals($expected, get_html_lang_attribute_value($langcode)); } + + /** + * Data provider for strip_querystring tests. + * + * @return array + */ + public function strip_querystring_provider(): array { + return [ + 'Null' => [null, ''], + 'Empty string' => ['', ''], + 'No querystring' => ['https://example.com', 'https://example.com'], + 'Querystring' => ['https://example.com?foo=bar', 'https://example.com'], + 'Querystring with fragment' => ['https://example.com?foo=bar#baz', 'https://example.com'], + 'Querystring with fragment and path' => ['https://example.com/foo/bar?foo=bar#baz', 'https://example.com/foo/bar'], + ]; + } + + /** + * Test the strip_querystring function with various exampels. + * + * @dataProvider strip_querystring_provider + * @param mixed $value + * @param mixed $expected + * @covers ::strip_querystring + */ + public function test_strip_querystring($value, $expected): void { + $this->assertEquals($expected, strip_querystring($value)); + } } diff --git a/lib/weblib.php b/lib/weblib.php index 488c717ee6e..45b00ef4e69 100644 --- a/lib/weblib.php +++ b/lib/weblib.php @@ -91,13 +91,18 @@ define('URL_MATCH_EXACT', 2); * @return string */ function s($var) { - if ($var === false) { return '0'; } - return preg_replace('/&#(\d+|x[0-9a-f]+);/i', '&#$1;', - htmlspecialchars($var, ENT_QUOTES | ENT_HTML401 | ENT_SUBSTITUTE)); + if ($var === null || $var === '') { + return ''; + } + + return preg_replace( + '/&#(\d+|x[0-9a-f]+);/i', '&#$1;', + htmlspecialchars($var, ENT_QUOTES | ENT_HTML401 | ENT_SUBSTITUTE) + ); } /** @@ -148,6 +153,9 @@ function addslashes_js($var) { * @return string The remaining URL. */ function strip_querystring($url) { + if ($url === null || $url === '') { + return ''; + } if ($commapos = strpos($url, '?')) { return substr($url, 0, $commapos); @@ -336,6 +344,7 @@ class moodle_url { $this->anchor = $url->anchor; } else { + $url = $url ?? ''; // Detect if anchor used. $apos = strpos($url, '#'); if ($apos !== false) { @@ -1115,7 +1124,7 @@ function validate_email($address) { require_once("{$CFG->libdir}/phpmailer/moodle_phpmailer.php"); - return moodle_phpmailer::validateAddress($address) && !preg_match('/[<>]/', $address); + return moodle_phpmailer::validateAddress($address ?? '') && !preg_match('/[<>]/', $address); } /** @@ -1528,7 +1537,7 @@ function format_string($string, $striplinks = true, $options = null) { * @return string */ function replace_ampersands_not_followed_by_entity($string) { - return preg_replace("/\&(?![a-zA-Z0-9#]{1,8};)/", "&", $string); + return preg_replace("/\&(?![a-zA-Z0-9#]{1,8};)/", "&", $string ?? ''); } /** diff --git a/media/player/youtube/classes/plugin.php b/media/player/youtube/classes/plugin.php index dd1478981e5..9537ae94138 100644 --- a/media/player/youtube/classes/plugin.php +++ b/media/player/youtube/classes/plugin.php @@ -63,7 +63,7 @@ class media_youtube_plugin extends core_media_player_external { protected function embed_external(moodle_url $url, $name, $width, $height, $options) { - $info = trim($name); + $info = trim($name ?? ''); if (empty($info) or strpos($info, 'http') === 0) { $info = get_string('pluginname', 'media_youtube'); } @@ -125,7 +125,7 @@ OET; if (is_numeric($rawtime)) { // Start time already specified as a number of seconds; ensure it's an integer. $seconds = $rawtime; - } else if (preg_match('/(\d+?h)?(\d+?m)?(\d+?s)?/i', $rawtime, $matches)) { + } else if (preg_match('/(\d+?h)?(\d+?m)?(\d+?s)?/i', $rawtime ?? '', $matches)) { // Convert into a raw number of seconds, as that's all embedded players accept. for ($i = 1; $i < count($matches); $i++) { if (empty($matches[$i])) { diff --git a/mod/data/classes/preset.php b/mod/data/classes/preset.php index fdeef6d8182..8f2c5ec1935 100644 --- a/mod/data/classes/preset.php +++ b/mod/data/classes/preset.php @@ -310,7 +310,7 @@ class preset { $instance = $this->manager->get_instance(); foreach (manager::TEMPLATES_LIST as $templatename => $templatefilename) { $templatefile = fopen("$exportdir/$templatefilename", 'w'); - fwrite($templatefile, $instance->{$templatename}); + fwrite($templatefile, $instance->{$templatename} ?? ''); fclose($templatefile); } @@ -710,7 +710,7 @@ class preset { $presetxmldata = "\n\n"; // Add description. - $presetxmldata .= '' . htmlspecialchars($this->description, ENT_COMPAT) . "\n\n"; + $presetxmldata .= '' . htmlspecialchars($this->description ?? '', ENT_COMPAT) . "\n\n"; // Add settings. // Raw settings are not preprocessed during saving of presets. diff --git a/mod/data/classes/search/entry.php b/mod/data/classes/search/entry.php index 95ee8a0a95c..7cf20be4d71 100644 --- a/mod/data/classes/search/entry.php +++ b/mod/data/classes/search/entry.php @@ -304,7 +304,7 @@ class entry extends \core_search\base_mod { continue; } $content->priority = $classname::get_priority(); - $content->addtemplateposition = strpos($template, '[['.$content->fldname.']]'); + $content->addtemplateposition = strpos($template ?? '', '[['.$content->fldname.']]'); } $orderqueue = new \SPLPriorityQueue(); diff --git a/mod/data/field/latlong/field.class.php b/mod/data/field/latlong/field.class.php index 19d094ee2fd..4d25d261ecc 100644 --- a/mod/data/field/latlong/field.class.php +++ b/mod/data/field/latlong/field.class.php @@ -170,11 +170,11 @@ class data_field_latlong extends data_field_base { } $lat = $content->content; - if (strlen($lat) < 1) { + if (strlen($lat ?? '') < 1) { return ''; } $long = $content->content1; - if (strlen($long) < 1) { + if (strlen($long ?? '') < 1) { return ''; } // We use format_float to display in the regional format. @@ -254,7 +254,7 @@ class data_field_latlong extends data_field_base { // When updating these values (which might be region formatted) we should format // the float to allow for a consistent float format in the database. $value = unformat_float($value); - $value = trim($value); + $value = trim($value ?? ''); if (strlen($value) > 0) { $value = floatval($value); } else { diff --git a/mod/data/field/number/field.class.php b/mod/data/field/number/field.class.php index df75243ad8f..e3053c8ba5b 100644 --- a/mod/data/field/number/field.class.php +++ b/mod/data/field/number/field.class.php @@ -68,7 +68,7 @@ class data_field_number extends data_field_base { return ''; } $number = $content->content; - $decimals = trim($this->field->param1); + $decimals = trim($this->field->param1 ?? ''); // Only apply number formatting if param1 contains an integer number >= 0. if (preg_match("/^\d+$/", $decimals)) { $decimals = $decimals * 1; diff --git a/mod/data/lib.php b/mod/data/lib.php index 355483a30f7..9ab3f3c45a8 100644 --- a/mod/data/lib.php +++ b/mod/data/lib.php @@ -840,19 +840,19 @@ function data_replace_field_in_templates($data, $searchfieldname, $newfieldname) $newdata = new stdClass(); $newdata->id = $data->id; $newdata->singletemplate = str_ireplace('[['.$searchfieldname.']]', - $prestring.$newfieldname.$poststring, $data->singletemplate); + $prestring.$newfieldname.$poststring, $data->singletemplate ?? ''); $newdata->listtemplate = str_ireplace('[['.$searchfieldname.']]', - $prestring.$newfieldname.$poststring, $data->listtemplate); + $prestring.$newfieldname.$poststring, $data->listtemplate ?? ''); $newdata->addtemplate = str_ireplace('[['.$searchfieldname.']]', - $prestring.$newfieldname.$poststring, $data->addtemplate); + $prestring.$newfieldname.$poststring, $data->addtemplate ?? ''); $newdata->addtemplate = str_ireplace('[['.$searchfieldname.'#id]]', - $prestring.$newfieldname.$idpart.$poststring, $data->addtemplate); + $prestring.$newfieldname.$idpart.$poststring, $data->addtemplate ?? ''); $newdata->rsstemplate = str_ireplace('[['.$searchfieldname.']]', - $prestring.$newfieldname.$poststring, $data->rsstemplate); + $prestring.$newfieldname.$poststring, $data->rsstemplate ?? ''); return $DB->update_record('data', $newdata); } diff --git a/mod/h5pactivity/classes/xapi/handler.php b/mod/h5pactivity/classes/xapi/handler.php index b05a9102377..358f6a52eea 100644 --- a/mod/h5pactivity/classes/xapi/handler.php +++ b/mod/h5pactivity/classes/xapi/handler.php @@ -83,7 +83,7 @@ class handler extends handler_base { // H5P add some extra params to ID to define subcontents. $parts = explode('?', $xapiobject, 2); $contextid = array_shift($parts); - $subcontent = str_replace('subContentId=', '', array_shift($parts)); + $subcontent = str_replace('subContentId=', '', array_shift($parts) ?? ''); if (empty($contextid) || !is_numeric($contextid)) { return null; } diff --git a/mod/lti/lib.php b/mod/lti/lib.php index bfe7cd71e28..c2a74c11e99 100644 --- a/mod/lti/lib.php +++ b/mod/lti/lib.php @@ -318,7 +318,7 @@ function mod_lti_get_all_content_items(\core_course\local\entity\content_item $d $type->name = 'lti_type_' . $ltitype->id; // Clean the name. We don't want tags here. $type->title = clean_param($ltitype->name, PARAM_NOTAGS); - $trimmeddescription = trim($ltitype->description); + $trimmeddescription = trim($ltitype->description ?? ''); $type->help = ''; if ($trimmeddescription != '') { // Clean the description. We don't want tags here. diff --git a/mod/lti/locallib.php b/mod/lti/locallib.php index 7bd5073ebd4..0a1ef823d57 100644 --- a/mod/lti/locallib.php +++ b/mod/lti/locallib.php @@ -2437,7 +2437,7 @@ function lti_get_configured_types($courseid, $sectionreturn = 0) { $type->name = 'lti_type_' . $ltitype->id; // Clean the name. We don't want tags here. $type->title = clean_param($ltitype->name, PARAM_NOTAGS); - $trimmeddescription = trim($ltitype->description); + $trimmeddescription = trim($ltitype->description ?? ''); if ($trimmeddescription != '') { // Clean the description. We don't want tags here. $type->help = clean_param($trimmeddescription, PARAM_NOTAGS); @@ -2454,7 +2454,7 @@ function lti_get_configured_types($courseid, $sectionreturn = 0) { function lti_get_domain_from_url($url) { $matches = array(); - if (preg_match(LTI_URL_DOMAIN_REGEX, $url, $matches)) { + if (preg_match(LTI_URL_DOMAIN_REGEX, $url ?? '', $matches)) { return $matches[1]; } } diff --git a/mod/quiz/accessrule/seb/classes/quiz_settings.php b/mod/quiz/accessrule/seb/classes/quiz_settings.php index 923fdb33d6d..a1512ad832b 100644 --- a/mod/quiz/accessrule/seb/classes/quiz_settings.php +++ b/mod/quiz/accessrule/seb/classes/quiz_settings.php @@ -662,7 +662,7 @@ class quiz_settings extends persistent { * @return array of string, the separate keys. */ private function split_keys($keys) : array { - $keys = preg_split('~[ \t\n\r,;]+~', $keys, -1, PREG_SPLIT_NO_EMPTY); + $keys = preg_split('~[ \t\n\r,;]+~', $keys ?? '', -1, PREG_SPLIT_NO_EMPTY); foreach ($keys as $i => $key) { $keys[$i] = strtolower($key); } diff --git a/mod/survey/classes/privacy/provider.php b/mod/survey/classes/privacy/provider.php index b545e4b7883..d31ffb99c1b 100644 --- a/mod/survey/classes/privacy/provider.php +++ b/mod/survey/classes/privacy/provider.php @@ -227,7 +227,7 @@ class provider implements 'options' => $record->qoptions ]); $qtype = $record->qtype; - $options = explode(',', $q->options); + $options = explode(',', $q->options ?? ''); $carry[] = [ 'question' => array_merge((array) $q, [ diff --git a/mod/workshop/classes/portfolio_caller.php b/mod/workshop/classes/portfolio_caller.php index 740a041822e..aaaad77f630 100644 --- a/mod/workshop/classes/portfolio_caller.php +++ b/mod/workshop/classes/portfolio_caller.php @@ -342,7 +342,7 @@ class mod_workshop_portfolio_caller extends portfolio_module_caller_base { } if ($this->workshop->overallfeedbackmode) { - if ($assessment->feedbackauthorattachment or trim($assessment->feedbackauthor) !== '') { + if ($assessment->feedbackauthorattachment || trim($assessment->feedbackauthor ?? '') !== '') { $output .= html_writer::tag('h3', get_string('overallfeedback', 'mod_workshop')); $content = $this->format_exported_text($assessment->feedbackauthor, $assessment->feedbackauthorformat); $content = portfolio_rewrite_pluginfile_urls($content, $this->workshop->context->id, 'mod_workshop', diff --git a/mod/workshop/view.php b/mod/workshop/view.php index 67b8bec38de..c8957a23108 100644 --- a/mod/workshop/view.php +++ b/mod/workshop/view.php @@ -695,7 +695,7 @@ switch ($workshop->phase) { echo $output->render($workshop->prepare_submission_summary($submission, $shownames)); echo $output->box_end(); - if (strlen(trim($assessment->feedbackreviewer)) > 0) { + if (!empty($assessment->feedbackreviewer) && strlen(trim($assessment->feedbackreviewer)) > 0) { echo $output->render(new workshop_feedback_reviewer($assessment)); } } diff --git a/privacy/classes/local/request/moodle_content_writer.php b/privacy/classes/local/request/moodle_content_writer.php index 88c95c434bc..a8d893b8776 100644 --- a/privacy/classes/local/request/moodle_content_writer.php +++ b/privacy/classes/local/request/moodle_content_writer.php @@ -14,17 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * This file contains the moodle format implementation of the content writer. - * - * @package core_privacy - * @copyright 2018 Andrew Nicols - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ namespace core_privacy\local\request; -defined('MOODLE_INTERNAL') || die(); - /** * The moodle_content_writer is the default Moodle implementation of a content writer. * @@ -33,6 +24,7 @@ defined('MOODLE_INTERNAL') || die(); * * Objects of data are stored as JSON. * + * @package core_privacy * @copyright 2018 Andrew Nicols * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ @@ -168,7 +160,10 @@ class moodle_content_writer implements content_writer { * @param string $text The text to be processed * @return string The processed string */ - public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string { + public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text): string { + if ($text === null || $text === '') { + return ''; + } // Need to take into consideration the subcontext to provide the full path to this file. $subcontextpath = ''; if (!empty($subcontext)) { diff --git a/privacy/classes/tests/request/content_writer.php b/privacy/classes/tests/request/content_writer.php index 7b2b9b85847..3a8c64e7e30 100644 --- a/privacy/classes/tests/request/content_writer.php +++ b/privacy/classes/tests/request/content_writer.php @@ -381,8 +381,8 @@ class content_writer implements \core_privacy\local\request\content_writer { * @param string $text The text to be processed * @return string The processed string */ - public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text) : string { - return str_replace('@@PLUGINFILE@@/', 'files/', $text); + public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text): string { + return str_replace('@@PLUGINFILE@@/', 'files/', $text ?? ''); } /** diff --git a/privacy/tests/moodle_content_writer_test.php b/privacy/tests/moodle_content_writer_test.php index d40736d1d4f..e2d8de216fe 100644 --- a/privacy/tests/moodle_content_writer_test.php +++ b/privacy/tests/moodle_content_writer_test.php @@ -1292,6 +1292,18 @@ class moodle_content_writer_test extends advanced_testcase { */ public function rewrite_pluginfile_urls_provider() { return [ + 'nullcontent' => [ + 'intro', + 0, + null, + '', + ], + 'emptycontent' => [ + 'intro', + 0, + '', + '', + ], 'zeroitemid' => [ 'intro', 0, diff --git a/question/engine/renderer.php b/question/engine/renderer.php index 4ce0e7c193c..b2f77b7e5f9 100644 --- a/question/engine/renderer.php +++ b/question/engine/renderer.php @@ -149,7 +149,7 @@ class core_question_renderer extends plugin_renderer_base { * @return HTML fragment. */ protected function number($number) { - if (trim($number) === '') { + if (trim($number ?? '') === '') { return ''; } $numbertext = ''; diff --git a/question/engine/tests/coverage.php b/question/engine/tests/coverage.php new file mode 100644 index 00000000000..2589273d3dc --- /dev/null +++ b/question/engine/tests/coverage.php @@ -0,0 +1,37 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the question_engine. + * + * @copyright 2022 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of files relative to the plugin root to include in coverage generation. */ + protected $includelistfiles = [ + 'bank.php', + 'datalib.php', + 'lib.php', + 'questionattempt.php', + 'questionattemptstep.php', + 'questionusage.php', + 'renderer.php', + 'states.php', + ]; +}; diff --git a/question/engine/tests/helpers.php b/question/engine/tests/helpers.php index 33d9894c01a..c7da45ae584 100644 --- a/question/engine/tests/helpers.php +++ b/question/engine/tests/helpers.php @@ -1391,22 +1391,3 @@ class question_test_recordset extends moodle_recordset { $this->records = null; } } - -/** - * Helper class for tests that help to test core_question_renderer. - * - * @copyright 2018 Huong Nguyen - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ -class testable_core_question_renderer extends core_question_renderer { - - /** - * Test the private number function. - * - * @param null|string $number - * @return HTML - */ - public function number($number) { - return parent::number($number); - } -} diff --git a/question/engine/tests/question_engine_test.php b/question/engine/tests/question_engine_test.php index a46be4b1fc7..edb2aa9fcf8 100644 --- a/question/engine/tests/question_engine_test.php +++ b/question/engine/tests/question_engine_test.php @@ -28,18 +28,18 @@ namespace core_question; use advanced_testcase; use moodle_exception; use question_engine; -use testable_core_question_renderer; /** * Unit tests for the question_engine class. * * @copyright 2009 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @coversDefaultClass \question_engine */ class question_engine_test extends advanced_testcase { /** - * Load required libraries + * Load required libraries. */ public static function setUpBeforeClass(): void { global $CFG; @@ -47,113 +47,294 @@ class question_engine_test extends advanced_testcase { require_once("{$CFG->dirroot}/question/engine/lib.php"); } - public function test_load_behaviour_class() { - // Exercise SUT + /** + * Tests for load_behaviour_class. + * + * @covers \question_engine::load_behaviour_class + */ + public function test_load_behaviour_class(): void { + // Exercise SUT. question_engine::load_behaviour_class('deferredfeedback'); - // Verify + + // Verify. $this->assertTrue(class_exists('qbehaviour_deferredfeedback')); } - public function test_load_behaviour_class_missing() { - // Exercise SUT + /** + * Tests for load_behaviour_class when a class is missing. + * + * @covers \question_engine::load_behaviour_class + */ + public function test_load_behaviour_class_missing(): void { + // Exercise SUT. $this->expectException(moodle_exception::class); question_engine::load_behaviour_class('nonexistantbehaviour'); } - public function test_get_behaviour_unused_display_options() { - $this->assertEquals(array(), question_engine::get_behaviour_unused_display_options('interactive')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('deferredfeedback')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('deferredcbm')); - $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'), - question_engine::get_behaviour_unused_display_options('manualgraded')); + /** + * Test the get_behaviour_unused_display_options with various options. + * + * @covers \question_engine::get_behaviour_unused_display_options + * @dataProvider get_behaviour_unused_display_options_provider + * @param string $behaviour + * @param array $expected + */ + public function test_get_behaviour_unused_display_options(string $behaviour, array $expected): void { + $this->assertEquals($expected, question_engine::get_behaviour_unused_display_options($behaviour)); } - public function test_can_questions_finish_during_the_attempt() { - $this->assertFalse(question_engine::can_questions_finish_during_the_attempt('deferredfeedback')); - $this->assertTrue(question_engine::can_questions_finish_during_the_attempt('interactive')); + /** + * Data provider for get_behaviour_unused_display_options. + * + * @return array + */ + public function get_behaviour_unused_display_options_provider(): array { + return [ + 'interactive' => [ + 'interactive', + [], + ], + 'deferredfeedback' => [ + 'deferredfeedback', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + 'deferredcbm' => [ + 'deferredcbm', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + 'manualgraded' => [ + 'manualgraded', + ['correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'], + ], + ]; } - public function test_sort_behaviours() { - $in = array('b1' => 'Behave 1', 'b2' => 'Behave 2', 'b3' => 'Behave 3', 'b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - - $out = array('b1' => 'Behave 1', 'b2' => 'Behave 2', 'b3' => 'Behave 3', 'b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, '', '', '')); - - $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b4', 'b4')); - - $out = array('b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b1,b2,b3,b4', 'b4')); - - $out = array('b6' => 'Behave 6', 'b1' => 'Behave 1', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b1,b4', 'b2,b3,b4,b5', 'b4')); - - $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b5,b4', 'b1,b2,b3', 'b4')); - - $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b1,b6,b5', 'b1,b2,b3,b4', 'b4')); - - $out = array('b2' => 'Behave 2', 'b4' => 'Behave 4', 'b6' => 'Behave 6'); - $this->assertSame($out, question_engine::sort_behaviours($in, 'b2,b4,b6', 'b1,b3,b5', 'b2')); - - // Ignore unknown input in the order argument. - $this->assertSame($in, question_engine::sort_behaviours($in, 'unknown', '', '')); - - // Ignore unknown input in the disabled argument. - $this->assertSame($in, question_engine::sort_behaviours($in, '', 'unknown', '')); + /** + * Tests for can_questions_finish_during_the_attempt. + * + * @covers \question_engine::can_questions_finish_during_the_attempt + * @dataProvider can_questions_finish_during_the_attempt_provider + * @param string $behaviour + * @param bool $expected + */ + public function test_can_questions_finish_during_the_attempt(string $behaviour, bool $expected): void { + $this->assertEquals($expected, question_engine::can_questions_finish_during_the_attempt($behaviour)); } - public function test_is_manual_grade_in_range() { - $_POST[] = array('q1:2_-mark' => 0.5, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => 0, 'q1:2_:maxfraction' => 1); + /** + * Data provider for can_questions_finish_during_the_attempt_provider. + * + * @return array + */ + public function can_questions_finish_during_the_attempt_provider(): array { + return [ + ['deferredfeedback', false], + ['interactive', true], + ]; + } + + /** + * Tests for sort_behaviours + * + * @covers \question_engine::sort_behaviours + * @dataProvider sort_behaviours_provider + * @param array $input The params passed to sort_behaviours + * @param array $expected + */ + public function test_sort_behaviours(array $input, array $expected): void { + $this->assertSame($expected, question_engine::sort_behaviours(...$input)); + } + + /** + * Data provider for sort_behaviours. + * + * @return array + */ + public function sort_behaviours_provider(): array { + $in = [ + 'b1' => 'Behave 1', + 'b2' => 'Behave 2', + 'b3' => 'Behave 3', + 'b4' => 'Behave 4', + 'b5' => 'Behave 5', + 'b6' => 'Behave 6', + ]; + + return [ + [ + [$in, '', '', ''], + $in, + ], + [ + [$in, '', 'b4', 'b4'], + $in, + ], + [ + [$in, '', 'b1,b2,b3,b4', 'b4'], + ['b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6'], + ], + [ + [$in, 'b6,b1,b4', 'b2,b3,b4,b5', 'b4'], + ['b6' => 'Behave 6', 'b1' => 'Behave 1', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b6,b5,b4', 'b1,b2,b3', 'b4'], + ['b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b1,b6,b5', 'b1,b2,b3,b4', 'b4'], + ['b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4'], + ], + [ + [$in, 'b2,b4,b6', 'b1,b3,b5', 'b2'], + ['b2' => 'Behave 2', 'b4' => 'Behave 4', 'b6' => 'Behave 6'], + ], + // Ignore unknown input in the order argument. + [ + [$in, 'unknown', '', ''], + $in, + ], + // Ignore unknown input in the disabled argument. + [ + [$in, '', 'unknown', ''], + $in, + ], + ]; + } + + /** + * Tests for is_manual_grade_in_range. + * + * @dataProvider is_manual_grade_in_range_provider + * @covers \question_engine::is_manual_grade_in_range + * @param array $post The values to add to $_POST + * @param array $params The params to pass to is_manual_grade_in_range + * @param bool $expected + */ + public function test_is_manual_grade_in_range(array $post, array $params, bool $expected): void { + $_POST[] = $post; + $this->assertEquals($expected, question_engine::is_manual_grade_in_range(...$params)); + } + + /** + * Data provider for is_manual_grade_in_range tests. + * + * @return array + */ + public function is_manual_grade_in_range_provider(): array { + return [ + 'In range' => [ + 'post' => [ + 'q1:2_-mark' => 0.5, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => 0, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Bottom end' => [ + 'post' => [ + 'q1:2_-mark' => -1.0, + 'q1:2_-maxmark' => 2.0, + 'q1:2_:minfraction' => -0.5, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Too low' => [ + 'post' => [ + 'q1:2_-mark' => -1.1, + 'q1:2_-maxmark' => 2.0, + 'q1:2_:minfraction' => -0.5, + 'q1:2_:maxfraction' => 1, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Top end' => [ + 'post' => [ + 'q1:2_-mark' => 3.0, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => -6.0, + 'q1:2_:maxfraction' => 3.0, + ], + 'range' => [1, 2], + 'expected' => true, + ], + 'Too high' => [ + 'post' => [ + 'q1:2_-mark' => 3.1, + 'q1:2_-maxmark' => 1.0, + 'q1:2_:minfraction' => -6.0, + 'q1:2_:maxfraction' => 3.0, + ], + 'range' => [1, 2], + 'expected' => true, + ], + ]; + } + + /** + * Tests for is_manual_grade_in_range. + * + * @covers \question_engine::is_manual_grade_in_range + */ + public function test_is_manual_grade_in_range_ungraded(): void { $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); } - public function test_is_manual_grade_in_range_bottom_end() { - $_POST[] = array('q1:2_-mark' => -1.0, 'q1:2_-maxmark' => 2.0, - 'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); + /** + * Ensure that the number renderer performs as expected. + * + * @covers \core_question_renderer::number + * @dataProvider render_question_number_provider + * @param mixed $value + * @param string $expected + */ + public function test_render_question_number($value, string $expected): void { + global $PAGE; + + $renderer = new \core_question_renderer($PAGE, 'core_question'); + $rc = new \ReflectionClass($renderer); + $rcm = $rc->getMethod('number'); + $rcm->setAccessible(true); + + $this->assertEquals($expected, $rcm->invoke($renderer, $value)); } - public function test_is_manual_grade_in_range_too_low() { - $_POST[] = array('q1:2_-mark' => -1.1, 'q1:2_-maxmark' => 2.0, - 'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_top_end() { - $_POST[] = array('q1:2_-mark' => 3.0, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => -6.0, 'q1:2_:maxfraction' => 3.0); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_too_high() { - $_POST[] = array('q1:2_-mark' => 3.1, 'q1:2_-maxmark' => 1.0, - 'q1:2_:minfraction' => -6.0, 'q1:2_:maxfraction' => 3.0); - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_is_manual_grade_in_range_ungraded() { - $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2)); - } - - public function test_render_question_number() { - global $CFG, $PAGE; - - require_once("{$CFG->dirroot}/question/engine/tests/helpers.php"); - $renderer = new testable_core_question_renderer($PAGE, 'core_question'); - - // Test with number is i character. - $this->assertEquals('

Information

', $renderer->number('i')); - // Test with number is empty string. - $this->assertEquals('', $renderer->number('')); - // Test with number is 0. - $this->assertEquals('

Question 0

', $renderer->number(0)); - // Test with number is numeric. - $this->assertEquals('

Question 1

', $renderer->number(1)); - // Test with number is string. - $this->assertEquals('

Question 1 of 2

', $renderer->number('1 of 2')); + /** + * Data provider for test_render_question_number. + * + * @return array + */ + public function render_question_number_provider(): array { + return [ + 'Test with number is i character' => [ + 'i', + '

Information

', + ], + 'Test with number is empty string' => [ + '', + '', + ], + 'Test with null' => [ + null, + '', + ], + 'Test with number is 0' => [ + 0, + '

Question 0

', + ], + 'Test with number is numeric' => [ + 1, + '

Question 1

', + ], + 'Test with number is string' => [ + '1 of 2', + '

Question 1 of 2

', + ], + ]; } } diff --git a/question/tests/coverage.php b/question/tests/coverage.php new file mode 100644 index 00000000000..4bcdaa33a5e --- /dev/null +++ b/question/tests/coverage.php @@ -0,0 +1,40 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the core_question. + * + * @copyright 2022 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of files relative to the plugin root to include in coverage generation. */ + protected $includelistfiles = [ + 'category_class.php', + 'category_form.php', + 'editlib.php', + 'export_form.php', + 'format.php', + 'import_form.php', + 'lib.php', + 'move_form.php', + 'previewlib.php', + 'renderer.php', + 'upgrade.php', + ]; +}; diff --git a/question/type/calculated/db/upgradelib.php b/question/type/calculated/db/upgradelib.php index 09803be12d7..c01b8ae6344 100644 --- a/question/type/calculated/db/upgradelib.php +++ b/question/type/calculated/db/upgradelib.php @@ -14,47 +14,35 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Upgrade library code for the calculated question type. - * - * @package qtype - * @subpackage calculated - * @copyright 2011 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - - -defined('MOODLE_INTERNAL') || die(); - - /** * Class for converting attempt data for calculated questions when upgrading * attempts to the new question engine. * * This class is used by the code in question/engine/upgrade/upgradelib.php. * + * @package qtype_calculated * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_updater { protected $selecteditem = null; /** @var array variable name => value */ - protected $values; + protected $values = []; /** @var array variable names wrapped in {...}. Used by {@link substitute_values()}. */ - protected $search; + protected $search = []; /** * @var array variable values, with negative numbers wrapped in (...). * Used by {@link substitute_values()}. */ - protected $safevalue; + protected $safevalue = []; /** * @var array variable values, with negative numbers wrapped in (...). * Used by {@link substitute_values()}. */ - protected $prettyvalue; + protected $prettyvalue = []; public function question_summary() { return ''; // Done later, after we know which dataset is used. @@ -86,7 +74,7 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update throw new coding_exception("Brokes state {$state->id} for calculated question {$state->question}. (It did not specify a dataset."); } - list($datasetbit, $realanswer) = explode('-', $state->answer, 2); + [$datasetbit, $realanswer] = explode('-', $state->answer, 2); $selecteditem = substr($datasetbit, 7); if (is_null($this->selecteditem)) { @@ -98,21 +86,21 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update } if (!$realanswer) { - return array('', ''); + return ['', '']; } if (strpos($realanswer, '|||||') === false) { $answer = $realanswer; $unit = ''; } else { - list($answer, $unit) = explode('|||||', $realanswer, 2); + [$answer, $unit] = explode('|||||', $realanswer, 2); } - return array($answer, $unit); + return [$answer, $unit]; } public function response_summary($state) { - list($answer, $unit) = $this->parse_response($state); + [$answer, $unit] = $this->parse_response($state); if (empty($answer) && empty($unit)) { $resp = null; @@ -157,9 +145,11 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update return; } - list($answer, $unit) = $this->parse_response($state); - if (!empty($this->question->options->showunits) && - $this->question->options->showunits == 1) { + [$answer, $unit] = $this->parse_response($state); + if ( + !empty($this->question->options->showunits) && + $this->question->options->showunits == 1 + ) { // Multichoice units. $data['answer'] = $answer; $data['unit'] = $unit; @@ -185,9 +175,9 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update $this->question->id, $selecteditem); // Prepare an array for {@link substitute_values()}. - $this->search = array(); - $this->safevalue = array(); - $this->prettyvalue = array(); + $this->search = []; + $this->safevalue = []; + $this->prettyvalue = []; foreach ($this->values as $name => $value) { if (!is_numeric($value)) { $a = new stdClass(); @@ -270,7 +260,7 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update * corresponding value. */ protected function substitute_values_for_eval($expression) { - return str_replace($this->search, $this->safevalue, $expression); + return str_replace($this->search, $this->safevalue, $expression ?? ''); } /** @@ -282,7 +272,7 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update * corresponding value. */ protected function substitute_values_pretty($text) { - return str_replace($this->search, $this->prettyvalue, $text); + return str_replace($this->search, $this->prettyvalue, $text ?? ''); } /** @@ -292,11 +282,17 @@ class qtype_calculated_qe2_attempt_updater extends question_qtype_attempt_update * @return string the text with values substituted. */ public function replace_expressions_in_text($text, $length = null, $format = null) { + if ($text === null || $text === '') { + return $text; + } $vs = $this; // Can't see to use $this in a PHP closure. - $text = preg_replace_callback('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~', - function ($matches) use ($vs, $format, $length) { - return $vs->format_float($vs->calculate($matches[1]), $length, $format); - }, $text); + $text = preg_replace_callback( + '~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~', + function ($matches) use ($vs, $format, $length) { + return $vs->format_float($vs->calculate($matches[1]), $length, $format); + }, + $text + ); return $this->substitute_values_pretty($text); } } diff --git a/question/type/calculatedmulti/db/upgradelib.php b/question/type/calculatedmulti/db/upgradelib.php index d17d43c1aa6..9beb0c82a0d 100644 --- a/question/type/calculatedmulti/db/upgradelib.php +++ b/question/type/calculatedmulti/db/upgradelib.php @@ -14,47 +14,35 @@ // You should have received a copy of the GNU General Public License // along with Moodle. If not, see . -/** - * Upgrade library code for the calculated multiple-choice question type. - * - * @package qtype - * @subpackage calculatedmulti - * @copyright 2011 The Open University - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later - */ - - -defined('MOODLE_INTERNAL') || die(); - - /** * Class for converting attempt data for calculated multiple-choice questions * when upgrading attempts to the new question engine. * * This class is used by the code in question/engine/upgrade/upgradelib.php. * + * @package qtype_calculatedmulti * @copyright 2011 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_updater { protected $selecteditem = null; /** @var array variable name => value */ - protected $values; + protected $values = []; /** @var array variable names wrapped in {...}. Used by {@link substitute_values()}. */ - protected $search; + protected $search = []; /** * @var array variable values, with negative numbers wrapped in (...). * Used by {@link substitute_values()}. */ - protected $safevalue; + protected $safevalue = []; /** * @var array variable values, with negative numbers wrapped in (...). * Used by {@link substitute_values()}. */ - protected $prettyvalue; + protected $prettyvalue = []; protected $order; @@ -69,9 +57,8 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u return $this->to_text($this->replace_expressions_in_text($ans->answer)); } } - } else { - $rightbits = array(); + $rightbits = []; foreach ($this->question->options->answers as $ans) { if ($ans->fraction >= 0.000001) { $rightbits[] = $this->to_text($this->replace_expressions_in_text($ans->answer)); @@ -87,7 +74,7 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u throw new coding_exception("Brokes state {$state->id} for calcluatedmulti question {$state->question}. (It did not specify a dataset."); } - list($datasetbit, $answer) = explode('-', $state->answer, 2); + [$datasetbit, $answer] = explode('-', $state->answer, 2); $selecteditem = substr($datasetbit, 7); if (is_null($this->selecteditem)) { @@ -99,7 +86,7 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u } if (strpos($answer, ':') !== false) { - list($order, $responses) = explode(':', $answer); + [$order, $responses] = explode(':', $answer); return $responses; } else { // Sometimes, a bug means that a state is missing the : bit, @@ -116,7 +103,8 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u if (is_numeric($responses)) { if (array_key_exists($responses, $this->question->options->answers)) { return $this->to_text($this->replace_expressions_in_text( - $this->question->options->answers[$responses]->answer)); + $this->question->options->answers[$responses]->answer + )); } else { $this->logger->log_assumption("Dealing with a place where the student selected a choice that was later deleted for @@ -126,15 +114,15 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u } else { return null; } - } else { if (!empty($responses)) { $responses = explode(',', $responses); - $bits = array(); + $bits = []; foreach ($responses as $response) { if (array_key_exists($response, $this->question->options->answers)) { $bits[] = $this->to_text($this->replace_expressions_in_text( - $this->question->options->answers[$response]->answer)); + $this->question->options->answers[$response]->answer + )); } else { $this->logger->log_assumption("Dealing with a place where the student selected a choice that was later deleted for @@ -161,15 +149,16 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u public function set_first_step_data_elements($state, &$data) { $this->explode_answer($state); $this->updater->qa->questionsummary = $this->to_text( - $this->replace_expressions_in_text($this->question->questiontext)); + $this->replace_expressions_in_text($this->question->questiontext) + ); $this->updater->qa->rightanswer = $this->right_answer($this->question); foreach ($this->values as $name => $value) { $data['_var_' . $name] = $value; } - list($datasetbit, $answer) = explode('-', $state->answer, 2); - list($order, $responses) = explode(':', $answer); + [$datasetbit, $answer] = explode('-', $state->answer, 2); + [$order, $responses] = explode(':', $answer); $data['_order'] = $order; $this->order = explode(',', $order); } @@ -189,7 +178,6 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u $data['answer'] = '-1'; } } - } else { $responses = explode(',', $responses); foreach ($this->order as $key => $ansid) { @@ -206,12 +194,14 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u $this->selecteditem = $selecteditem; $this->updater->qa->variant = $selecteditem; $this->values = $this->qeupdater->load_dataset( - $this->question->id, $selecteditem); + $this->question->id, + $selecteditem + ); // Prepare an array for {@link substitute_values()}. - $this->search = array(); - $this->safevalue = array(); - $this->prettyvalue = array(); + $this->search = []; + $this->safevalue = []; + $this->prettyvalue = []; foreach ($this->values as $name => $value) { if (!is_numeric($value)) { $a = new stdClass(); @@ -294,7 +284,7 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u * corresponding value. */ protected function substitute_values_for_eval($expression) { - return str_replace($this->search, $this->safevalue, $expression); + return str_replace($this->search, $this->safevalue, $expression ?? ''); } /** @@ -306,7 +296,7 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u * corresponding value. */ protected function substitute_values_pretty($text) { - return str_replace($this->search, $this->prettyvalue, $text); + return str_replace($this->search, $this->prettyvalue, $text ?? ''); } /** @@ -316,11 +306,18 @@ class qtype_calculatedmulti_qe2_attempt_updater extends question_qtype_attempt_u * @return string the text with values substituted. */ public function replace_expressions_in_text($text, $length = null, $format = null) { + if ($text === null || $text === '') { + return $text; + } + $vs = $this; // Can't see to use $this in a PHP closure. - $text = preg_replace_callback(qtype_calculated::FORMULAS_IN_TEXT_REGEX, - function ($matches) use ($vs, $format, $length) { - return $vs->format_float($vs->calculate($matches[1]), $length, $format); - }, $text); + $text = preg_replace_callback( + qtype_calculated::FORMULAS_IN_TEXT_REGEX, + function ($matches) use ($vs, $format, $length) { + return $vs->format_float($vs->calculate($matches[1]), $length, $format); + }, + $text + ); return $this->substitute_values_pretty($text); } } diff --git a/question/type/multianswer/renderer.php b/question/type/multianswer/renderer.php index aa4a444bec3..a935ed9bd5a 100644 --- a/question/type/multianswer/renderer.php +++ b/question/type/multianswer/renderer.php @@ -235,7 +235,7 @@ class qtype_multianswer_textfield_renderer extends qtype_multianswer_subq_render } // Work out a good input field size. - $size = max(1, core_text::strlen(trim($response)) + 1); + $size = max(1, core_text::strlen(trim($response ?? '')) + 1); foreach ($subq->answers as $ans) { $size = max($size, core_text::strlen(trim($ans->answer))); } diff --git a/question/type/numerical/questiontype.php b/question/type/numerical/questiontype.php index ccf62b1bea6..37d29dafbb7 100644 --- a/question/type/numerical/questiontype.php +++ b/question/type/numerical/questiontype.php @@ -647,10 +647,14 @@ class qtype_numerical_answer_processor { * default unit, by using the given unit multiplier. * * @param string $response a value, optionally with a unit. - * @return array(numeric, sting) the value with the unit stripped, and normalised + * @return array(numeric, string, multiplier) the value with the unit stripped, and normalised * by the unit multiplier, if any, and the unit string, for reference. */ - public function apply_units($response, $separateunit = null) { + public function apply_units($response, $separateunit = null): array { + if ($response === null || trim($response) === '') { + return [null, null, null]; + } + // Strip spaces (which may be thousands separators) and change other forms // of writing e to e. $response = str_replace(' ', '', $response); diff --git a/question/type/numerical/tests/answerprocessor_test.php b/question/type/numerical/tests/answerprocessor_test.php index e50bfc56829..dd764665b5d 100644 --- a/question/type/numerical/tests/answerprocessor_test.php +++ b/question/type/numerical/tests/answerprocessor_test.php @@ -27,17 +27,6 @@ namespace qtype_numerical; use qtype_numerical_answer_processor; -defined('MOODLE_INTERNAL') || die(); - -global $CFG; -require_once($CFG->dirroot . '/question/type/numerical/questiontype.php'); - -class testable_qtype_numerical_answer_processor extends qtype_numerical_answer_processor { - public function parse_response($response) { - return parent::parse_response($response); - } -} - /** * Unit test for the numerical questions answers processor. * @@ -45,60 +34,109 @@ class testable_qtype_numerical_answer_processor extends qtype_numerical_answer_p * @category test * @copyright 2008 The Open University * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @covers \qtype_numerical_answer_processor */ class answerprocessor_test extends \advanced_testcase { - public function test_parse_response() { - $ap = new testable_qtype_numerical_answer_processor( - array('m' => 1, 'cm' => 100), false, '.', ','); + /** + * Test setup. + */ + public function setUp(): void { + global $CFG; - $this->assertEquals(array('3', '142', '', ''), $ap->parse_response('3.142')); - $this->assertEquals(array('', '2', '', ''), $ap->parse_response('.2')); - $this->assertEquals(array('1', '', '', ''), $ap->parse_response('1.')); - $this->assertEquals(array('1', '0', '', ''), $ap->parse_response('1.0')); - $this->assertEquals(array('-1', '', '', ''), $ap->parse_response('-1.')); - $this->assertEquals(array('+1', '0', '', ''), $ap->parse_response('+1.0')); - - $this->assertEquals(array('1', '', '4', ''), $ap->parse_response('1e4')); - $this->assertEquals(array('3', '142', '-4', ''), $ap->parse_response('3.142E-4')); - $this->assertEquals(array('', '2', '+2', ''), $ap->parse_response('.2e+2')); - $this->assertEquals(array('1', '', '-1', ''), $ap->parse_response('1.e-1')); - $this->assertEquals(array('1', '0', '0', ''), $ap->parse_response('1.0e0')); - - $this->assertEquals(array('3', '', '8', ''), $ap->parse_response('3x10^8')); - $this->assertEquals(array('3', '', '8', ''), $ap->parse_response('3×10^8')); - $this->assertEquals(array('3', '0', '8', ''), $ap->parse_response('3.0*10^8')); - $this->assertEquals(array('3', '00', '-8', ''), $ap->parse_response('3.00x10**-8')); - $this->assertEquals(array('0', '001', '7', ''), $ap->parse_response('0.001×10**7')); - - $this->assertEquals(array('1', '', '', 'm'), $ap->parse_response('1m')); - $this->assertEquals(array('3', '142', '', 'm'), $ap->parse_response('3.142 m')); - $this->assertEquals(array('', '2', '', 'm'), $ap->parse_response('.2m')); - $this->assertEquals(array('1', '', '', 'cm'), $ap->parse_response('1.cm')); - $this->assertEquals(array('1', '0', '', 'cm'), $ap->parse_response('1.0 cm')); - $this->assertEquals(array('-1', '', '', 'm'), $ap->parse_response('-1.m')); - $this->assertEquals(array('+1', '0', '', 'cm'), $ap->parse_response('+1.0cm')); - - $this->assertEquals(array('1', '', '4', 'm'), $ap->parse_response('1e4 m')); - $this->assertEquals(array('3', '142', '-4', 'cm'), $ap->parse_response('3.142E-4 cm')); - $this->assertEquals(array('', '2', '+2', 'm'), $ap->parse_response('.2e+2m')); - $this->assertEquals(array('1', '', '-1', 'm'), $ap->parse_response('1.e-1 m')); - $this->assertEquals(array('1', '0', '0', 'cm'), $ap->parse_response('1.0e0cm')); - - $this->assertEquals(array('1000000', '', '', ''), - $ap->parse_response('1,000,000')); - $this->assertEquals(array('1000', '00', '', 'm'), - $ap->parse_response('1,000.00 m')); - - $this->assertEquals(array(null, null, null, null), $ap->parse_response('frog')); - $this->assertEquals(array('3', '', '', 'frogs'), $ap->parse_response('3 frogs')); - $this->assertEquals(array(null, null, null, null), $ap->parse_response('. m')); - $this->assertEquals(array(null, null, null, null), $ap->parse_response('.e8 m')); - $this->assertEquals(array(null, null, null, null), $ap->parse_response(',')); + require_once("{$CFG->dirroot}/question/type/numerical/questiontype.php"); } - protected function verify_value_and_unit($exectedval, $expectedunit, $expectedmultiplier, - qtype_numerical_answer_processor $ap, $input, $separateunit = null) { - list($val, $unit, $multiplier) = $ap->apply_units($input, $separateunit); + /** + * Test the parse_response function. + * + * @covers ::parse_response + * @dataProvider parse_response_provider + * @param array $expected + * @param mixed $args + */ + public function test_parse_response(array $expected, $args): void { + $ap = new qtype_numerical_answer_processor([ + 'm' => 1, + 'cm' => 100, + ], false, '.', ','); + + $rc = new \ReflectionClass($ap); + $rcm = $rc->getMethod('parse_response'); + $rcm->setAccessible(true); + + $this->assertEquals($expected, $rcm->invoke($ap, $args)); + } + + /** + * Data provider for the parse_response function. + * + * @return array + */ + public function parse_response_provider(): array { + return [ + [['3', '142', '', ''], '3.142'], + [['', '2', '', ''], '.2'], + [['1', '', '', ''], '1.'], + [['1', '0', '', ''], '1.0'], + [['-1', '', '', ''], '-1.'], + [['+1', '0', '', ''], '+1.0'], + + [['1', '', '4', ''], '1e4'], + [['3', '142', '-4', ''], '3.142E-4'], + [['', '2', '+2', ''], '.2e+2'], + [['1', '', '-1', ''], '1.e-1'], + [['1', '0', '0', ''], '1.0e0'], + + [['3', '', '8', ''], '3x10^8'], + [['3', '', '8', ''], '3×10^8'], + [['3', '0', '8', ''], '3.0*10^8'], + [['3', '00', '-8', ''], '3.00x10**-8'], + [['0', '001', '7', ''], '0.001×10**7'], + + [['1', '', '', 'm'], '1m'], + [['3', '142', '', 'm'], '3.142 m'], + [['', '2', '', 'm'], '.2m'], + [['1', '', '', 'cm'], '1.cm'], + [['1', '0', '', 'cm'], '1.0 cm'], + [['-1', '', '', 'm'], '-1.m'], + [['+1', '0', '', 'cm'], '+1.0cm'], + + [['1', '', '4', 'm'], '1e4 m'], + [['3', '142', '-4', 'cm'], '3.142E-4 cm'], + [['', '2', '+2', 'm'], '.2e+2m'], + [['1', '', '-1', 'm'], '1.e-1 m'], + [['1', '0', '0', 'cm'], '1.0e0cm'], + + [['1000000', '', '', ''], '1,000,000'], + [['1000', '00', '', 'm'], '1,000.00 m'], + + [[null, null, null, null], 'frog'], + [['3', '', '', 'frogs'], '3 frogs'], + [[null, null, null, null], '. m'], + [[null, null, null, null], '.e8 m'], + [[null, null, null, null], ','], + ]; + } + + /** + * Call apply_units and verify the value and units returned. + * + * @param int|float $exectedval + * @param null|string $expectedunit + * @param int|float $expectedmultiplier + * @param qtype_numerical_answer_processor $ap + * @param null|int|float $input + * @param null|string $separateunit + */ + protected function verify_value_and_unit( + $exectedval, + $expectedunit, + $expectedmultiplier, + qtype_numerical_answer_processor $ap, + $input, + $separateunit = null + ): void { + [$val, $unit, $multiplier] = $ap->apply_units($input, $separateunit); if (is_null($exectedval)) { $this->assertNull($val); } else { @@ -112,57 +150,198 @@ class answerprocessor_test extends \advanced_testcase { } } - public function test_apply_units() { + /** + * Test the apply_units function with various parameters. + * + * @covers \qtype_numerical_answer_processor::apply_units + * @dataProvider apply_units_provider + * @param mixed $expectedvalue + * @param string|null $expectedunit + * @param float|int|null $expectedmultiplier + * @param string|null $input + */ + public function test_apply_units( + $expectedvalue, + $expectedunit, + $expectedmultiplier, + $input + ): void { $ap = new qtype_numerical_answer_processor( - array('m/s' => 1, 'c' => 3.3356409519815E-9, - 'mph' => 2.2369362920544), false, '.', ','); + [ + 'm/s' => 1, + 'c' => 3.3356409519815E-9, + 'mph' => 2.2369362920544 + ], + false, + '.', + ',' + ); - $this->verify_value_and_unit(3e8, 'm/s', 1, $ap, '3x10^8 m/s'); - $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8'); - $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1c'); - $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000 mph'); - - $this->verify_value_and_unit(1, 'frogs', null, $ap, '1 frogs'); - $this->verify_value_and_unit(null, null, null, $ap, '. m/s'); + $this->verify_value_and_unit( + $expectedvalue, + $expectedunit, + $expectedmultiplier, + $ap, + $input + ); } - public function test_apply_units_separate_unit() { + /** + * Data provider for apply_units tests. + * + * @return array + */ + public function apply_units_provider(): array { + return [ + [3e8, 'm/s', 1, '3x10^8 m/s'], + [3e8, '', null, '3x10^8'], + [1, 'c', 299792458, '1c'], + [1, 'mph', 0.44704, '0001.000 mph'], + + [1, 'frogs', null, '1 frogs'], + [null, null, null, '. m/s'], + [null, null, null, null], + [null, null, null, ''], + [null, null, null, ' '], + ]; + } + + /** + * Test the apply_units function with various parameters and different units. + * + * @covers \qtype_numerical_answer_processor::apply_units + * @dataProvider apply_units_provider_with_units + * @param mixed $expectedvalue + * @param string|null $expectedunit + * @param float|int|null $expectedmultiplier + * @param string|null $input + * @param string $units + */ + public function test_apply_units_with_unit( + $expectedvalue, + $expectedunit, + $expectedmultiplier, + $input, + $units + ): void { $ap = new qtype_numerical_answer_processor( - array('m/s' => 1, 'c' => 3.3356409519815E-9, - 'mph' => 2.2369362920544), false, '.', ','); + [ + 'm/s' => 1, + 'c' => 3.3356409519815E-9, + 'mph' => 2.2369362920544 + ], + false, + '.', + ',' + ); - $this->verify_value_and_unit(3e8, 'm/s', 1, $ap, '3x10^8', 'm/s'); - $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8', ''); - $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1', 'c'); - $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000', 'mph'); - - $this->verify_value_and_unit(1, 'frogs', null, $ap, '1', 'frogs'); - $this->verify_value_and_unit(null, null, null, $ap, '.', 'm/s'); + $this->verify_value_and_unit( + $expectedvalue, + $expectedunit, + $expectedmultiplier, + $ap, + $input, + $units + ); } - public function test_euro_style() { - $ap = new qtype_numerical_answer_processor(array(), false, ',', ' '); + /** + * Data provider for apply_units with different units. + * + * @return array + */ + public function apply_units_provider_with_units(): array { + return [ + [3e8, 'm/s', 1, '3x10^8', 'm/s'], + [3e8, '', null, '3x10^8', ''], + [1, 'c', 299792458, '1', 'c'], + [1, 'mph', 0.44704, '0001.000', 'mph'], - $this->assertEquals(array(-1000, '', null), $ap->apply_units('-1 000')); - $this->assertEquals(array(3.14159, '', null), $ap->apply_units('3,14159')); + [1, 'frogs', null, '1', 'frogs'], + [null, null, null, '.', 'm/s'], + ]; } - public function test_percent() { - $ap = new qtype_numerical_answer_processor(array('%' => 100), false, '.', ','); - - $this->assertEquals(array('3', '%', 0.01), $ap->apply_units('3%')); - $this->assertEquals(array('1e-6', '%', 0.01), $ap->apply_units('1e-6 %')); - $this->assertEquals(array('100', '', null), $ap->apply_units('100')); + /** + * Test apply_units with a comma float unit. + * + * @covers \qtype_numerical_answer_processor::apply_units + * @dataProvider euro_provider + * @param array $expected + * @param string $params + */ + public function test_euro_style(array $expected, string $params): void { + $ap = new qtype_numerical_answer_processor([], false, ',', ' '); + $this->assertEquals($expected, $ap->apply_units($params)); } - public function test_currency() { - $ap = new qtype_numerical_answer_processor(array('$' => 1, '£' => 1), true, '.', ','); + /** + * Data provider for apply_units with euro float separators. + * + * return array + */ + public function euro_provider(): array { + return [ + [[-1000, '', null], '-1 000'], + [[3.14159, '', null], '3,14159'], + ]; + } - $this->assertEquals(array('1234.56', '£', 1), $ap->apply_units('£1,234.56')); - $this->assertEquals(array('100', '$', 1), $ap->apply_units('$100')); - $this->assertEquals(array('100', '$', 1), $ap->apply_units('$100.')); - $this->assertEquals(array('100.00', '$', 1), $ap->apply_units('$100.00')); - $this->assertEquals(array('100', '', null), $ap->apply_units('100')); - $this->assertEquals(array('100', 'frog', null), $ap->apply_units('frog 100')); + /** + * Test apply_units with percentage values. + * + * @covers \qtype_numerical_answer_processor::apply_units + * @dataProvider percent_provider + * @param array $expected + * @param string $params + */ + public function test_percent(array $expected, string $params): void { + $ap = new qtype_numerical_answer_processor(['%' => 100], false, '.', ','); + $this->assertEquals($expected, $ap->apply_units($params)); + } + + /** + * Data provider for apply_units with percentages. + * + * @return array + */ + public function percent_provider(): array { + return [ + [['3', '%', 0.01], '3%'], + [['1e-6', '%', 0.01], '1e-6 %'], + [['100', '', null], '100'], + ]; + } + + /** + * Test apply_units with currency values. + * + * @covers \qtype_numerical_answer_processor::apply_units + * @dataProvider currency_provider + * @param array $expected + * @param string $params + */ + public function test_currency(array $expected, string $params): void { + $ap = new qtype_numerical_answer_processor([ + '$' => 1, + '£' => 1, + ], true, '.', ','); + $this->assertEquals($expected, $ap->apply_units($params)); + } + + /** + * Data provider for apply_units with currency values. + * + * @return array + */ + public function currency_provider(): array { + return [ + [['1234.56', '£', 1], '£1,234.56'], + [['100', '$', 1], '$100'], + [['100', '$', 1], '$100.'], + [['100.00', '$', 1], '$100.00'], + [['100', '', null], '100'], + [['100', 'frog', null], 'frog 100'], + ]; } } diff --git a/question/type/numerical/tests/coverage.php b/question/type/numerical/tests/coverage.php new file mode 100644 index 00000000000..8d8e9355908 --- /dev/null +++ b/question/type/numerical/tests/coverage.php @@ -0,0 +1,31 @@ +. + +defined('MOODLE_INTERNAL') || die(); + +/** + * Coverage information for the qtype_numerical. + * + * @copyright 2022 Andrew Nicols + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +return new class extends phpunit_coverage_info { + /** @var array The list of files relative to the plugin root to include in coverage generation. */ + protected $includelistfiles = [ + 'question.php', + 'questiontype.php', + ]; +};