From fff4242f1abd4639b45346ae67a6dfbb760dd7ba Mon Sep 17 00:00:00 2001 From: Sergey Biryukov Date: Mon, 9 Aug 2021 17:19:21 +0000 Subject: [PATCH] Code Modernization: Rename the `readonly()` function to `wp_readonly()`. Since PHP 8.1, `readonly` is a reserved keyword and cannot be used as a function name. In order to avoid PHP parser errors, the `readonly()` function was extracted to a separate file and is now only included conditionally on PHP < 8.1. This commit also: * Moves the tests for the `__checked_selected_helper()` function and all the related functions to their own file. * Switches to named data providers. This makes the output when using the `--testdox` option more descriptive and is helpful when trying to debug which data set from a data provider failed the test. * Improves the tests in question to make them feature-complete and expand test coverage. Props jrf, ayeshrajans, haosun, knutsp, swissspidy, SergeyBiryukov. Fixes #53858. git-svn-id: https://develop.svn.wordpress.org/trunk@51586 602fd350-edb4-49c9-b593-d223f7449a82 --- phpunit.xml.dist | 1 + src/wp-includes/general-template.php | 14 +- src/wp-includes/php-compat/readonly.php | 35 +++ tests/phpunit/multisite.xml | 1 + tests/phpunit/tests/general/template.php | 49 ---- .../template_CheckedSelectedHelper.php | 246 ++++++++++++++++++ 6 files changed, 295 insertions(+), 51 deletions(-) create mode 100644 src/wp-includes/php-compat/readonly.php create mode 100644 tests/phpunit/tests/general/template_CheckedSelectedHelper.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 39722fe0a9..5669bb3506 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -5,6 +5,7 @@ backupGlobals="false" colors="true" beStrictAboutTestsThatDoNotTestAnything="true" + beStrictAboutOutputDuringTests="true" > diff --git a/src/wp-includes/general-template.php b/src/wp-includes/general-template.php index ab8877f7be..00351b1b93 100644 --- a/src/wp-includes/general-template.php +++ b/src/wp-includes/general-template.php @@ -4813,17 +4813,27 @@ function disabled( $disabled, $current = true, $echo = true ) { * * Compares the first two arguments and if identical marks as readonly * - * @since 4.9.0 + * @since 5.9.0 * * @param mixed $readonly One of the values to compare * @param mixed $current (true) The other value to compare if not just true * @param bool $echo Whether to echo or just return the string * @return string HTML attribute or empty string */ -function readonly( $readonly, $current = true, $echo = true ) { +function wp_readonly( $readonly, $current = true, $echo = true ) { return __checked_selected_helper( $readonly, $current, $echo, 'readonly' ); } +/* + * Include a compat `readonly()` function on PHP < 8.1. Since PHP 8.1, + * `readonly` is a reserved keyword and cannot be used as a function name. + * In order to avoid PHP parser errors, this function was extracted + * to a separate file and is only included conditionally on PHP < 8.1. + */ +if ( PHP_VERSION_ID < 80100 ) { + require_once __DIR__ . '/php-compat/readonly.php'; +} + /** * Private helper function for checked, selected, disabled and readonly. * diff --git a/src/wp-includes/php-compat/readonly.php b/src/wp-includes/php-compat/readonly.php new file mode 100644 index 0000000000..1e1264631f --- /dev/null +++ b/src/wp-includes/php-compat/readonly.php @@ -0,0 +1,35 @@ += 8.1 results in a fatal error. + * + * @package WordPress + * @since 5.9.0 + */ + +/** + * Outputs the HTML readonly attribute. + * + * Compares the first two arguments and if identical marks as readonly + * + * This function is deprecated, and cannot be used on PHP >= 8.1. + * + * @since 4.9.0 + * @deprecated 5.9.0 Use `wp_readonly` introduced in 5.9.0. + * + * @see wp_readonly() + * + * @param mixed $readonly One of the values to compare + * @param mixed $current (true) The other value to compare if not just true + * @param bool $echo Whether to echo or just return the string + * @return string HTML attribute or empty string + */ +function readonly( $readonly, $current = true, $echo = true ) { + _deprecated_function( __FUNCTION__, '5.9.0', 'wp_readonly()' ); + return wp_readonly( $readonly, $current, $echo ); +} diff --git a/tests/phpunit/multisite.xml b/tests/phpunit/multisite.xml index ba92ea5806..ea9fa9d203 100644 --- a/tests/phpunit/multisite.xml +++ b/tests/phpunit/multisite.xml @@ -5,6 +5,7 @@ backupGlobals="false" colors="true" beStrictAboutTestsThatDoNotTestAnything="true" + beStrictAboutOutputDuringTests="true" > diff --git a/tests/phpunit/tests/general/template.php b/tests/phpunit/tests/general/template.php index d4c14d4ffc..fd788a4cff 100644 --- a/tests/phpunit/tests/general/template.php +++ b/tests/phpunit/tests/general/template.php @@ -571,55 +571,6 @@ class Tests_General_Template extends WP_UnitTestCase { get_template_part( 'template', 'part', array( 'foo' => 'baz' ) ); } - /** - * @ticket 9862 - * @ticket 51166 - * @dataProvider data_selected_and_checked_with_equal_values - * - * @covers ::selected - * @covers ::checked - */ - function test_selected_and_checked_with_equal_values( $selected, $current ) { - $this->assertSame( " selected='selected'", selected( $selected, $current, false ) ); - $this->assertSame( " checked='checked'", checked( $selected, $current, false ) ); - } - - function data_selected_and_checked_with_equal_values() { - return array( - array( 'foo', 'foo' ), - array( '1', 1 ), - array( '1', true ), - array( 1, 1 ), - array( 1, true ), - array( true, true ), - array( '0', 0 ), - array( 0, 0 ), - array( '', false ), - array( false, false ), - ); - } - - /** - * @ticket 9862 - * @ticket 51166 - * @dataProvider data_selected_and_checked_with_non_equal_values - * - * @covers ::selected - * @covers ::checked - */ - function test_selected_and_checked_with_non_equal_values( $selected, $current ) { - $this->assertSame( '', selected( $selected, $current, false ) ); - $this->assertSame( '', checked( $selected, $current, false ) ); - } - - function data_selected_and_checked_with_non_equal_values() { - return array( - array( '0', '' ), - array( 0, '' ), - array( 0, false ), - ); - } - /** * @ticket 44183 * diff --git a/tests/phpunit/tests/general/template_CheckedSelectedHelper.php b/tests/phpunit/tests/general/template_CheckedSelectedHelper.php new file mode 100644 index 0000000000..2c3afa9a80 --- /dev/null +++ b/tests/phpunit/tests/general/template_CheckedSelectedHelper.php @@ -0,0 +1,246 @@ + true, + 'checked' => true, + 'disabled' => true, + 'wp_readonly' => true, + ); + + /** + * Tests that the return value for selected() is as expected with equal values. + * + * @ticket 53858 + * @covers ::selected + */ + public function test_selected_with_equal_values() { + $this->assertSame( " selected='selected'", selected( 'foo', 'foo', false ) ); + } + + /** + * Tests that the return value for checked() is as expected with equal values. + * + * @ticket 53858 + * @covers ::checked + */ + public function test_checked_with_equal_values() { + $this->assertSame( " checked='checked'", checked( 'foo', 'foo', false ) ); + } + + /** + * Tests that the return value for disabled() is as expected with equal values. + * + * @ticket 53858 + * @covers ::disabled + */ + public function test_disabled_with_equal_values() { + $this->assertSame( " disabled='disabled'", disabled( 'foo', 'foo', false ) ); + } + + /** + * Tests that the return value for readonly() is as expected with equal values. + * + * @ticket 53858 + * @covers ::readonly + */ + public function test_readonly_with_equal_values() { + if ( ! function_exists( 'readonly' ) ) { + $this->markTestSkipped( 'readonly() function is not available on PHP 8.1' ); + } + + $this->setExpectedDeprecated( 'readonly' ); + + // Call the function via a variable to prevent a parse error for this file on PHP 8.1. + $fn = 'readonly'; + $this->assertSame( " readonly='readonly'", $fn( 'foo', 'foo', false ) ); + } + + /** + * Tests that the return value for wp_readonly() is as expected with equal values. + * + * @ticket 53858 + * @covers ::wp_readonly + */ + public function test_wp_readonly_with_equal_values() { + $this->assertSame( " readonly='readonly'", wp_readonly( 'foo', 'foo', false ) ); + } + + /** + * @dataProvider data_equal_values + * + * @ticket 9862 + * @ticket 51166 + * @ticket 53858 + * @covers ::__checked_selected_helper + * + * @param mixed $helper One of the values to compare. + * @param mixed $current The other value to compare. + */ + public function test_checked_selected_helper_with_equal_values( $helper, $current ) { + $this->assertSame( " test='test'", __checked_selected_helper( $helper, $current, false, 'test' ) ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_equal_values() { + return array( + 'same value, "foo"; 1: string; 2: string' => array( 'foo', 'foo' ), + 'same value, 1; 1: string; 2: int' => array( '1', 1 ), + 'same value, 1; 1: string; 2: float' => array( '1', 1.0 ), + 'same value, 1; 1: string; 2: bool true' => array( '1', true ), + 'same value, 1; 1: int; 2: int' => array( 1, 1 ), + 'same value, 1; 1: int; 2: float' => array( 1, 1.0 ), + 'same value, 1; 1: int; 2: bool true' => array( 1, true ), + 'same value, 1; 1: float; 2: bool true' => array( 1.0, true ), + 'same value, 1; 1: bool true; 2: bool true' => array( true, true ), + 'same value, 1; 1: float 1.0; 2: float calculation 1.0' => array( 1.0, 3 / 3 ), + 'same value, 0; 1: string; 2: int' => array( '0', 0 ), + 'same value, 0; 1: string; 2: float' => array( '0', 0.0 ), + 'same value, 0; 1: int; 2: int' => array( 0, 0 ), + 'same value, 0; 1: int; 2: float' => array( 0, 0.0 ), + 'same value, empty string; 1: string; 2: string' => array( '', '' ), + 'same value, empty string; 1: empty string; 2: bool false' => array( '', false ), + 'same value, empty string; 1: bool false; 2: bool false' => array( false, false ), + 'same value, empty string; 1: empty string; 2: null' => array( '', null ), + 'same value, empty string; 1: bool false; 2: null' => array( false, null ), + 'same value, null; 1: null; 2: null' => array( null, null ), + ); + } + + /** + * @dataProvider data_non_equal_values + * + * @ticket 9862 + * @ticket 51166 + * @ticket 53858 + * @covers ::__checked_selected_helper + * + * @param mixed $helper One of the values to compare. + * @param mixed $current The other value to compare. + */ + public function test_checked_selected_helper_with_non_equal_values( $helper, $current ) { + $this->assertSame( '', __checked_selected_helper( $helper, $current, false, 'test' ) ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_non_equal_values() { + return array( + '1: string foo; 2: string bar' => array( 'foo', 'bar' ), + '1: string 0; 2: empty string' => array( '0', '' ), + '1: string 0; 2: null' => array( '0', null ), + '1: int 0; 2: empty string' => array( 0, '' ), + '1: int 0; 2: bool true' => array( 0, true ), + '1: int 0; 2: bool false' => array( 0, false ), + '1: int 0; 2: null' => array( 0, null ), + '1: float 0; 2: empty string' => array( 0.0, '' ), + '1: float 0; 2: bool true' => array( 0.0, true ), + '1: float 0; 2: bool false' => array( 0.0, false ), + '1: float 0; 2: null' => array( 0.0, null ), + '1: null; 2: bool true' => array( null, true ), + '1: null 0; 2: string "foo"' => array( null, 'foo' ), + '1: int 1; 2: float 1.5' => array( 1, 1.5 ), + ); + } + + /** + * Tests that the `$echo` parameter is handled correctly and that even when the output is echoed out, + * the text is also returned. + * + * @ticket 53858 + * @covers ::__checked_selected_helper + */ + public function test_checked_selected_helper_echoes_result_by_default() { + $expected = " disabled='disabled'"; + $this->expectOutputString( $expected ); + $this->assertSame( $expected, disabled( 'foo', 'foo' ) ); + } + + /** + * Tests that the function compares against `true` when the second parameter is not passed. + * + * @dataProvider data_checked_selected_helper_default_value_for_second_parameter + * + * @ticket 53858 + * @covers ::__checked_selected_helper + * @covers ::selected + * @covers ::checked + * @covers ::disabled + * @covers ::wp_readonly + * + * @param mixed $input Input value + * @param mixed $expect_output Optional. Whether output is expected. Defaults to false. + */ + public function test_checked_selected_helper_default_value_for_second_parameter( $input, $expect_output = false ) { + $fn = array_rand( $this->child_functions ); + $expected = ''; + + if ( false !== $expect_output ) { + $expected = " {$fn}='{$fn}'"; + if ( 'wp_readonly' === $fn ) { + // Account for the function name not matching the expected output string. + $expected = " readonly='readonly'"; + } + + // Only set output expectation when output is expected, so the test will fail on unexpected output. + $this->expectOutputString( $expected ); + } + + // Function will always return the value, even when echoing it out. + $this->assertSame( $expected, $fn( $input ) ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_checked_selected_helper_default_value_for_second_parameter() { + return array( + 'truthy; boolean true' => array( + 'input' => true, + 'expect_output' => true, + ), + 'truthy; int 1' => array( + 'input' => 1, + 'expect_output' => true, + ), + 'truthy; string 1' => array( + 'input' => '1', + 'expect_output' => true, + ), + 'truthy, but not equal to true' => array( + 'input' => 'foo', + ), + 'falsy; null' => array( + 'input' => null, + ), + 'falsy; bool false' => array( + 'input' => false, + ), + 'falsy; int 0' => array( + 'input' => 0, + ), + ); + } +}