Changeset 54362

Timestamp:

09/30/2022 12:38:58 AM (2 years ago)

Author:

SergeyBiryukov

Message:

Code Modernization: Fix null to non-nullable deprecation in WP_Theme_JSON::get_property_value().

This commit aims to fix errors caused by incorrect usage of the strncmp() function inside the WP_Theme_JSON::get_property_value() method on PHP 8.1 and above.

Some history of the affected code:

• [50973] introduced the WP_Theme_JSON::get_property_value() method.
• [54162] removed the $default parameter from the _wp_array_get() call in the method.

With the latter change, the default value that is returned if the path does not exist within the array, or if $array or $path are not arrays, became null instead of an empty string. Since null would then be unintentionally passed to the strncmp() function further in the code, this caused ~35 errors in the test suite along the lines of:

1) Tests_Blocks_Editor::test_get_block_editor_settings_theme_json_settings
strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated

/var/www/src/wp-includes/class-wp-theme-json.php:1754
/var/www/src/wp-includes/class-wp-theme-json.php:1641
/var/www/src/wp-includes/class-wp-theme-json.php:2066
/var/www/src/wp-includes/class-wp-theme-json.php:1002
/var/www/src/wp-includes/class-wp-theme-json.php:898
/var/www/src/wp-includes/global-styles-and-settings.php:140
/var/www/src/wp-includes/block-editor.php:421
/var/www/tests/phpunit/tests/blocks/editor.php:388
/var/www/vendor/bin/phpunit:123

This commit includes:

• Restoring the $default value for _wp_array_get() call.
• Adding an early return if the value is an empty string or null.
• Adding a dedicated unit test to ensure that the method returns a string for invalid paths or null values.

Follow-up to [50973], [54162].

Props antonvlasenko, jrf, imadarshakshat, SergeyBiryukov.
Fixes #56620.

Location:

trunk

Files:

• src/wp-includes/class-wp-theme-json.php (modified) (1 diff)
• tests/phpunit/tests/theme/wpThemeJson.php (modified) (2 diffs)

trunk/src/wp-includes/class-wp-theme-json.php

@@ -1710 +1710 @@
      */
     protected static function get_property_value( $styles, $path, $theme_json = null ) {
-        $value = _wp_array_get( $styles, $path );
+        $value = _wp_array_get( $styles, $path, '' );
+
+        if ( '' === $value || null === $value ) {
+            // No need to process the value further.
+            return '';
+        }
 
         /*

trunk/tests/phpunit/tests/theme/wpThemeJson.php

@@ -2998 +2998 @@
 
     /**
+     * Tests that get_property_value() static method returns an empty string
+     * if the path is invalid or the value is null.
+     *
+     * Also, tests that PHP 8.1 "passing null to non-nullable" deprecation notice
+     * is not thrown when passing the value to strncmp() in the method.
+     *
+     * The notice that we should not see:
+     * `Deprecated: strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated`.
+     *
+     * @dataProvider data_get_property_value_should_return_string_for_invalid_paths_or_null_values
+     *
+     * @ticket 56620
+     *
+     * @covers WP_Theme_JSON::get_property_value
+     *
+     * @param array $styles An array with style definitions.
+     * @param array $path   Path to the desired properties.
+     *
+     */
+    public function test_get_property_value_should_return_string_for_invalid_paths_or_null_values( $styles, $path ) {
+        $reflection_class = new ReflectionClass( WP_Theme_JSON::class );
+
+        $get_property_value_method = $reflection_class->getMethod( 'get_property_value' );
+        $get_property_value_method->setAccessible( true );
+        $result = $get_property_value_method->invoke( null, $styles, $path );
+
+        $this->assertSame( '', $result );
+    }
+
+    /**
+     * Data provider for test_get_property_value_should_return_string_for_invalid_paths_or_null_values().
+     *
+     * @return array
+     */
+    public function data_get_property_value_should_return_string_for_invalid_paths_or_null_values() {
+        return array(
+            'empty string' => array(
+                'styles' => array(),
+                'path'   => array( 'non_existent_path' ),
+            ),
+            'null'         => array(
+                'styles' => array( 'some_null_value' => null ),
+                'path'   => array( 'some_null_value' ),
+            ),
+        );
+    }
+
+    /**
      * Testing that dynamic properties in theme.json that refer to other dynamic properties in a loop
      * should be left untouched.
@@ -3084 +3132 @@
         $this->assertSame( $expected, $theme_json->get_stylesheet() );
     }
-
 
     /**