Changeset 56349 for trunk/tests/phpunit/tests/admin/wpListTable.php

Timestamp:

08/02/2023 06:35:24 PM (3 years ago)

Author:

hellofromTonya

Message:

Code Modernization: Deprecate dynamic properties in WP_List_Table magic methods.

The unknown use of unknown dynamic property within the WP_List_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_List_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.
• Removes returning the value when setting a declared property, as (a) unnecessary and (b) __set() should return void ​per the PHP handbook.
• Adds unit tests for happy and unhappy paths.

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 / defined as a property on the child class. When getting its value, e.g. $list_table->data, the WP_List_Table::__get() magic method is invoked, the following deprecation notice thrown, and null returned:

The property data is not defined. Setting a dynamic (undefined) property is deprecated since version 6.4.0! Instead, define 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.

Several plugins, one of which has over 5M installs, add a property to the $compat_fields array. Removing the property would cause an Undefined property Warning (PHP 8) | Notice (PHP 7) to be thrown. Removing the associated code would change the functionality.

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_List_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:

• Dynamic (non-explicitly declared) properties are deprecated as of PHP 8.2 and are expected to become a fatal error in PHP 9.0.
• A ​live open public working session where these changes were discussed and agreed to.
• ​PHP RFC: Deprecate dynamic properties.

Related to #14579, #22234, #30891.

Follow-up to [15491], [28493], [28521], [28524], [31146].

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

File:

• trunk/tests/phpunit/tests/admin/wpListTable.php (modified) (1 diff)

trunk/tests/phpunit/tests/admin/wpListTable.php

@@ -362 +362 @@
                 );
         }
+
+        /**
+         * @dataProvider data_compat_fields
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__get()
+         *
+         * @param string $property_name Property name to get.
+         * @param mixed $expected       Expected value.
+         */
+        public function test_should_get_compat_fields_defined_property( $property_name, $expected ) {
+                $list_table = new WP_List_Table( array( 'plural' => '_wp_tests__get' ) );
+
+                if ( 'screen' === $property_name ) {
+                        $this->assertInstanceOf( $expected, $list_table->$property_name );
+                } else {
+                        $this->assertSame( $expected, $list_table->$property_name );
+                }
+        }
+
+        /**
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__get()
+         */
+        public function test_should_throw_deprecation_when_getting_dynamic_property() {
+                $this->expectDeprecation();
+                $this->expectDeprecationMessage(
+                        'The property `undefined_property` is not defined. Getting a dynamic (undefined) property is ' .
+                        'deprecated since version 6.4.0! Instead, define the property on the class.'
+                );
+                $this->assertNull( $this->list_table->undefined_property, 'Getting a dynamic property should return null from WP_List_Table::__get()' );
+        }
+
+        /**
+         * @dataProvider data_compat_fields
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__set()
+         *
+         * @param string $property_name Property name to set.
+         */
+        public function test_should_set_compat_fields_defined_property( $property_name ) {
+                $value                            = uniqid();
+                $this->list_table->$property_name = $value;
+
+                $this->assertSame( $value, $this->list_table->$property_name );
+        }
+
+        /**
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__set()
+         */
+        public function test_should_throw_deprecation_when_setting_dynamic_property() {
+                $this->expectDeprecation();
+                $this->expectDeprecationMessage(
+                        'The property `undefined_property` is not defined. Setting a dynamic (undefined) property is ' .
+                        'deprecated since version 6.4.0! Instead, define the property on the class.'
+                );
+                $this->list_table->undefined_property = 'some value';
+        }
+
+        /**
+         * @dataProvider data_compat_fields
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__isset()
+         *
+         * @param string $property_name Property name to check.
+         * @param mixed $expected       Expected value.
+         */
+        public function test_should_isset_compat_fields_defined_property( $property_name, $expected ) {
+                $actual = isset( $this->list_table->$property_name );
+                if ( is_null( $expected ) ) {
+                        $this->assertFalse( $actual );
+                } else {
+                        $this->assertTrue( $actual );
+                }
+        }
+
+        /**
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__isset()
+         */
+        public function test_should_throw_deprecation_when_isset_of_dynamic_property() {
+                $this->expectDeprecation();
+                $this->expectDeprecationMessage(
+                        'The property `undefined_property` is not defined. Checking `isset()` on a dynamic (undefined) property ' .
+                        'is deprecated since version 6.4.0! Instead, define the property on the class.'
+                );
+                $this->assertFalse( isset( $this->list_table->undefined_property ), 'Checking a dyanmic property should return false from WP_List_Table::__isset()' );
+        }
+
+        /**
+         * @dataProvider data_compat_fields
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__unset()
+         *
+         * @param string $property_name Property name to unset.
+         */
+        public function test_should_unset_compat_fields_defined_property( $property_name ) {
+                unset( $this->list_table->$property_name );
+                $this->assertFalse( isset( $this->list_table->$property_name ) );
+        }
+
+        /**
+         * @ticket 58896
+         *
+         * @covers WP_List_Table::__unset()
+         */
+        public function test_should_throw_deprecation_when_unset_of_dynamic_property() {
+                $this->expectDeprecation();
+                $this->expectDeprecationMessage(
+                        'A property `undefined_property` is not defined. Unsetting a dynamic (undefined) property is ' .
+                        'deprecated since version 6.4.0! Instead, define the property on the class.'
+                );
+                unset( $this->list_table->undefined_property );
+        }
+
+        /**
+         * Data provider.
+         *
+         * @return array
+         */
+        public function data_compat_fields() {
+                return array(
+                        '_args'            => array(
+                                'property_name' => '_args',
+                                'expected'      => array(
+                                        'plural'   => '_wp_tests__get',
+                                        'singular' => '',
+                                        'ajax'     => false,
+                                        'screen'   => null,
+                                ),
+                        ),
+                        '_pagination_args' => array(
+                                'property_name' => '_pagination_args',
+                                'expected'      => array(),
+                        ),
+                        'screen'           => array(
+                                'property_name' => 'screen',
+                                'expected'      => WP_Screen::class,
+                        ),
+                        '_actions'         => array(
+                                'property_name' => '_actions',
+                                'expected'      => null,
+                        ),
+                        '_pagination'      => array(
+                                'property_name' => '_pagination',
+                                'expected'      => null,
+                        ),
+                );
+        }
 }