Merge branch 'MDL-76362-master-extra' of https://github.com/andrewnicols/moodle

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 ?? '');
     }
 
     /**

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) {

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 ?? '');
     }
 
     /**

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';

admin/tool/dataprivacy/classes/external/purpose_exporter.php

@@ -109,7 +109,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))) {

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);
         }
     }
 

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->filename = pathinfo($this->url->get_path() ?? '', PATHINFO_FILENAME);
-        $this->extension = pathinfo($this->url->get_path(), PATHINFO_EXTENSION);
+        $this->extension = pathinfo($this->url->get_path() ?? '', PATHINFO_EXTENSION);
         $this->metadata = $metadata;
     }
 

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 {

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);

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;
     }
 }
 

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
+        } else if (strlen($cdata ?? '') < 32) {
-            return $cdata;                // (http://10.0.0.1/file.php/1/1.jpg, http://10.0.0.1/mod/url/view.php?id=)
+            // 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) {

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;
         }

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;
     }

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('[PARAM1]', $param1 ?? '', $url);
-        $url = str_replace('[PARAM2]', $param2, $url);
+        $url = str_replace('[PARAM2]', $param2 ?? '', $url);
 
         return $url;
     }

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.

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),

course/format/classes/base.php

@@ -618,7 +618,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 = [];

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();
         }
 

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);

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();
     }
 }

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 {
+        } else if (!empty($key)) {
-            $key256 = DataConnector::getConsumerKey($consumer->getKey());
+            $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)) {

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);
+            $resheaders = substr($res ?? '', 0, $headersize);
-            $resbody = substr($res, $headersize);
+            $resbody = substr($res ?? '', $headersize);
             $headerlines = array_filter(explode("\r\n", $resheaders));
             $parsedresponseheaders = [
                 'httpstatus' => array_shift($headerlines)

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) {
+                if ($filter == 'all' || $empties) {
-                    $data->$varname = ($isscale and empty($insertvalue)) ?
+                    $data->$varname = ($isscale && empty($insertvalue)) ? -1 : $insertvalue;
-                        -1 : $insertvalue;
                 }
             }
         }

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)) {

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])) {

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;

lib/accesslib.php

@@ -4560,7 +4560,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;
@@ -4568,7 +4568,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;

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);
         }
 

lib/classes/component.php

@@ -1001,10 +1001,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);
         }
     }
 

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 {

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;
+    }
 }

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;

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();

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

lib/filebrowser/file_info.php

@@ -100,7 +100,7 @@ abstract class file_info {
      */
     protected function build_search_files_sql($extensions, $prefix = null) {
         global $DB;
-        if (strlen($prefix)) {
+        if ($prefix && strlen($prefix)) {
             $prefix = $prefix . '.';
         } else {
             $prefix = '';

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);
+    $safemimetype = str_replace('+', '_', $mimetype ?? '');
-    $safemimetypestr = str_replace('+', '_', $mimetypestr);
+    $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;

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 ?? '');
     }
 }

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);

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)) {

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 [];
         }
 

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;
     }

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';

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])/';

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;

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;

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

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);
         }
 

lib/outputrequirementslib.php

@@ -722,7 +722,7 @@ class page_requirements_manager {
             }
             // The URL is not a Moodle resource.
             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) {

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;

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();
     }
 }
-

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;
             }

lib/tests/component_test.php

@@ -212,33 +212,58 @@ class component_test extends advanced_testcase {
         }
     }
 
