From abc8f149ce1bd71d18384d3b5f2f4c721663c2f0 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Sat, 2 Nov 2019 20:03:40 +0100 Subject: [PATCH] MDL-67114 core: php74 fix. Fix use of scalar as array in core 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. --- competency/tests/privacy_test.php | 4 ++-- enrol/meta/lib.php | 6 ++++-- lib/editor/tests/fixtures/editor_form.php | 2 +- lib/filestorage/file_system.php | 5 +++++ lib/form/duration.php | 2 +- lib/form/filetypes.php | 2 ++ lib/tests/session_manager_test.php | 2 +- mod/glossary/import_form.php | 2 +- mod/wiki/comments_form.php | 4 ++-- mod/wiki/parser/parser.php | 2 +- question/engine/questionusage.php | 2 +- repository/dropbox/classes/dropbox.php | 2 +- 12 files changed, 22 insertions(+), 13 deletions(-) diff --git a/competency/tests/privacy_test.php b/competency/tests/privacy_test.php index 77c5a5a45ce..7105753f50d 100644 --- a/competency/tests/privacy_test.php +++ b/competency/tests/privacy_test.php @@ -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')})"])); diff --git a/enrol/meta/lib.php b/enrol/meta/lib.php index b64817b7140..632860a4bb9 100644 --- a/enrol/meta/lib.php +++ b/enrol/meta/lib.php @@ -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) { diff --git a/lib/editor/tests/fixtures/editor_form.php b/lib/editor/tests/fixtures/editor_form.php index 645728ad919..cc81222b01b 100644 --- a/lib/editor/tests/fixtures/editor_form.php +++ b/lib/editor/tests/fixtures/editor_form.php @@ -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'); diff --git a/lib/filestorage/file_system.php b/lib/filestorage/file_system.php index ae1d3621d1c..8cf9ffcdc6d 100644 --- a/lib/filestorage/file_system.php +++ b/lib/filestorage/file_system.php @@ -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; diff --git a/lib/form/duration.php b/lib/form/duration.php index d1c8eee40aa..91ed7686450 100644 --- a/lib/form/duration.php +++ b/lib/form/duration.php @@ -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); diff --git a/lib/form/filetypes.php b/lib/form/filetypes.php index 9e4e3b06a53..9c70b5a8ff5 100644 --- a/lib/form/filetypes.php +++ b/lib/form/filetypes.php @@ -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']); diff --git a/lib/tests/session_manager_test.php b/lib/tests/session_manager_test.php index 0c715523634..bbafd02aa10 100644 --- a/lib/tests/session_manager_test.php +++ b/lib/tests/session_manager_test.php @@ -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); } /** diff --git a/mod/glossary/import_form.php b/mod/glossary/import_form.php index 2f37a149e4d..22255c1a1c7 100644 --- a/mod/glossary/import_form.php +++ b/mod/glossary/import_form.php @@ -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'); diff --git a/mod/wiki/comments_form.php b/mod/wiki/comments_form.php index fe53834d317..c8769f9379f 100644 --- a/mod/wiki/comments_form.php +++ b/mod/wiki/comments_form.php @@ -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); diff --git a/mod/wiki/parser/parser.php b/mod/wiki/parser/parser.php index 07a8df5c72d..9616e3b19f5 100644 --- a/mod/wiki/parser/parser.php +++ b/mod/wiki/parser/parser.php @@ -47,7 +47,7 @@ class wiki_parser_proxy { return $content; } else { - return $content[1]; + return is_array($content) ? $content[1] : null; } } else { diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index 606c30b12bb..061df4a3024 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -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. diff --git a/repository/dropbox/classes/dropbox.php b/repository/dropbox/classes/dropbox.php index 81724b5c14c..dceee7d6b50 100644 --- a/repository/dropbox/classes/dropbox.php +++ b/repository/dropbox/classes/dropbox.php @@ -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; }