From 1fe666f574bd57050d13eee489ce62f2e9c16174 Mon Sep 17 00:00:00 2001 From: Sergey Biryukov Date: Fri, 30 Sep 2022 00:38:58 +0000 Subject: [PATCH] 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. git-svn-id: https://develop.svn.wordpress.org/trunk@54362 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/class-wp-theme-json.php | 7 +++- tests/phpunit/tests/theme/wpThemeJson.php | 49 ++++++++++++++++++++++- 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/wp-includes/class-wp-theme-json.php b/src/wp-includes/class-wp-theme-json.php index 749ee90c0cea4..4f5627a7904b9 100644 --- a/src/wp-includes/class-wp-theme-json.php +++ b/src/wp-includes/class-wp-theme-json.php @@ -1709,7 +1709,12 @@ protected static function compute_style_properties( $styles, $settings = array() * @return string|array Style property value. */ 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 ''; + } /* * This converts references to a path to the value at that path diff --git a/tests/phpunit/tests/theme/wpThemeJson.php b/tests/phpunit/tests/theme/wpThemeJson.php index 4cc81c98b2327..1eb8bb4f6eea3 100644 --- a/tests/phpunit/tests/theme/wpThemeJson.php +++ b/tests/phpunit/tests/theme/wpThemeJson.php @@ -2996,6 +2996,54 @@ public function test_get_property_value_valid() { $this->assertSame( $expected, $theme_json->get_stylesheet() ); } + /** + * 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,7 +3132,6 @@ public function test_get_property_value_self() { $this->assertSame( $expected, $theme_json->get_stylesheet() ); } - /** * @dataProvider data_get_layout_definitions *