-    public function test_is_valid_plugin_name() {
+    /**
-        $this->assertTrue(core_component::is_valid_plugin_name('mod', 'example1'));
+     * Test that the get_plugin_list_with_file() function returns the correct list of plugins.
-        $this->assertTrue(core_component::is_valid_plugin_name('mod', 'feedback360'));
+     *
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', 'feedback_360'));
+     * @covers \core_component::is_valid_plugin_name
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', '2feedback'));
+     * @dataProvider is_valid_plugin_name_provider
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', '1example'));
+     * @param array $arguments
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example.xx'));
+     * @param bool $expected
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', '.example'));
+     */
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', '_example'));
+    public function test_is_valid_plugin_name(array $arguments, bool $expected): void {
-        $this->assertFalse(core_component::is_valid_plugin_name('mod', 'example_'));
+        $this->assertEquals($expected, core_component::is_valid_plugin_name(...$arguments));
-        $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'));
 
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example1'));
+    /**
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example_x1'));
+     * Data provider for the is_valid_plugin_name function.
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'example_x1_xxx'));
+     *
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'feedback360'));
+     * @return array
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'feed_back360'));
+     */
-        $this->assertTrue(core_component::is_valid_plugin_name('tool', 'role'));
+    public function is_valid_plugin_name_provider(): array {
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', '1example'));
+        return [
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example.xx'));
+            [['mod', 'example1'], true],
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example-xx'));
+            [['mod', 'feedback360'], true],
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', '.example'));
+            [['mod', 'feedback_360'], false],
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', '_example'));
+            [['mod', '2feedback'], false],
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example_'));
+            [['mod', '1example'], false],
-        $this->assertFalse(core_component::is_valid_plugin_name('tool', 'example__x1'));
+            [['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() {

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
      */
@@ -1119,4 +1117,32 @@ EXPECTED;
             ],
         ];
     }
+
+    /**
+     * 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));
+    }
 }

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;',
+    if ($var === null || $var === '') {
-            htmlspecialchars($var, ENT_QUOTES | ENT_HTML401 | ENT_SUBSTITUTE));
+        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) {
@@ -1127,7 +1136,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);
 }
 
 /**
@@ -1540,7 +1549,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};)/", "&amp;", $string);
+    return preg_replace("/\&(?![a-zA-Z0-9#]{1,8};)/", "&amp;", $string ?? '');
 }
 
 /**

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])) {

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 = "<preset>\n\n";
 
         // Add description.
-        $presetxmldata .= '<description>' . htmlspecialchars($this->description, ENT_COMPAT) . "</description>\n\n";
+        $presetxmldata .= '<description>' . htmlspecialchars($this->description ?? '', ENT_COMPAT) . "</description>\n\n";
 
         // Add settings.
         // Raw settings are not preprocessed during saving of presets.

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();

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 {

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;

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);
 }

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;
         }

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.

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];
     }
 }

mod/quiz/accessrule/seb/classes/seb_quiz_settings.php

@@ -662,7 +662,7 @@ class seb_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);
         }

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, [

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',

mod/workshop/renderer.php

@@ -1841,7 +1841,7 @@ class mod_workshop_renderer extends plugin_renderer_base {
                         $output .= $this->render($workshop->prepare_submission_summary($submission, $shownames));
                         $output .= $this->box_end();
 
-                        if (strlen(trim($assessment->feedbackreviewer)) > 0) {
+                        if (!empty($assessment->feedbackreviewer) && strlen(trim($assessment->feedbackreviewer)) > 0) {
                             $output .= $this->render(new workshop_feedback_reviewer($assessment));
                         }
                     }

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 <http://www.gnu.org/licenses/>.
 
-/**
- * This file contains the moodle format implementation of the content writer.
- *
- * @package core_privacy
- * @copyright 2018 Andrew Nicols <andrew@nicols.co.uk>
- * @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 <andrew@nicols.co.uk>
  * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -169,6 +161,9 @@ class moodle_content_writer implements content_writer {
      * @return  string                      The processed 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)) {

privacy/classes/tests/request/content_writer.php

@@ -382,7 +382,7 @@ class content_writer implements \core_privacy\local\request\content_writer {
      * @return  string                      The processed string
      */
     public function rewrite_pluginfile_urls(array $subcontext, $component, $filearea, $itemid, $text): string {
-        return str_replace('@@PLUGINFILE@@/', 'files/', $text);
+        return str_replace('@@PLUGINFILE@@/', 'files/', $text ?? '');
     }
 
     /**

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,

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 = '';

question/engine/tests/coverage.php

@@ -0,0 +1,38 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Coverage information for the question_engine.
+ *
+ * @copyright  2022 Andrew Nicols <andrew@nicols.co.uk>
+ * @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',
+        'phpunit.xml',
+        'questionattempt.php',
+        'questionattemptstep.php',
+        'questionusage.php',
+        'renderer.php',
+
+    ];
+};

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 <huongnv13@gmail.com>
- * @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);
-    }
-}

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'));
+     * Test the get_behaviour_unused_display_options with various options.
-        $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'),
+     *
-                question_engine::get_behaviour_unused_display_options('deferredfeedback'));
+     * @covers \question_engine::get_behaviour_unused_display_options
-        $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'),
+     * @dataProvider get_behaviour_unused_display_options_provider
-                question_engine::get_behaviour_unused_display_options('deferredcbm'));
+     * @param string $behaviour
-        $this->assertEquals(array('correctness', 'marks', 'specificfeedback', 'generalfeedback', 'rightanswer'),
+     * @param array $expected
-                question_engine::get_behaviour_unused_display_options('manualgraded'));
+     */
+    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'));
+     * Data provider for get_behaviour_unused_display_options.
-        $this->assertTrue(question_engine::can_questions_finish_during_the_attempt('interactive'));
+     *
+     * @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');
+     * 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));
+    }
 
-        $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, '', '', ''));
+     * 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],
+        ];
+    }
 
