[wp-trac] [WordPress Trac] #30891: Unchecked property overloading is detrimental to OOP.

[WordPress Trac]

#30891: Unchecked property overloading is detrimental to OOP.
--------------------------+-----------------------------
 Reporter:  aercolino     |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  General       |    Version:  4.0
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 Original Summary: Unchecked property overloading is detrimental to OOP.

 [http://php.net/manual/en/language.oop5.overloading.php#language.oop5.overloading.members
 Property] and
 [http://php.net/manual/en/language.oop5.overloading.php#language.oop5.overloading.methods
 Method] overloading (allowed by magic methods !__get, !__set, !__isset,
 !__unset, and !__call) have been thoroughly introduced into WP code for
 Backward Compatibility, maybe starting from Changeset/21512, but mostly
 since version 4.0.0.

 The copy-&-paste-d code I'm referring to is:

 {{{
 #!php
         /**
          * Make private properties readable for backwards compatibility.
          *
          * @since 4.0.0
          * @access public
          *
          * @param string $name Property to get.
          * @return mixed Property.
          */
         public function __get( $name ) {
                 return $this->$name;
         }

         /**
          * Make private properties settable for backwards compatibility.
          *
          * @since 4.0.0
          * @access public
          *
          * @param string $name  Property to set.
          * @param mixed  $value Property value.
          * @return mixed Newly-set property.
          */
         public function __set( $name, $value ) {
                 return $this->$name = $value;
         }

         /**
          * Make private properties checkable for backwards compatibility.
          *
          * @since 4.0.0
          * @access public
          *
          * @param string $name Property to check if set.
          * @return bool Whether the property is set.
          */
         public function __isset( $name ) {
                 return isset( $this->$name );
         }

         /**
          * Make private properties un-settable for backwards
 compatibility.
          *
          * @since 4.0.0
          * @access public
          *
          * @param string $name Property to unset.
          */
         public function __unset( $name ) {
                 unset( $this->$name );
         }

         /**
          * Make private/protected methods readable for backwards
 compatibility.
          *
          * @since 4.0.0
          * @access public
          *
          * @param callable $name      Method to call.
          * @param array    $arguments Arguments to pass when calling.
          * @return mixed|bool Return value of the callback, false
 otherwise.
          */
         public function __call( $name, $arguments ) {
                 return call_user_func_array( array( $this, $name ),
 $arguments );
         }
 }}}

 Unchecked overloading (i.e. lazy programming) is detrimental to OOP
 because it neutralizes all protections. No matter what, all properties and
 methods of classes with unchecked overloading are always public.

 I mean, not only legacy properties and methods that existed before
 introducing unchecked overloading but also all others that '''were, are
 being and will be''' introduced afterward and really meant to be private
 or protected.

 This is a very important issue, whose implications will be appreciated in
 the future. Currently, it all seems fine, but it's an illusion. As time
 goes by, on one side the WP core will continue to migrate to PHP5 (and its
 access rules) and on the other side theme and plugin hackers will continue
 to squeeze WP as much as possible. When core developers introduce
 protection for new properties and methods, they expect that protection to
 be taken into account by the ecosystem, and finally by theme and plugin
 developers, which are the ones those protections were thought for. BUT if
 WP theme plugin and core developers will always be able to access whatever
 they want as they please, then I wonder why PHP5 protections were
 introduced in the first place.

 Here is a tedious but simple fix (i.e. checked overloading):
 {{{
 #!php
 if (! array_key_exists($name, $this->legacy_properties)) {
   $trace = debug_backtrace();
   trigger_error('Undefined property' ..., E_USER_NOTICE);
 }
 // rest of __get, __set, __isset, __unset code here
 }}}
 {{{
 #!php
 if (! array_key_exists($name, $this->legacy_methods)) {
   $trace = debug_backtrace();
   trigger_error('Call to undefined method' ..., E_USER_ERROR);
 }
 // rest of __call code here
 }}}
 If this issue is not fixed ASAP, developers will find these "backdoors"
 and start to use them. Then it gets worse.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30891>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform