Make WordPress Core

Changeset 56354


Ignore:
Timestamp:
08/03/2023 06:11:36 PM (10 months ago)
Author:
hellofromTonya
Message:

Code Modernization: Deprecate dynamic properties in WP_Text_Diff_Renderer_Table magic methods.

The unknown use of unknown dynamic property within the WP_Text_Diff_Renderer_Table property magic methods is now deprecated. A descriptive deprecation notice is provided to alert developers to declare the property on the child class extending WP_Text_Diff_Renderer_Table.

Changes in this commit:

  • Adds a deprecation notice to the __get(), __set(), __isset(), __unset() magic methods, i.e. to alert and inform developers when attempting to get/set/isset/unset a dynamic property.
  • Fixes __get() to explicitly returns null when attempting to get a dynamic property.
  • Fixes __set() by removing the value return after setting a declared property, as (a) unnecessary and (b) __set() should return void per the PHP handbook.
  • Fixes __isset() to return false if not in the $compat_fields, as isset() and __isset() should always return bool:
  • Adds a test class with happy and unhappy paths for these changes.

For backward compatibility, no changes are made to the internal declared properties listed in $compat_fields and accessed through the magic methods.

For example:
A child class uses a property named $data that is not declared as a property on the child class. When getting its value, e.g. $user_query->data, the WP_Text_Diff_Renderer_Table::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not declared. Setting a dynamic property is deprecated since version 6.4.0! Instead, declare the property on the class.

Why not remove the magic methods, remove the $compat_fields property, and restore the properties public?

tl;dr Backward compatibility.

If a plugin adds a property to $compat_fields array, then sites using that plugin would experience (a) an Undefined property Warning (PHP 8) | Notice (PHP 7) and (b) a possible change in behavior.

Why not limit the deprecation for PHP versions >= 8.2?

tl;dr original design intent and inform

The magic methods and $compat_fields property were added for one purpose: to continue providing external access to internal properties declared on WP_Text_Diff_Renderer_Table. They were not intended to be used for dynamic properties.

Deprecating that unintended usage both alerts developers a change is needed in their child class and informs them what to change.

References:

Follow-up to [28525], [31135].

Props antonvlasenko, rajinsharwar, jrf, markjaquith, hellofromTonya, SergeyBiryukov, desrosj, peterwilsoncc, audrasjb, costdev, oglekler, jeffpaul.
Fixes #58898.
See #56034.

Location:
trunk
Files:
2 added
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/wp-includes/class-wp-text-diff-renderer-table.php

    r56178 r56354  
    512512     *
    513513     * @since 4.0.0
     514     * @since 6.4.0 Getting a dynamic property is deprecated.
    514515     *
    515516     * @param string $name Property to get.
    516      * @return mixed Property.
     517     * @return mixed A declared property's value, else null.
    517518     */
    518519    public function __get( $name ) {
     
    520521            return $this->$name;
    521522        }
     523
     524        trigger_error(
     525            "The property `{$name}` is not declared. Getting a dynamic property is " .
     526            'deprecated since version 6.4.0! Instead, declare the property on the class.',
     527            E_USER_DEPRECATED
     528        );
     529        return null;
    522530    }
    523531
     
    526534     *
    527535     * @since 4.0.0
     536     * @since 6.4.0 Setting a dynamic property is deprecated.
    528537     *
    529538     * @param string $name  Property to check if set.
    530539     * @param mixed  $value Property value.
    531      * @return mixed Newly-set property.
    532540     */
    533541    public function __set( $name, $value ) {
    534542        if ( in_array( $name, $this->compat_fields, true ) ) {
    535             return $this->$name = $value;
    536         }
     543            $this->$name = $value;
     544            return;
     545        }
     546
     547        trigger_error(
     548            "The property `{$name}` is not declared. Setting a dynamic property is " .
     549            'deprecated since version 6.4.0! Instead, declare the property on the class.',
     550            E_USER_DEPRECATED
     551        );
    537552    }
    538553
     
    541556     *
    542557     * @since 4.0.0
     558     * @since 6.4.0 Checking a dynamic property is deprecated.
    543559     *
    544560     * @param string $name Property to check if set.
     
    549565            return isset( $this->$name );
    550566        }
     567
     568        trigger_error(
     569            "The property `{$name}` is not declared. Checking `isset()` on a dynamic property " .
     570            'is deprecated since version 6.4.0! Instead, declare the property on the class.',
     571            E_USER_DEPRECATED
     572        );
     573        return false;
    551574    }
    552575
     
    555578     *
    556579     * @since 4.0.0
     580     * @since 6.4.0 Unsetting a dynamic property is deprecated.
    557581     *
    558582     * @param string $name Property to unset.
     
    561585        if ( in_array( $name, $this->compat_fields, true ) ) {
    562586            unset( $this->$name );
    563         }
     587            return;
     588        }
     589
     590        trigger_error(
     591            "A property `{$name}` is not declared. Unsetting a dynamic property is " .
     592            'deprecated since version 6.4.0! Instead, declare the property on the class.',
     593            E_USER_DEPRECATED
     594        );
    564595    }
    565596}
Note: See TracChangeset for help on using the changeset viewer.