-        $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b4', 'b4'));
+    /**
+     * 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));
+    }
 
-        $out = array('b4' => 'Behave 4', 'b5' => 'Behave 5', 'b6' => 'Behave 6');
+    /**
-        $this->assertSame($out, question_engine::sort_behaviours($in, '', 'b1,b2,b3,b4', 'b4'));
+     * Data provider for sort_behaviours.
-
+     *
-        $out = array('b6' => 'Behave 6', 'b1' => 'Behave 1', 'b4' => 'Behave 4');
+     * @return array
-        $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b1,b4', 'b2,b3,b4,b5', 'b4'));
+     */
-
+    public function sort_behaviours_provider(): array {
-        $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4');
+        $in = [
-        $this->assertSame($out, question_engine::sort_behaviours($in, 'b6,b5,b4', 'b1,b2,b3', 'b4'));
+            'b1' => 'Behave 1',
-
+            'b2' => 'Behave 2',
-        $out = array('b6' => 'Behave 6', 'b5' => 'Behave 5', 'b4' => 'Behave 4');
+            'b3' => 'Behave 3',
-        $this->assertSame($out, question_engine::sort_behaviours($in, 'b1,b6,b5', 'b1,b2,b3,b4', 'b4'));
+            'b4' => 'Behave 4',
-
+            'b5' => 'Behave 5',
-        $out = array('b2' => 'Behave 2', 'b4' => 'Behave 4', 'b6' => 'Behave 6');
+            'b6' => 'Behave 6',
-        $this->assertSame($out, question_engine::sort_behaviours($in, 'b2,b4,b6', 'b1,b3,b5', 'b2'));
+        ];
 
+        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.
-        $this->assertSame($in, question_engine::sort_behaviours($in, 'unknown', '', ''));
+            [
-
+                [$in, 'unknown', '', ''],
+                $in,
+            ],
             // Ignore unknown input in the disabled argument.
-        $this->assertSame($in, question_engine::sort_behaviours($in, '', 'unknown', ''));
+            [
+                [$in, '', 'unknown', ''],
+                $in,
+            ],
+        ];
     }
 
-    public function test_is_manual_grade_in_range() {
+    /**
-        $_POST[] = array('q1:2_-mark' => 0.5, 'q1:2_-maxmark' => 1.0,
+     * Tests for is_manual_grade_in_range.
-                'q1:2_:minfraction' => 0, 'q1:2_:maxfraction' => 1);
+     *
+     * @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,
+     * Ensure that the number renderer performs as expected.
-                'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1);
+     *
-        $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2));
+     * @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,
+     * Data provider for test_render_question_number.
-                'q1:2_:minfraction' => -0.5, 'q1:2_:maxfraction' => 1);
+     *
-        $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2));
+     * @return array
-    }
+     */
-
+    public function render_question_number_provider(): array {
-    public function test_is_manual_grade_in_range_top_end() {
+        return [
-        $_POST[] = array('q1:2_-mark' => 3.0, 'q1:2_-maxmark' => 1.0,
+            'Test with number is i character' => [
-                'q1:2_:minfraction' => -6.0, 'q1:2_:maxfraction' => 3.0);
+                'i',
-        $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2));
+                '<h3 class="no">Information</h3>',
-    }
+            ],
-
+            'Test with number is empty string' => [
-    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));
+            'Test with null' => [
-    }
+                null,
-
+                '',
-    public function test_is_manual_grade_in_range_ungraded() {
+            ],
-        $this->assertTrue(question_engine::is_manual_grade_in_range(1, 2));
+            'Test with number is 0' => [
-    }
+                0,
-
+                '<h3 class="no">Question <span class="qno">0</span></h3>',
-    public function test_render_question_number() {
+            ],
-        global $CFG, $PAGE;
+            'Test with number is numeric' => [
-
+                1,
-        require_once("{$CFG->dirroot}/question/engine/tests/helpers.php");
+                '<h3 class="no">Question <span class="qno">1</span></h3>',
-        $renderer = new testable_core_question_renderer($PAGE, 'core_question');
+            ],
-
+            'Test with number is string' => [
-        // Test with number is i character.
+                '1 of 2',
-        $this->assertEquals('<h3 class="no">Information</h3>', $renderer->number('i'));
+                '<h3 class="no">Question <span class="qno">1 of 2</span></h3>',
-        // Test with number is empty string.
+            ],
-        $this->assertEquals('', $renderer->number(''));
+        ];
-        // Test with number is 0.
-        $this->assertEquals('<h3 class="no">Question <span class="qno">0</span></h3>', $renderer->number(0));
-        // Test with number is numeric.
-        $this->assertEquals('<h3 class="no">Question <span class="qno">1</span></h3>', $renderer->number(1));
-        // Test with number is string.
-        $this->assertEquals('<h3 class="no">Question <span class="qno">1 of 2</span></h3>', $renderer->number('1 of 2'));
     }
 }

question/tests/coverage.php

