MDL-67114 core: php74 fix. Fix use of scalar as array in core
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Sat, 2 Nov 2019 19:03:40 +0000 (20:03 +0100)
committerEloy Lafuente (stronk7) <stronk7@moodle.org>
Fri, 3 Jan 2020 10:33:15 +0000 (11:33 +0100)
There are various places where it's not guaranteed that the
variable being used is array, and instead, can be null, bool, int...

We need to check that because php74 warns about it.

Where possible we have used the coalesce operator as
replacement for isset() ternary operations.

12 files changed:
competency/tests/privacy_test.php
enrol/meta/lib.php
lib/editor/tests/fixtures/editor_form.php
lib/filestorage/file_system.php
lib/form/duration.php
lib/form/filetypes.php
lib/tests/session_manager_test.php
mod/glossary/import_form.php
mod/wiki/comments_form.php
mod/wiki/parser/parser.php
question/engine/questionusage.php
repository/dropbox/classes/dropbox.php

index 77c5a5a..7105753 100644 (file)
@@ -2302,7 +2302,7 @@ class core_competency_privacy_testcase extends provider_testcase {
         $this->assertEquals('-', $comp['rating']['rating']);
         $comp = $data->competencies[2];
         $this->assertEquals($comp4->get('shortname'), $comp['name']);
-        $this->assertNull($comp['rating']['rating']);
+        $this->assertNull($comp['rating']);
         $data = writer::with_context($u1ctx)->get_data(array_merge($path, ["{$p1a->get('name')} ({$p1a->get('id')})",
             get_string('commentsubcontext', 'core_comment')]));
         $this->assert_exported_comments(['Hello.', 'It\'s me.', 'After all these years...'], $data->comments);
@@ -2320,7 +2320,7 @@ class core_competency_privacy_testcase extends provider_testcase {
         $this->assertEquals('C', $comp['rating']['rating']);
         $comp = $data->competencies[2];
         $this->assertEquals($comp4->get('shortname'), $comp['name']);
-        $this->assertNull($comp['rating']['rating']);
+        $this->assertNull($comp['rating']);
 
         // This plan is complete.
         $data = writer::with_context($u1ctx)->get_data(array_merge($path, ["{$p1c->get('name')} ({$p1c->get('id')})"]));
index b64817b..632860a 100644 (file)
@@ -120,10 +120,12 @@ class enrol_meta_plugin extends enrol_plugin {
         require_once("$CFG->dirroot/enrol/meta/locallib.php");
 
         // Support creating multiple at once.
-        if (is_array($fields['customint1'])) {
+        if (isset($fields['customint1']) && is_array($fields['customint1'])) {
             $courses = array_unique($fields['customint1']);
-        } else {
+        } else if (isset($fields['customint1'])) {
             $courses = array($fields['customint1']);
+        } else {
+            $courses = array(null); // Strange? Yes, but that's how it's working or instance is not created ever.
         }
         foreach ($courses as $courseid) {
             if (!empty($fields['customint2']) && $fields['customint2'] == ENROL_META_CREATE_GROUP) {
index 645728a..cc81222 100644 (file)
@@ -44,7 +44,7 @@ class editor_form extends moodleform {
      */
     protected function definition() {
         $mform = $this->_form;
-        $editoroptions = $this->_customdata['editoroptions'];
+        $editoroptions = $this->_customdata['editoroptions'] ?? null;
 
         // Add header.
         $mform->addElement('header', 'myheader', 'Editor in Moodle form');
index ae1d362..8cf9ffc 100644 (file)
@@ -416,11 +416,16 @@ abstract class file_system {
     protected function get_imageinfo_from_path($path) {
         $imageinfo = getimagesize($path);
 
+        if (!is_array($imageinfo)) {
+            return false; // Nothing to process, the file was not recognised as image by GD.
+        }
+
         $image = array(
                 'width'     => $imageinfo[0],
                 'height'    => $imageinfo[1],
                 'mimetype'  => image_type_to_mime_type($imageinfo[2]),
             );
+
         if (empty($image['width']) or empty($image['height']) or empty($image['mimetype'])) {
             // GD can not parse it, sorry.
             return false;
index d1c8eee..91ed768 100644 (file)
@@ -200,7 +200,7 @@ class MoodleQuickForm_duration extends MoodleQuickForm_group {
                 break;
 
             case 'createElement':
-                if ($arg[2]['optional']) {
+                if (!empty($arg[2]['optional'])) {
                     $caller->disabledIf($arg[0], $arg[0] . '[enabled]');
                 }
                 $caller->setType($arg[0] . '[number]', PARAM_FLOAT);
index 9e4e3b0..9c70b5a 100644 (file)
@@ -221,6 +221,8 @@ class MoodleQuickForm_filetypes extends MoodleQuickForm_group {
      */
     public function validateSubmitValue($value) {
 
+        $value = $value ?? ['filetypes' => null]; // A null $value can arrive here. Coalesce, creating the default array.
+
         if (!$this->allowall) {
             // Assert that there is an actual list provided.
             $normalized = $this->util->normalize_file_types($value['filetypes']);
index 0c71552..bbafd02 100644 (file)
@@ -835,7 +835,7 @@ class core_session_manager_testcase extends advanced_testcase {
         $SESSION->recentsessionlocks = $this->sessionlock_history();
 
         $page = \core\session\manager::get_locked_page_at($time);
-        $this->assertEquals($url, $page['url']);
+        $this->assertEquals($url, is_array($page) ? $page['url'] : null);
     }
 
     /**
index 2f37a14..22255c1 100644 (file)
@@ -10,7 +10,7 @@ class mod_glossary_import_form extends moodleform {
     function definition() {
         global $CFG;
         $mform =& $this->_form;
-        $cmid = $this->_customdata['id'];
+        $cmid = $this->_customdata['id'] ?? null;
 
         $mform->addElement('filepicker', 'file', get_string('filetoimport', 'glossary'));
         $mform->addHelpButton('file', 'filetoimport', 'glossary');
index fe53834..c8769f9 100644 (file)
@@ -10,8 +10,8 @@ class mod_wiki_comments_form extends moodleform {
     protected function definition() {
         $mform = $this->_form;
 
-        $current = $this->_customdata['current'];
-        $commentoptions = $this->_customdata['commentoptions'];
+        $current = $this->_customdata['current'] ?? null;
+        $commentoptions = $this->_customdata['commentoptions'] ?? null;
 
         // visible elements
         $mform->addElement('editor', 'entrycomment_editor', get_string('comment', 'glossary'), null, $commentoptions);
index 07a8df5..9616e3b 100644 (file)
@@ -47,7 +47,7 @@ class wiki_parser_proxy {
                return $content;
             }
             else {
-               return $content[1];
+               return is_array($content) ? $content[1] : null;
                }
         }
         else {
index 606c30b..061df4a 100644 (file)
@@ -701,7 +701,7 @@ class question_usage_by_activity {
             // Behaviour vars should not be processed by question type, just add prefix.
             $behaviourvars = $this->get_question_attempt($slot)->get_behaviour()->get_expected_data();
             foreach (array_keys($responsedata) as $responsedatakey) {
-                if ($responsedatakey[0] === '-') {
+                if (is_string($responsedatakey) && $responsedatakey[0] === '-') {
                     $behaviourvarname = substr($responsedatakey, 1);
                     if (isset($behaviourvars[$behaviourvarname])) {
                         // Expected behaviour var found.
index 81724b5..dceee7d 100644 (file)
@@ -181,7 +181,7 @@ class dropbox extends \oauth2_client {
      * @throws  moodle_exception
      */
     protected function check_and_handle_api_errors($data) {
-        if ($this->info['http_code'] == 200) {
+        if (!is_array($this->info) or $this->info['http_code'] == 200) {
             // Dropbox only returns errors on non-200 response codes.
             return;
         }