MDL-66968 php74: array_key_exists() for objects is deprecated

Replace it for correct property_exists() when the element being inspected is a property of object/class. Amended and squased changes: - keep mongo unmodified. The information is array, hence correct. - fix a couple of messaging phpdocs that were incorrect. Amended take#2: - As far as mongo resturns BSONDocument that is ArrayObject, aka implements ArrayAccess, we have decided to explicitly cast results to array so existing array_key_exists() and other accesses will continue working the same.
rudivdme · Oct 24, 2019 · f4feabb · f4feabb
1 parent aaff669
commit f4feabb
Show file tree

Hide file tree

Showing 21 changed files with 36 additions and 30 deletions.
diff --git a/admin/message.php b/admin/message.php
@@ -77,7 +77,7 @@
                     // Record the site preference.
                     $preferences[$processor->name.'_provider_'.$componentprovidersetting] = $value;
                 }
-            } else if (array_key_exists($componentprovidersetting, $form)) {
+            } else if (property_exists($form, $componentprovidersetting)) {
                 // We must be processing loggedin or loggedoff checkboxes. Store
                 // defained comma-separated processors as setting value.
                 // Using array_filter eliminates elements set to 0 above.

diff --git a/admin/repository.php b/admin/repository.php
@@ -123,13 +123,13 @@ function repository_action_url($repository) {
             }
             $instanceoptionnames = repository::static_function($repository, 'get_instance_option_names');
             if (!empty($instanceoptionnames)) {
-                if (array_key_exists('enablecourseinstances', $fromform)) {
+                if (property_exists($fromform, 'enablecourseinstances')) {
                     $settings['enablecourseinstances'] = $fromform->enablecourseinstances;
                 }
                 else {
                     $settings['enablecourseinstances'] = 0;
                 }
-                if (array_key_exists('enableuserinstances', $fromform)) {
+                if (property_exists($fromform, 'enableuserinstances')) {
                     $settings['enableuserinstances'] = $fromform->enableuserinstances;
                 }
                 else {

diff --git a/cache/stores/mongodb/lib.php b/cache/stores/mongodb/lib.php
@@ -254,7 +254,13 @@ public function get($key) {
         }
 
         $result = $this->collection->findOne($key);
-        if ($result === null || !array_key_exists('data', $result)) {
+        // Note $result is really an object, BSONDocument extending ArrayObject,
+        // which implements ArrayAccess. That enables access to its information
+        // using square brackets and some array operations. But, it seems that
+        // it's not enough for array_key_exists() to operate on it. Hence, we
+        // are explicitly casting to array, after having checked that the operation
+        // doesn't incur into any performance penalty.
+        if ($result === null || !array_key_exists('data', (array)$result)) {
             return false;
         }
         $data = @unserialize($result['data']);

diff --git a/competency/classes/api.php b/competency/classes/api.php
@@ -3839,7 +3839,7 @@ public static function update_user_evidence($data, $draftitemid = null) {
         if (!$userevidence->can_manage()) {
             throw new required_capability_exception($context, 'moodle/competency:userevidencemanage', 'nopermissions', '');
 
-        } else if (array_key_exists('userid', $data) && $data->userid != $userevidence->get('userid')) {
+        } else if (property_exists($data, 'userid') && $data->userid != $userevidence->get('userid')) {
             throw new coding_exception('Can not change the userid of a user evidence.');
         }
 

diff --git a/enrol/ldap/lib.php b/enrol/ldap/lib.php
@@ -460,7 +460,7 @@ public function sync_enrolments(progress_trace $trace, $onecourse = null) {
                         // this is an odd array -- mix of hash and array --
                         $ldapmembers = array();
 
-                        if (array_key_exists('memberattribute_role'.$role->id, $this->config)
+                        if (property_exists($this->config, 'memberattribute_role'.$role->id)
                             && !empty($this->config->{'memberattribute_role'.$role->id})
                             && !empty($course[$this->config->{'memberattribute_role'.$role->id}])) { // May have no membership!
 

diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php
@@ -2192,7 +2192,7 @@ private static function _get_children_recursion($category) {
         $children_array = array();
         foreach ($category->children as $sortorder=>$child) {
 
-            if (array_key_exists('itemtype', $child)) {
+            if (property_exists($child, 'itemtype')) {
                 $grade_item = new grade_item($child, false);
 
                 if (in_array($grade_item->itemtype, array('course', 'category'))) {

diff --git a/lib/outputcomponents.php b/lib/outputcomponents.php
@@ -228,7 +228,7 @@ public function __construct(stdClass $user) {
         // only touch the DB if we are missing data and complain loudly...
         $needrec = false;
         foreach (self::$fields as $field) {
-            if (!array_key_exists($field, $user)) {
+            if (!property_exists($user, $field)) {
                 $needrec = true;
                 debugging('Missing '.$field.' property in $user object, this is a performance problem that needs to be fixed by a developer. '
                           .'Please use user_picture::fields() to get the full list of required fields.', DEBUG_DEVELOPER);

diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php
@@ -1996,7 +1996,7 @@ public function single_button($url, $label, $method='post', array $options=null)
         $button = new single_button($url, $label, $method);
 
         foreach ((array)$options as $key=>$value) {
-            if (array_key_exists($key, $button)) {
+            if (property_exists($button, $key)) {
                 $button->$key = $value;
             } else {
                 $button->set_attribute($key, $value);
@@ -2479,7 +2479,7 @@ public function spacer(array $attributes = null, $br = false) {
     public function user_picture(stdClass $user, array $options = null) {
         $userpicture = new user_picture($user);
         foreach ((array)$options as $key=>$value) {
-            if (array_key_exists($key, $userpicture)) {
+            if (property_exists($userpicture, $key)) {
                 $userpicture->$key = $value;
             }
         }

diff --git a/message/defaultoutputs.php b/message/defaultoutputs.php
@@ -76,7 +76,7 @@
                     // record the site preference
                     $preferences[$processor->name.'_provider_'.$componentprovidersetting] = $value;
                 }
-            } else if (array_key_exists($componentprovidersetting, $form)) {
+            } else if (property_exists($form, $componentprovidersetting)) {
                 // we must be processing loggedin or loggedoff checkboxes. Store
                 // defained comma-separated processors as setting value.
                 // Using array_filter eliminates elements set to 0 above

diff --git a/message/renderer.php b/message/renderer.php
@@ -42,7 +42,7 @@ class core_message_renderer extends plugin_renderer_base {
      * @param  array $allprocessors  array of objects containing all message processors
      * @param  array $processors  array of objects containing active message processors
      * @param  array $providers   array of objects containing message providers
-     * @param  array $preferences array of objects containing current preferences
+     * @param  stdClass $preferences object containing current preferences
      * @return string The text to render
      */
     public function manage_messageoutput_settings($allprocessors, $processors, $providers, $preferences) {
@@ -120,7 +120,7 @@ public function manage_messageoutputs($processors) {
      *
      * @param  array $processors  array of objects containing message processors
      * @param  array $providers   array of objects containing message providers
-     * @param  array $preferences array of objects containing current preferences
+     * @param  stdClass $preferences object containing current preferences
      * @return string The text to render
      */
     public function manage_defaultmessageoutputs($processors, $providers, $preferences) {
@@ -177,7 +177,7 @@ public function manage_defaultmessageoutputs($processors, $providers, $preferenc
                         $preference = $processor->name.'_provider_'.$preferencebase;
                         if ($providerdisabled) {
                             $select = MESSAGE_DISALLOWED;
-                        } else if (array_key_exists($preference, $preferences)) {
+                        } else if (property_exists($preferences, $preference)) {
                             $select = $preferences->{$preference};
                         }
                         // dropdown menu
@@ -192,7 +192,7 @@ public function manage_defaultmessageoutputs($processors, $providers, $preferenc
                             $checked = true;
                         } else if ($select == 'permitted') {
                             $preference = 'message_provider_'.$preferencebase;
-                            if (array_key_exists($preference, $preferences)) {
+                            if (property_exists($preferences, $preference)) {
                                 $checked = (int)in_array($processor->name, explode(',', $preferences->{$preference}));
                             }
                         }

diff --git a/mod/lesson/lib.php b/mod/lesson/lib.php
@@ -669,7 +669,7 @@ function lesson_grade_item_update($lesson, $grades=null) {
         require_once($CFG->libdir.'/gradelib.php');
     }
 
-    if (array_key_exists('cmidnumber', $lesson)) { //it may not be always present
+    if (property_exists($lesson, 'cmidnumber')) { //it may not be always present
         $params = array('itemname'=>$lesson->name, 'idnumber'=>$lesson->cmidnumber);
     } else {
         $params = array('itemname'=>$lesson->name);

diff --git a/mod/quiz/lib.php b/mod/quiz/lib.php
@@ -741,7 +741,7 @@ function quiz_grade_item_update($quiz, $grades = null) {
     require_once($CFG->dirroot . '/mod/quiz/locallib.php');
     require_once($CFG->libdir . '/gradelib.php');
 
-    if (array_key_exists('cmidnumber', $quiz)) { // May not be always present.
+    if (property_exists($quiz, 'cmidnumber')) { // May not be always present.
         $params = array('itemname' => $quiz->name, 'idnumber' => $quiz->cmidnumber);
     } else {
         $params = array('itemname' => $quiz->name);

diff --git a/user/filters/checkbox.php b/user/filters/checkbox.php
@@ -100,12 +100,12 @@ public function check_data($formdata) {
         // Check if disable if options are set. if yes then don't add this..
         if (!empty($this->disableelements) && is_array($this->disableelements)) {
             foreach ($this->disableelements as $disableelement) {
-                if (array_key_exists($disableelement, $formdata)) {
+                if (property_exists($formdata, $disableelement)) {
                     return false;
                 }
             }
         }
-        if (array_key_exists($field, $formdata) and $formdata->$field !== '') {
+        if (property_exists($formdata, $field) and $formdata->$field !== '') {
             return array('value' => (string)$formdata->$field);
         }
         return false;

diff --git a/user/filters/cohort.php b/user/filters/cohort.php
@@ -92,7 +92,7 @@ public function check_data($formdata) {
         $field    = $this->_name;
         $operator = $field.'_op';
 
-        if (array_key_exists($operator, $formdata)) {
+        if (property_exists($formdata, $operator)) {
             if ($formdata->$operator != 5 and $formdata->$field == '') {
                 // No data - no change except for empty filter.
                 return false;

diff --git a/user/filters/courserole.php b/user/filters/courserole.php
@@ -98,7 +98,7 @@ public function check_data($formdata) {
         $role     = $field .'_rl';
         $category = $field .'_ct';
 
-        if (array_key_exists($field, $formdata)) {
+        if (property_exists($formdata, $field)) {
             if (empty($formdata->$field) and empty($formdata->$role) and empty($formdata->$category)) {
                 // Nothing selected.
                 return false;

diff --git a/user/filters/globalrole.php b/user/filters/globalrole.php
@@ -82,7 +82,7 @@ public function setupForm(&$mform) {
     public function check_data($formdata) {
         $field = $this->_name;
 
-        if (array_key_exists($field, $formdata) and !empty($formdata->$field)) {
+        if (property_exists($formdata, $field) and !empty($formdata->$field)) {
             return array('value' => (int)$formdata->$field);
         }
         return false;

diff --git a/user/filters/profilefield.php b/user/filters/profilefield.php
@@ -122,7 +122,7 @@ public function check_data($formdata) {
         $operator = $field.'_op';
         $profile  = $field.'_fld';
 
-        if (array_key_exists($profile, $formdata)) {
+        if (property_exists($formdata, $profile)) {
             if ($formdata->$operator < 5 and $formdata->$field === '') {
                 return false;
             }

diff --git a/user/filters/select.php b/user/filters/select.php
@@ -108,7 +108,7 @@ public function check_data($formdata) {
         $field    = $this->_name;
         $operator = $field.'_op';
 
-        if (array_key_exists($field, $formdata) and !empty($formdata->$operator)) {
+        if (property_exists($formdata, $field) and !empty($formdata->$operator)) {
             return array('operator' => (int)$formdata->$operator,
                          'value'    => (string)$formdata->$field);
         }

diff --git a/user/filters/simpleselect.php b/user/filters/simpleselect.php
@@ -86,7 +86,7 @@ public function setupForm(&$mform) {
     public function check_data($formdata) {
         $field = $this->_name;
 
-        if (array_key_exists($field, $formdata) and $formdata->$field !== '') {
+        if (property_exists($formdata, $field) and $formdata->$field !== '') {
             return array('value' => (string)$formdata->$field);
         }
 

diff --git a/user/filters/text.php b/user/filters/text.php
@@ -96,7 +96,7 @@ public function check_data($formdata) {
         $field    = $this->_name;
         $operator = $field.'_op';
 
-        if (array_key_exists($operator, $formdata)) {
+        if (property_exists($formdata, $operator)) {
             if ($formdata->$operator != 5 and $formdata->$field == '') {
                 // No data - no change except for empty filter.
                 return false;

diff --git a/user/tests/privacy_test.php b/user/tests/privacy_test.php
@@ -116,11 +116,11 @@ public function test_export_user_data() {
         $courserequestdata = (array) $writer->get_data([get_string('privacy:courserequestpath', 'user')]);
         $entry = array_shift($courserequestdata);
         // Make sure that the password is not exported.
-        $this->assertFalse(array_key_exists('password', $entry));
+        $this->assertFalse(property_exists($entry, 'password'));
         // Check that some of the other fields are present.
-        $this->assertTrue(array_key_exists('fullname', $entry));
-        $this->assertTrue(array_key_exists('shortname', $entry));
-        $this->assertTrue(array_key_exists('summary', $entry));
+        $this->assertTrue(property_exists($entry, 'fullname'));
+        $this->assertTrue(property_exists($entry, 'shortname'));
+        $this->assertTrue(property_exists($entry, 'summary'));
 
          // User details.
         $userdata = (array) $writer->get_data([]);