Skip to content

Commit

Permalink
WP/DeprecatedParameters: add support for PHP 8.0+ named parameters
Browse files Browse the repository at this point in the history 
1. Adjusted the way the correct parameter is retrieved to use the new PHPCSUtils 1.0.0-alpha4 `PassedParameters::getParameterFromStack()` method.
2. Verified the parameter name used is in line with the name as per the WP 6.1 release.
    WP has been renaming parameters and is probably not done yet, but it doesn't look like those changes (so far) made it into changelog entries.... _sigh_.
    It also looks like in the past, deprecated parameters were being renamed to `$deprecated` on deprecation. This practice should be strongly discouraged in the context of named parameters, but that's a different discussion and outside of the scope of this commit.
    For the purposes of this exercise, I've taken the _current_ parameter name as the "truth" as support for named parameters hasn't officially been announced yet, so any renames _after_ this moment are the only ones relevant.
3. Adjusted the error message/data determination to take named parameters into account.

Name verification notes:
* Removed the `comments_link()` function.
    Per [the changelog](https://developer.wordpress.org/reference/functions/comments_number/#changelog), the previously deprecated fourth parameter is now being used for something else since WP 5.4.0....
    Far from best practice, but it is what it is, so I've removed the listing for that function.

What has **not** been done: a scan of the WP Core codebase to find newly deprecated parameters.

Includes additional unit tests and updating one existing test to ensure the code logic is properly covered.

Note: I thought of updating the error message to no longer refer to the position of the parameter, but to use the parameter name instead (with a "Found: %s" for displaying value), as in:
```bash
# Current:
The parameter "$variable" at position #2 of wp_new_user_notification() has been deprecated since WordPress version 4.3.1. Instead do not pass the parameter.

# Variation:
The $deprecated parameter of the wp_new_user_notification() function has been deprecated since WordPress version 4.3.1. Instead do not pass the parameter. Found: $variable
```
... but as WP renamed all deprecated parameters to `$deprecated`, this wouldn't necessarily be more descriptive than it is now.

Along the same lines, I considered updating the error code to refer to the param name instead of the position, but aside from it not making things more descriptive as things are, that would also constitute a BC-break which I don't think is warranted.

Opinions appreciated.
  • Loading branch information
@jrfnl
jrfnl committed Dec 8, 2022
1 parent 7057243 commit 9002551
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 29 deletions.
74 changes: 51 additions & 23 deletions WordPress/Sniffs/WP/DeprecatedParametersSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace WordPressCS\WordPress\Sniffs\WP;

use PHPCSUtils\Utils\MessageHelper;
use PHPCSUtils\Utils\PassedParameters;
use PHPCSUtils\Utils\TextStrings;
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
use WordPressCS\WordPress\Helpers\MinimumWPVersionTrait;
Expand Down Expand Up @@ -58,15 +59,14 @@ class DeprecatedParametersSniff extends AbstractFunctionParameterSniff {
* $target_functions = array(
* (string) Function name. => array(
* (int) Target parameter position, 1-based. => array(
* 'value' => (mixed) Expected default value for the
* deprecated parameter. Currently the default
* values: true, false, null, empty arrays and
* both empty and non-empty strings can be
* handled correctly by the process_parameters()
* method. When an additional default value is
* added, the relevant code in the
* process_parameters() method will need to be
* adjusted.
* 'name' => (string|array) Parameter name or list of names if the parameter
* was renamed since the release of PHP 8.0.
* 'value' => (mixed) Expected default value for the deprecated parameter.
* Currently the default values: true, false, null, empty arrays
* and both empty and non-empty strings can be handled correctly
* by the process_parameters() method.
* When an additional default value is added, the relevant code
* in the process_parameters() method will need to be adjusted.
* 'version' => (int) The WordPress version when deprecated.
* )
* )
Expand All @@ -76,194 +76,220 @@ class DeprecatedParametersSniff extends AbstractFunctionParameterSniff {

'add_option' => array(
3 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.3.0',
),
),
'comments_link' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '0.72',
),
2 => array(
'value' => '',
'version' => '1.3.0',
),
),
'comments_number' => array(
4 => array(
'name' => 'deprecated_2',
'value' => '',
'version' => '1.3.0',
),
),
'convert_chars' => array(
2 => array(
'name' => 'deprecated',
'value' => '',
'version' => '0.71',
),
),
'discover_pingback_server_uri' => array(
2 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.7.0',
),
),
'get_category_parents' => array(
5 => array(
'name' => 'deprecated',
'value' => array(),
'version' => '4.8.0',
),
),
'get_delete_post_link' => array(
2 => array(
'name' => 'deprecated',
'value' => '',
'version' => '3.0.0',
),
),
'get_last_updated' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '3.0.0', // Was previously part of MU.
),
),
'get_the_author' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.1.0',
),
),
'get_user_option' => array(
3 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.3.0',
),
),
'get_wp_title_rss' => array(
1 => array(
'name' => 'deprecated',
'value' => '–',
'version' => '4.4.0',
),
),
'is_email' => array(
2 => array(
'name' => 'deprecated',
'value' => false,
'version' => '3.0.0',
),
),
'load_plugin_textdomain' => array(
2 => array(
'name' => 'deprecated',
'value' => false,
'version' => '2.7.0',
),
),
'safecss_filter_attr' => array(
2 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.8.1',
),
),
'the_attachment_link' => array(
3 => array(
'name' => 'deprecated',
'value' => false,
'version' => '2.5.0',
),
),
'the_author' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.1.0',
),
2 => array(
'name' => 'deprecated_echo',
'value' => true,
'version' => '1.5.0',
),
),
'the_author_posts_link' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.1.0',
),
),
'trackback_rdf' => array(
1 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.5.0',
),
),
'trackback_url' => array(
1 => array(
'name' => 'deprecated_echo',
'value' => true,
'version' => '2.5.0',
),
),
'update_blog_option' => array(
4 => array(
'name' => 'deprecated',
'value' => null,
'version' => '3.1.0',
),
),
'update_blog_status' => array(
4 => array(
'name' => 'deprecated',
'value' => null,
'version' => '3.1.0',
),
),
'update_user_status' => array(
4 => array(
'name' => 'deprecated',
'value' => null,
'version' => '3.0.2',
),
),
'unregister_setting' => array(
4 => array(
'name' => 'deprecated',
'value' => '',
'version' => '4.7.0',
),
),
'wp_get_http_headers' => array(
2 => array(
'name' => 'deprecated',
'value' => false,
'version' => '2.7.0',
),
),
'wp_get_sidebars_widgets' => array(
1 => array(
'name' => 'deprecated',
'value' => true,
'version' => '2.8.1',
),
),
'wp_install' => array(
5 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.6.0',
),
),
'wp_new_user_notification' => array(
2 => array(
'name' => 'deprecated',
'value' => null,
'version' => '4.3.1',
),
),
'wp_notify_postauthor' => array(
2 => array(
'name' => 'deprecated',
'value' => null,
'version' => '3.8.0',
),
),
'wp_title_rss' => array(
1 => array(
'name' => 'deprecated',
'value' => '–',
'version' => '4.4.0',
),
),
'wp_upload_bits' => array(
2 => array(
'name' => 'deprecated',
'value' => null,
'version' => '2.0.0',
),
),
'xfn_check' => array(
3 => array(
'name' => 'deprecated',
'value' => '',
'version' => '2.5.0',
),
Expand All @@ -289,13 +315,13 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
$paramCount = \count( $parameters );
foreach ( $this->target_functions[ $matched_content ] as $position => $parameter_args ) {

// Check that number of parameters defined is not less than the position to check.
if ( $position > $paramCount ) {
break;
$found_param = PassedParameters::getParameterFromStack( $parameters, $position, $parameter_args['name'] );
if ( false === $found_param ) {
continue;
}

// The list will need to updated if the default value is not supported.
switch ( $parameters[ $position ]['raw'] ) {
switch ( $found_param['raw'] ) {
case 'true':
$matched_parameter = true;
break;
Expand All @@ -310,7 +336,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
$matched_parameter = array();
break;
default:
$matched_parameter = TextStrings::stripQuotes( $parameters[ $position ]['raw'] );
$matched_parameter = TextStrings::stripQuotes( $found_param['raw'] );
break;
}

Expand All @@ -323,13 +349,16 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
$code = MessageHelper::stringToErrorcode( ucfirst( $matched_content ) . 'Param' . $position . 'Found' );

$data = array(
$parameters[ $position ]['raw'],
$found_param['raw'],
$position,
$matched_content,
$parameter_args['version'],
);

if ( isset( $parameter_args['value'] ) && $position < $paramCount ) {
if ( isset( $parameter_args['value'] )
&& isset( $found_param['name'] ) === false
&& $position < $paramCount
) {
$message .= ' Use "%s" instead.';
$data[] = (string) $parameter_args['value'];
} else {
Expand All @@ -339,5 +368,4 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
MessageHelper::addMessage( $this->phpcsFile, $message, $stackPtr, $is_error, $code, $data, 0 );
}
}

}
18 changes: 16 additions & 2 deletions WordPress/Tests/WP/DeprecatedParametersUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ wp_new_user_notification( '', $variable );
wp_new_user_notification( '', function_name() );
wp_new_user_notification( '', $this->method_name() );

/*
* Test support for named parameters.
*/
// OK: Parameter is not passed.
wp_install( blog_title: '', user_name: '', user_email: '', is_public: '' );
// OK: Another (optional) parameter is passed as the 5th param.
wp_install( blog_title: '', user_name: '', user_email: '', is_public: '', language: '' );
// OK: Parameter is passed with correct default, mixed positional and named params, unconventional order.
wp_install( '', '', deprecated: '', user_email: '', is_public: '' );
// OK: Parameter is passed with correct default, all named params, unconventional order.
wp_install( user_name: '', deprecated: '', user_email: '', blog_title: '', is_public: '' );
// Error: Parameter is passed with incorrect default, unconventional order.
wp_install( is_public: '', user_name: '', user_email: '', deprecated: 'should be empty', blog_title: '' );

// All will give an ERROR. The functions are ordered alphabetically.

add_option( '', '', [] );
Expand All @@ -31,7 +45,6 @@ add_option( '', '', 10 );
add_option( '', '', false );
add_option( '', '', 'deprecated' );
comments_link( 'deprecated', 'deprecated' );
comments_number( '', '', '', 'deprecated' );
convert_chars( '', 'deprecated' );
discover_pingback_server_uri( '', 'deprecated' );
get_category_parents( '', '', '', '', array( 'deprecated') );
Expand All @@ -54,7 +67,7 @@ update_blog_option( '', '', '', 'deprecated' );
update_user_status( '', '', '', 'deprecated' );
wp_get_http_headers( '', 'deprecated' );
wp_get_sidebars_widgets( 'deprecated' );
wp_install( '', '', '', '', 'deprecated' );
wp_install( '', '', '', '', 'deprecated', 'password', 'language' );
wp_new_user_notification( '', 'deprecated' );
wp_notify_postauthor( '', 'deprecated' );
wp_notify_postauthor( '', 'null' ); // Null as a string not null.
Expand All @@ -63,3 +76,4 @@ wp_upload_bits( '', 'deprecated' );
xfn_check( '', '', 'deprecated' );

// All will give an WARNING as they have been deprecated after WP 4.9.
// Nothing yet.
12 changes: 8 additions & 4 deletions WordPress/Tests/WP/DeprecatedParametersUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,20 @@ class DeprecatedParametersUnitTest extends AbstractSniffUnitTest {
* @return array <int line number> => <int number of errors>
*/
public function getErrorList() {
$errors = array_fill( 28, 36, 1 );
$start_line = 42;
$end_line = 76;
$errors = array_fill( $start_line, ( ( $end_line - $start_line ) + 1 ), 1 );

$errors[22] = 1;
$errors[23] = 1;
$errors[24] = 1;

// Named param.
$errors[38] = 1;

// Override number of errors.
$errors[33] = 2;
$errors[48] = 2;
$errors[47] = 2;
$errors[61] = 2;

return $errors;
}
Expand All @@ -48,5 +53,4 @@ public function getErrorList() {
public function getWarningList() {
return array();
}

}

0 comments on commit 9002551

Please sign in to comment.