@@ -0,0 +1,40 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Coverage information for the core_question.
+ *
+ * @copyright  2022 Andrew Nicols <andrew@nicols.co.uk>
+ * @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',
+    ];
+};

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 <http://www.gnu.org/licenses/>.
 
-/**
- * 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);
+        [$answer, $unit] = $this->parse_response($state);
-        if (!empty($this->question->options->showunits) &&
+        if (
-                $this->question->options->showunits == 1) {
+            !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->search = [];
-        $this->safevalue = array();
+        $this->safevalue = [];
-        $this->prettyvalue = array();
+        $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('~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
+        $text = preg_replace_callback(
+            '~\{=([^{}]*(?:\{[^{}]+}[^{}]*)*)}~',
             function ($matches) use ($vs, $format, $length) {
                 return $vs->format_float($vs->calculate($matches[1]), $length, $format);
-                }, $text);
+            },
+            $text
+        );
         return $this->substitute_values_pretty($text);
     }
 }

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 <http://www.gnu.org/licenses/>.
 
-/**
- * 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 <order>: 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);
+        [$datasetbit, $answer] = explode('-', $state->answer, 2);
-        list($order, $responses) = explode(':', $answer);
+        [$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->search = [];
-        $this->safevalue = array();
+        $this->safevalue = [];
-        $this->prettyvalue = array();
+        $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,
+        $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
+        );
         return $this->substitute_values_pretty($text);
     }
 }

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)));
         }

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);

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(
+     * Test setup.
-                array('m' => 1, 'cm' => 100), false, '.', ',');
+     */
+    public function setUp(): void {
+        global $CFG;
 
-        $this->assertEquals(array('3', '142', '', ''), $ap->parse_response('3.142'));
+        require_once("{$CFG->dirroot}/question/type/numerical/questiontype.php");
-        $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(','));
     }
 
-    protected function verify_value_and_unit($exectedval, $expectedunit, $expectedmultiplier,
+    /**
-            qtype_numerical_answer_processor $ap, $input, $separateunit = null) {
+     * Test the parse_response function.
-        list($val, $unit, $multiplier) = $ap->apply_units($input, $separateunit);
+     *
+     * @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(
-        $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8');
+            $expectedvalue,
-        $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1c');
+            $expectedunit,
-        $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000 mph');
+            $expectedmultiplier,
-
+            $ap,
-        $this->verify_value_and_unit(1, 'frogs', null, $ap, '1 frogs');
+            $input
-        $this->verify_value_and_unit(null, null, null, $ap, '. m/s');
+        );
     }
 
-    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(
-        $this->verify_value_and_unit(3e8, '', null, $ap, '3x10^8', '');
+            $expectedvalue,
-        $this->verify_value_and_unit(1, 'c', 299792458, $ap, '1', 'c');
+            $expectedunit,
-        $this->verify_value_and_unit(1, 'mph', 0.44704, $ap, '0001.000', 'mph');
+            $expectedmultiplier,
-
+            $ap,
-        $this->verify_value_and_unit(1, 'frogs', null, $ap, '1', 'frogs');
+            $input,
-        $this->verify_value_and_unit(null, null, null, $ap, '.', 'm/s');
+            $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'));
+            [1, 'frogs', null, '1', 'frogs'],
-        $this->assertEquals(array(3.14159, '', null), $ap->apply_units('3,14159'));
+            [null, null, null, '.', 'm/s'],
+        ];
     }
 
-    public function test_percent() {
+    /**
-        $ap = new qtype_numerical_answer_processor(array('%' => 100), false, '.', ',');
+     * Test apply_units with a comma float unit.
-
+     *
-        $this->assertEquals(array('3', '%', 0.01), $ap->apply_units('3%'));
+     * @covers \qtype_numerical_answer_processor::apply_units
-        $this->assertEquals(array('1e-6', '%', 0.01), $ap->apply_units('1e-6 %'));
+     * @dataProvider euro_provider
-        $this->assertEquals(array('100', '', null), $ap->apply_units('100'));
+     * @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'));
+     * Test apply_units with percentage values.
-        $this->assertEquals(array('100', '$', 1), $ap->apply_units('$100.'));
+     *
-        $this->assertEquals(array('100.00', '$', 1), $ap->apply_units('$100.00'));
+     * @covers \qtype_numerical_answer_processor::apply_units
-        $this->assertEquals(array('100', '', null), $ap->apply_units('100'));
+     * @dataProvider percent_provider
-        $this->assertEquals(array('100', 'frog', null), $ap->apply_units('frog 100'));
+     * @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'],
+        ];
     }
 }

question/type/numerical/tests/coverage.php

@@ -0,0 +1,31 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+defined('MOODLE_INTERNAL') || die();
+
+/**
+ * Coverage information for the qtype_numerical.
+ *
+ * @copyright  2022 Andrew Nicols <andrew@nicols.co.uk>
+ * @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',
+    ];
+};